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
  1. darosior commented at 7:53 pm on January 21, 2025: member
    Just 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.
  2. 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.

    Type Reviewers
    ACK fanquake, dergoegge

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

  3. DrahtBot added the label Docs on Jan 21, 2025
  4. 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 compile clang from source to build an instrumentalized libc++? I figured probably not, but maybe building a libc++ of a given version requires the clang 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).

  5. 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.

  6. 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.
  7. maflcko approved
  8. maflcko commented at 8:13 pm on January 21, 2025: member
    lgtm. Seems fine to add a note/pointer to the CI task.
  9. maflcko commented at 8:45 am on January 22, 2025: member
    Could 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?
  10. darosior commented at 2:36 pm on January 22, 2025: member
    Ah right, forgot to add that.
  11. darosior force-pushed on Jan 22, 2025
  12. doc: add a section about using MSan
    Thanks to Niklas Pieter and Michael for the pointers.
    5c3e4d8b29
  13. darosior force-pushed on Jan 22, 2025
  14. darosior commented at 2:48 pm on January 22, 2025: member
    Done.
  15. fanquake approved
  16. fanquake commented at 11:49 am on January 23, 2025: member
    ACK 5c3e4d8b293fab06d2311a863c675a392f24e383
  17. dergoegge approved
  18. dergoegge commented at 2:17 pm on January 23, 2025: member
    ACK 5c3e4d8b293fab06d2311a863c675a392f24e383
  19. fanquake merged this on Jan 23, 2025
  20. fanquake 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