-
Notifications
You must be signed in to change notification settings - Fork 37.7k
test: only declare a main() when fuzzing with AFL #18008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ACK 6b04aff -- diff looks correct Thanks for fixing this! |
ACK 6b04aff Tested compiling fuzzer tests with libFuzzer which had not been working previously. |
What error am I supposed to be seeing on master (I run into plenty of problems, but don't know if that's because not following instructions correctly)? On macOS 10.15.2 with llvm/9.0.1 (homebrew) I followed the instructions in #17942, first on master then on this PR. On master I ran into |
For me the fuzzers compile when following the instructions from #17942 but when I start any of the fuzzers, they just hang. There is not output and they don't stop until interrupted manually. Running them through the python harness gives an error message about only AFL being supported but that's because the fuzzer hangs and the harness has a timeout built in (error message could be improved though). |
@Sjors It doesn't just exit, the bech32 fuzzer is run. However the log level defaults to Fuzz targets selected: ['bech32']
Run bech32 with args ['/Users/michael/github/fanquake-bitcoin/src/test/fuzz/bech32', '-runs=1', '-detect_leaks=0', '../qa-assets/fuzz_seed_corpus/bech32']
Output: INFO: Seed: 254137248
INFO: Loaded 1 modules (4751 inline 8-bit counters): 4751 [0x1010c4b58, 0x1010c5de7),
INFO: Loaded 1 PC tables (4751 PCs): 4751 [0x1010c5de8,0x1010d86d8),
INFO: 254 files found in ../qa-assets/fuzz_seed_corpus/bech32
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: seed corpus: files: 254 min: 1b max: 713b total: 50505b rss: 27Mb
#255 INITED cov: 1076 ft: 4020 corp: 151/19Kb exec/s: 0 rss: 33Mb
#255 DONE cov: 1076 ft: 4020 corp: 151/19Kb lim: 713 exec/s: 0 rss: 33Mb
Done 255 runs in 0 second(s) |
This is exactly the error you are supposed to see without this patch on macos. The fuzzer will run the main function, but not the one provided by libFuzzer. |
libFuzzer will provide a main(). This also fixes a weak linking issue when fuzzing with libFuzzer on macOS.
6b04aff
to
b35567f
Compare
Updated to drop |
ACK b35567f Can't test on mac, sorry 😬 |
Tested b35567f on macOS 10.15.2 (also checked that AFL still works) |
ACK b35567f Tested libFuzzer on macOS. |
b35567f test: only declare a main() when fuzzing with AFL (fanquake) Pull request description: This fixes fuzzing using [libFuzzer](https://llvm.org/docs/LibFuzzer.html) on macOS, which caused a few issues during the recent review club. macOS users could only fuzz using afl, or inside a VM. It seems that the `__attribute__((weak))` marking is not quite enough to properly mark `main()` as weak on macOS. See Apples docs on [Frameworks and Weak Linking](https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPFrameworks/Concepts/WeakLinking.html#//apple_ref/doc/uid/20002378-107262-CJBJAEID). Have tested fuzzing using libFuzzer and AFL with this patch. ACKs for top commit: MarcoFalke: ACK b35567f fjahr: ACK b35567f Tree-SHA512: b881fdd98c7e1587fcf44debd31f5e7a52df938059ab91c41d0785077b3329b793e051a2bf2eee64488b9f6029d9288c911052ec23ab3ab8c0561a2be1682dae
…ding uninstrumented harnesses with --enable-fuzz. 1087807 tests: Provide main(...) function in fuzzer (practicalswift) Pull request description: Provide `main(...)` function in fuzzer. Allow building uninstrumented harnesses with only `--enable-fuzz`. This PR restores the behaviour to how things worked prior to #18008. #18008 worked around an macOS specific issue but did it in a way which unnecessarily affected platforms not in need of the workaround :) Before this patch: ``` # Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation) $ ./configure --enable-fuzz $ make CXXLD test/fuzz/span /usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/Scrt1.o: In function `_start': (.text+0x20): undefined reference to `main' collect2: error: ld returned 1 exit status Makefile:7244: recipe for target 'test/fuzz/span' failed make[2]: *** [test/fuzz/span] Error 1 make[2]: *** Waiting for unfinished jobs.... $ ``` After this patch: ``` # Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation) $ ./configure --enable-fuzz $ make $ echo foo | src/test/fuzz/span $ ``` The examples above show the change in non-macOS functionality. macOS functionality is unaffected by this patch. ACKs for top commit: MarcoFalke: ACK 1087807 Tree-SHA512: 9c16ea32ffd378057c4fae9d9124636d11e3769374d340f68a1b761b9e3e3b8a33579e60425293c96b8911405d8b96ac3ed378e669ea4c47836af06892aca73d
Summary: This makes it possible to build with libfuzzer on macOS ([[bitcoin/bitcoin#18008 | PR18008]]), fix the issue that this PR introduced for other platforms ([[bitcoin/bitcoin#19366 | PR19366]]) and add the corresponding documentation ([[bitcoin/bitcoin#17942 | PR17942]]). This is a backport of core [[bitcoin/bitcoin#17942 | PR17942]], [[bitcoin/bitcoin#18008 | PR18008]] and [[bitcoin/bitcoin#19366 | PR19366]]. Test Plan: On macOS, read the documentation and follow the instructions. I only ran the instructions for libfuzzer. ninja bitcoin-fuzzers ./test/fuzz/test_runner.py <path_to_corpus> Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Differential Revision: https://reviews.bitcoinabc.org/D8956
This fixes fuzzing using libFuzzer on macOS, which caused a few issues during the recent review club. macOS users could only fuzz using afl, or inside a VM.
It seems that the
__attribute__((weak))
marking is not quite enough to properly markmain()
as weak on macOS. See Apples docs on Frameworks and Weak Linking.Have tested fuzzing using libFuzzer and AFL with this patch.