doc: add a section in the fuzzing documentation about using MSan #31704
pull darosior wants to merge 1 commits into bitcoin:master from darosior:2501_doc_fuzz_msan changing 1 files +12 −0-
darosior commented at 7:53 pm on January 21, 2025: memberJust a couple lines in a subsection of the sanitizers section mentioning that using the memory sanitizer is a bit more involve than other sanitizers, describing the steps and pointing to an example.
-
DrahtBot commented at 7:53 pm on January 21, 2025: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage & Benchmarks
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31704.
Reviews
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
-
DrahtBot added the label Docs on Jan 21, 2025
-
in doc/fuzzing.md:120 in ac6011cb85 outdated
112@@ -113,6 +113,15 @@ the qa-assets repo 113 Patience is useful; even with improved throughput, libFuzzer may need days and 114 10s of millions of executions to reach deep/hard targets. 115 116+### Using the MemorySanitizer (MSan) 117+ 118+MSan [requires](https://clang.llvm.org/docs/MemorySanitizer.html#handling-external-code) 119+that all linked code be instrumented. It's necessary to compile a custom libc++ to run a Bitcoin 120+Core fuzz target with MSan. The exact steps to achieve this may vary but involve compiling `clang`
darosior commented at 7:55 pm on January 21, 2025:Is it necessary to compileclang
from source to build an instrumentalized libc++? I figured probably not, but maybe building a libc++ of a given version requires theclang
binary for this very version? Or maybe not required but recommended? Anyhow i kept it here as that’s what the MSan CI job does.
dergoegge commented at 5:23 pm on January 22, 2025:Is it necessary to compile clang from source to build an instrumentalized libc++?
No I don’t think so but the simplest/foolproof way to do it is to just clone llvm and build everything from scratch yourself. Building clang yourself ensures you avoid any version related issues (like you mentioned).
in doc/fuzzing.md:119 in ac6011cb85 outdated
112@@ -113,6 +113,15 @@ the qa-assets repo 113 Patience is useful; even with improved throughput, libFuzzer may need days and 114 10s of millions of executions to reach deep/hard targets. 115 116+### Using the MemorySanitizer (MSan) 117+ 118+MSan [requires](https://clang.llvm.org/docs/MemorySanitizer.html#handling-external-code) 119+that all linked code be instrumented. It's necessary to compile a custom libc++ to run a Bitcoin
sipa commented at 7:58 pm on January 21, 2025:libc++ is the name of one implementation of the C++ standard library. I assume it works with other standard C++ library implementations (like libstdc++ as used on non-Android Linux systems) too?
darosior commented at 2:39 pm on January 22, 2025:I don’t know. Similarly to #31704 (review) i don’t know why not, but i’m not sure. Also their docs only mention libc and libc++. So barring any objection/suggestion i’ll leave it like that.
darosior commented at 2:47 pm on January 22, 2025:Just dropped the sentence altogether.
dergoegge commented at 5:17 pm on January 22, 2025:I assume it works with other standard C++ library implementations (like libstdc++ as used on non-Android Linux systems) too?
In theory it should be possible but I’ve not heard about anyone doing it and I also can’t find anything on google. Gcc also doesn’t support MSan (accroding to https://stackoverflow.com/questions/71675493/how-use-memory-sanitizer-with-use-gcc) and I’m not sure if there is a benefit in mixing MSan instrumented libstdc++ with a llvm based bitcoind build.
in doc/fuzzing.md:116 in ac6011cb85 outdated
112@@ -113,6 +113,15 @@ the qa-assets repo 113 Patience is useful; even with improved throughput, libFuzzer may need days and 114 10s of millions of executions to reach deep/hard targets. 115 116+### Using the MemorySanitizer (MSan)
maflcko commented at 8:12 pm on January 21, 2025:Wrong subsection? (This is under## Run without sanitizers for increased throughput
)
darosior commented at 2:36 pm on January 22, 2025:ugh, thanks.maflcko approvedmaflcko commented at 8:13 pm on January 21, 2025: memberlgtm. Seems fine to add a note/pointer to the CI task.maflcko commented at 8:45 am on January 22, 2025: memberCould also add a note that in most cases valgrind is the easier option, as it doesn’t require a full build with a different config?darosior commented at 2:36 pm on January 22, 2025: memberAh right, forgot to add that.darosior force-pushed on Jan 22, 2025doc: add a section about using MSan
Thanks to Niklas Pieter and Michael for the pointers.
darosior force-pushed on Jan 22, 2025darosior commented at 2:48 pm on January 22, 2025: memberDone.fanquake approvedfanquake commented at 11:49 am on January 23, 2025: memberACK 5c3e4d8b293fab06d2311a863c675a392f24e383dergoegge approveddergoegge commented at 2:17 pm on January 23, 2025: memberACK 5c3e4d8b293fab06d2311a863c675a392f24e383fanquake merged this on Jan 23, 2025fanquake closed this on Jan 23, 2025
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-02-07 18:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me