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 mark main() as weak on macOS. See Apples docs on Frameworks and Weak Linking.
Have tested fuzzing using libFuzzer and AFL with this patch.
ACK 6b04aff38c06ee9db74132c783fde3e72c117ca0 – diff looks correct
Thanks for fixing this!
45@@ -46,6 +46,9 @@ extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv)
46
47 // Declare main(...) "weak" to allow for libFuzzer linking. libFuzzer provides
48 // the main(...) function.
49+#if defined(MAC_OSX) && !defined(__AFL_COMPILER)
50+extern int main(int argc, char** argv) __attribute__((weak_import));
What is the point of declaring main here if it is already declared in libFuzzer?
I guess we could save us all of this __attribute__ wrangling with a simple compile time check:
0#if defined(__AFL_COMPILER)
1static bool read_stdin(std::vector<uint8_t>& data) { ... }
2int main(int argc, char** argv) { ... }
3#endif
main declaration entirely, I just thought it might be clearer in regards to it being weak linked. If you prefer I can replace it with a comment as to why we special case libFuzzer on macOS.
I can replace it with a comment as to why we special case libFuzzer on macOS
My point is that we don’t need to special case libFuzzer on macOS. I think, we can have the same simple code (a single compile time check) for all operating systems. There would be no need for _attribute__ and #if defined(MAC_OSX) wrangling.
should fix the issue where compilation and linking succeeds and the fuzzer just runs “blank”, I think.
That is what is fixed by this PR in its current state. Are you still seeing that issue somewhere else with this change applied?
Sorry, I think I might have been unclear. I see that your patch does fix the issue. However, I think we should go even further: A main function that does not exist should be better than a main function that is weak, as it would properly lead to linker issues instead of odd runtime behaviour.
The following patch (on top of master) should achieve the same result while also removing the main function altogether for fuzzers (except AFL).
0diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp
1index a6ab620e21..5a7b96c89b 100644
2--- a/src/test/fuzz/fuzz.cpp
3+++ b/src/test/fuzz/fuzz.cpp
4@@ -44,9 +44,9 @@ extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv)
5 return 0;
6 }
7
8-// Declare main(...) "weak" to allow for libFuzzer linking. libFuzzer provides
9-// the main(...) function.
10-__attribute__((weak)) int main(int argc, char** argv)
11+// The fuzzer generally provides the main function, except for AFL
12+#if defined(__AFL_COMPILER)
13+int main(int argc, char** argv)
14 {
15 initialize();
16 #ifdef __AFL_INIT
17@@ -74,3 +74,4 @@ __attribute__((weak)) int main(int argc, char** argv)
18 #endif
19 return 0;
20 }
21+#endif
__attribute__ usage in favour of an #ifdef.
ACK 6b04aff38c06ee9db74132c783fde3e72c117ca0
Tested both AFL and libFuzzer using documentation from #17942
Both AFL and libFuzzer build and run without issues.
ACK 6b04aff38c06ee9db74132c783fde3e72c117ca0
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 subprocess timed out: Currently only libFuzzer is supported. With this branch it just exits after Fuzz targets selected: ['bech32']. And then I’m able to run src/test/fuzz/bech32 and watch a bunch of cryptic info scroll by.
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)?
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).
With this branch it just exits after Fuzz targets selected: [‘bech32’] @Sjors It doesn’t just exit, the bech32 fuzzer is run. However the log level defaults to
INFO, which doesn’t generate any output. Pass-l DEBUGand you’ll see:
0Fuzz targets selected: ['bech32']
1Run bech32 with args ['/Users/michael/github/fanquake-bitcoin/src/test/fuzz/bech32', '-runs=1', '-detect_leaks=0', '../qa-assets/fuzz_seed_corpus/bech32']
2Output: INFO: Seed: 254137248
3INFO: Loaded 1 modules (4751 inline 8-bit counters): 4751 [0x1010c4b58, 0x1010c5de7),
4INFO: Loaded 1 PC tables (4751 PCs): 4751 [0x1010c5de8,0x1010d86d8),
5INFO: 254 files found in ../qa-assets/fuzz_seed_corpus/bech32
6INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
7INFO: seed corpus: files: 254 min: 1b max: 713b total: 50505b rss: 27Mb
8[#255](/bitcoin-bitcoin/255/) INITED cov: 1076 ft: 4020 corp: 151/19Kb exec/s: 0 rss: 33Mb
9[#255](/bitcoin-bitcoin/255/) DONE cov: 1076 ft: 4020 corp: 151/19Kb lim: 713 exec/s: 0 rss: 33Mb
10Done 255 runs in 0 second(s)
On master I ran into
subprocess timed out: Currently only libFuzzer is supported.
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.
main() deceleration entirely except for when using AFL.
ACK b35567fe0ba3e6f8d8f9525088eb8ee62065ad01
Can’t test on mac, sorry :grimacing:
ACK b35567f
Tested libFuzzer on macOS.