Conversation
strager
left a comment
strager
left a comment
There was a problem hiding this comment.
See 'must fix' comment. I think this PR will introduce too much flakiness.
Right now, it only runs on Linux. Should we also test this on Windows or Mac?
Nah. I've noticed that libFuzzer isn't the best on macOS. I don't know about Windows, but let's not bother. Linux-only is good for now.
.github/workflows/build-fuzzer.yml
Outdated
| - { runs_on: ubuntu-latest, name: "Clang 13 libc++", container: "ghcr.io/quick-lint/quick-lint-js-github-clang:v1", CC: clang-13, CXX: clang++-13, CFLAGS: "-stdlib=libc++", CMAKE_BUILD_TYPE: "Debug", } | ||
| - { runs_on: ubuntu-latest, name: "Clang 13 libstdc++", container: "ghcr.io/quick-lint/quick-lint-js-github-clang:v1", CC: clang-13, CXX: clang++-13, CFLAGS: "-stdlib=libstdc++", CMAKE_BUILD_TYPE: "Debug", } | ||
| - { runs_on: ubuntu-latest, name: "Clang 13 Release libc++", container: "ghcr.io/quick-lint/quick-lint-js-github-clang:v1", CC: clang-13, CXX: clang++-13, CFLAGS: "-stdlib=libc++", CMAKE_BUILD_TYPE: "Release", } | ||
| - { runs_on: ubuntu-latest, name: "Clang 13 Release libstdc++", container: "ghcr.io/quick-lint/quick-lint-js-github-clang:v1", CC: clang-13, CXX: clang++-13, CFLAGS: "-stdlib=libstdc++", CMAKE_BUILD_TYPE: "Release", } |
There was a problem hiding this comment.
Our CI is slow, and I don't want to add too many more slow jobs.
Let's cut this down to one build. Just "Clang 13 Release libstdc++" should be good.
.github/workflows/build-fuzzer.yml
Outdated
| mkdir fuzz-tmp | ||
| ls build/fuzz/ | ||
| # try running every fuzzer for a very short time | ||
| for FILE in build/fuzz/quick-lint-js-fuzz-*; do echo running: $FILE; $FILE fuzz-tmp -runs=100 || exit 1; done |
There was a problem hiding this comment.
Must fix: We should not run the fuzzers on PRs or branches. Random fuzzer failures should not block a PR or a release.
We can run fuzzers nightly, for example. We can also run fuzzers on PRs and branches if there is a fixed seed or corpus.
In other words:
| Job trigger | Compile fuzzers | Run fuzzers | Acceptable? |
|---|---|---|---|
| PR | yes | no | OK |
| PR | yes | yes, fixed seed or corpus | OK |
| PR | yes | yes, random seed | BAD (introduces flakiness) |
| branch push | yes | no | OK |
| branch push | yes | yes, fixed seed or corpus | OK |
| branch push | yes | yes, random seed | BAD (introduces flakiness) |
| nightly | yes | no | BAD (PRs/branches do this already; waste of resources) |
| nightly | yes | yes, fixed seed or corpus | BAD (PRs/branches do this already; waste of resources) |
| nightly | yes | yes, random seed | OK |
.github/workflows/build-fuzzer.yml
Outdated
| env | grep '^ASAN_OPTIONS\|^CMAKE\|^QUICK_LINT_JS' | sort | ||
| mkdir build | ||
| cd build | ||
| CC=$CMAKE_C_COMPILER CXX=$CMAKE_CXX_COMPILER CFLAGS='-fsanitize=address,undefined,fuzzer-no-link $CMAKE_C_FLAGS' CXXFLAGS='-fsanitize=address,undefined,fuzzer-no-link $CMAKE_CXX_FLAGS' cmake -G Ninja -DCMAKE_BUILD_TYPE=$CMAKE_BUILD_TYPE -DQUICK_LINT_JS_ENABLE_LLVM_LIBFUZZER_TESTS=ON .. |
There was a problem hiding this comment.
Minor: Also add -DBUILD_TESTING=NO to speed up compilation.
Co-authored-by: strager <strager.nds@gmail.com>
2 participants
I added a workflow to check if the fuzzer build still works. It will build the fuzzers like described in
docs/FUZZING.mdand after the build succeeds, it will run each fuzzer for a short time to check if there is an obvious runtime error.To give an example, there are two commits on this branch. 0f9bfee will fail at compile time. c1792af will be caught by ASAN when running the fuzzers.
You should be able to see the workflow runs for the example commits here.
Right now, it only runs on Linux. Should we also test this on Windows or Mac?
fix #926