There are several small sanitizer option issues:
Fix both issues by adding a with_sanitizer_env.py ...cmd... command wrapper that sets the default values.
Also, enable https://clang.llvm.org/docs/ThreadSanitizer.html#adaptive-delay, which may help to find some kind of rare races more frequently.
The package name depends on the system and it should be trivial for a
developer to install it in any way.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline and AI policy for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | Conradgabe, BrandonOdiwuor, sedited |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
To run a command with default sanitizer options.
This allows to de-duplicate the various places where options are
hard-coded.
Also, it should be easier to run tests locally with the options applied.
This allows to run the functional tests without having to manually set
the sanitizer env.
utACk fa0d826eac336238642428a14db9d122aa6ecd4d
Forgot to mention how to test the new sanitizer env:
-DSANITIZERS=undefined,integer,float-divide-by-zero (integer sanitizer requires a lot of suppressions)E.g the functional ones:
./bld-cmake/test/functional/wallet_disable.py
./bld-cmake/test/functional/test_runner.py wallet_disable.py
Or a unit test (requires the env explicitly to pass):
./bld-cmake/bin/test_bitcoin -t util_string_tests # fails
./bld-cmake/test/with_sanitizer_env.py ./bld-cmake/bin/test_bitcoin -t util_string_tests # passes
tACK fa0d826eac336238642428a14db9d122aa6ecd4d
I tested the sanitizer-env wrapper (with_sanitizer_env.py) and its auto-load in functional tests and confirmed both work as described.
I did not test the thread sanitizer / enable_adaptive_delay=1 change.
ACK fa0d826eac336238642428a14db9d122aa6ecd4d
Tested on Ubuntu 24.04, Clang:
with_sanitizer_env.py sets the project defaults, and lets an existing variable replace the whole default string rather than merging keys:<img width="1608" height="597" alt="Image" src="https://github.com/user-attachments/assets/44f16e08-2bb4-47bd-9d95-7c6ea357b1db" />
Integer/UBSan build (-DSANITIZERS=undefined,integer,float-divide-by-zero):
<img width="1915" height="959" alt="Image" src="https://github.com/user-attachments/assets/0921e34c-7219-437e-be0e-6bdfe8e20f8c" />
with_sanitizer_env.py:<img width="986" height="117" alt="Image" src="https://github.com/user-attachments/assets/a9347fb1-67e3-4c16-9ae7-329441ac59aa" />
wallet_disable.py picks up those options with no manual export, both directly and through test_runner.py:<img width="1188" height="385" alt="Image" src="https://github.com/user-attachments/assets/d4bebe48-860c-4439-964c-ff5ebb2085ec" />
enable_adaptive_delay=1 on tsan<img width="1477" height="388" alt="Image" src="https://github.com/user-attachments/assets/dce4ea41-a51c-44fc-b415-e65417a3d01e" />
test-only change with three acks rfm?
utACK fa0d826eac336238642428a14db9d122aa6ecd4d