It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that llvm-symbolizer is already in PATH, but this has fixed the 2 issues outlined in #28147.
Use LLVM_SYMBOLIZER_PATH as the env var, as that is somewhat also used inside LLVM, but not consistently, i.e it's checked for in the asan_symbolize script, but not in in the ubsan_symbolize script, or from in compiler-rt.
Alternative to #28804.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For detailed information about the code coverage, see the test coverage report.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | maflcko |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
15 | @@ -16,11 +16,14 @@ 16 | 17 | 18 | def get_fuzz_env(*, target, source_dir): 19 | + symbolizer_path = "/usr/bin/llvm-symbolizer"
Is this true on all platforms? If not, what would happen?
Looks like the CI provides the answer already:
==18180==WARNING: invalid path to external symbolizer!
==18180==WARNING: Failed to use and restart external symbolizer!
test/fuzz/FuzzedDataProvider.h:212:47: runtime error: unsigned integer overflow: 9223372036854775807 - 9223372036854775808 cannot be represented in type 'uint64_t' (aka 'unsigned long')
[#0](/bitcoin-bitcoin/0/) 0x556fc34a1b1b (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1a12b1b) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
[#1](/bitcoin-bitcoin/1/) 0x556fc3512d02 (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1a83d02) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
[#2](/bitcoin-bitcoin/2/) 0x556fc3b52a8c (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x20c3a8c) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
[#3](/bitcoin-bitcoin/3/) 0x556fc33a6024 (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1917024) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
[#4](/bitcoin-bitcoin/4/) 0x556fc33a7221 (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1918221) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
[#5](/bitcoin-bitcoin/5/) 0x556fc33a7827 (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1918827) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
[#6](/bitcoin-bitcoin/6/) 0x556fc3394f7b (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1905f7b) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
[#7](/bitcoin-bitcoin/7/) 0x556fc33bf4e6 (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19304e6) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
[#8](/bitcoin-bitcoin/8/) 0x7fe22802e0cf (/lib/x86_64-linux-gnu/libc.so.6+0x280cf) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89)
[#9](/bitcoin-bitcoin/9/) 0x7fe22802e188 (/lib/x86_64-linux-gnu/libc.so.6+0x28188) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89)
[#10](/bitcoin-bitcoin/10/) 0x556fc3389e44 (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x18fae44) (BuildId: 3ec865ec03abc64911598eb6d119635991ba8624)
SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow test/fuzz/FuzzedDataProvider.h:212:47 in
MS: 0 ; base unit: 0000000000000000000000000000000000000000
artifact_prefix='./'; Test unit written to ./crash-da39a3ee5e6b4b0d3255bfef95601890afd80709
Base64:
Yea, the worst that can happen is the problem we already have. Let's see if we can make this more generic.
I guess there are also other issues here, as the ASAN job does find the symbolizer, but not at that path:
AddressSanitizer: reading suppressions file at /ci_container_base/test/sanitizer_suppressions/ubsan
==28779==Using llvm-symbolizer found at: /usr/bin/llvm-symbolizer-17
==28779==AddressSanitizer Init done
Error parsing command line arguments: ==28779==__tls_get_addr: DTLS_Find 0x7f7962720770 2
Yes, the task does not have llvm installed, but only llvm-17
Yea, my question is why is it now failing, if it's successfully finding a symbolizer.
Ah, it looks like the verbosity=2 output is breaking things that are parsing output:
/usr/bin/python3.11 ../test/util/test_runner.py
2023-11-07 15:13:31,154 - ERROR - Error mismatch:
Expected: Error parsing command line arguments: Invalid command 'foo'
and the ASAN job here would otherwise be working as expected.
nice, lgtm
Changed the approach slightly, which should fix the failing job. This should continue to work everywhere. Might also send a change to LLVM, to try and consolidate the LLVM_SYMBOLIZER_PATH usage.
It's not completely clear to me why this needs to be explicitly
specified in some environments, and not in others, while at the same time
that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues
outlined in #28147.
Use `LLVM_SYMBOLIZER_PATH` as the env var, as that is somewhat also used
inside LLVM, but not consistently, i.e it's checked for in the asan_symbolize
script, but not in in the ubsan_symbolize script, or from in compiler-rt.
lgtm ACK 49d953281df5618430728c0a88471695207f086b
Missing test prefix in pull title?