fuzz: Faster leak check when generating corpus #31481

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2412-fuzz-stable-fast changing 1 files +1 −0
  1. maflcko commented at 10:55 AM on December 12, 2024: member

    Currently, when running the fuzz targets under asan, leak detection may be expensive when global memory is involved on every iteration. For example when SeedRandomStateForTest(SeedRand::ZEROS) is called on every iteration.

    Fix it by disabling leak check on every iteration, but still do it at the end.

  2. DrahtBot commented at 10:55 AM on December 12, 2024: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31481.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK hodlinator

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  3. DrahtBot added the label Tests on Dec 12, 2024
  4. in test/fuzz/test_runner.py:281 in fa10785a7f outdated 
     277 | @@ -278,6 +278,7 @@ def job(command, t, t_env):
 278 |          use_value_profile = int(random.random() < .3)
 279 |          command = [
 280 |              fuzz_bin,
 281 | +            "-detect_leaks=0",  # Disable leak detection on every iteration, it is still run on shutdown.
    maflcko commented at 10:58 AM on December 12, 2024:

    libfuzzer will usually do this by itself, after some time:

    ...
INFO: libFuzzer disabled leak detection after every mutation.
      Most likely the target function accumulates allocated
      memory in a global state w/o actually leaking it.
      You may try running this binary with -trace_malloc=[12]      to get a trace of mallocs and frees.
      If LeakSanitizer is enabled in this process it will still
      run on the process shutdown.
...

    However, it seems better to not rely on that and set the desired value from the start.

    mzumsande commented at 4:36 PM on December 12, 2024:

    This message helped me find a bug recently (I was fuzzing something affecting global state and forgot to clean up after each mutation, so some global vector would grow with each iteration). While that was just a bug in my fuzzing code, I guess the same could happen for some remotely triggerable memory-exhaustion attacks.

    So there may be some value in this info message itself (but obviously only if someone is actually looking at it).

    maflcko commented at 12:41 PM on December 13, 2024:

    I'd say the -rss_limit_mb setting is a better tool to find OOM issues than the leak detector. I understand that the default setting in libfuzzer, or the setting in this file for rss_limit_mb may not be effective at that.

    Not sure what the best solution to this problem, but possibly rss_limit_mb could be set on a per-fuzz-target basis, possibly with a scaling factor for sanitizer builds with higher overhead?

    maflcko commented at 11:43 AM on December 20, 2024:

    Also, the message is hidden anyway by default in the fuzz runner, unless you specified -l DEBUG.

  5. in src/test/fuzz/fuzz.cpp:85 in fa10785a7f outdated 
      77 | @@ -78,6 +78,14 @@ void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target,
  78 |  static std::string_view g_fuzz_target;
  79 |  static const TypeTestOneInput* g_test_one_input{nullptr};
  80 |  
  81 | +inline void test_one_input(FuzzBufferType buffer)
  82 | +{
  83 | +    // Re-seed the global random state for every input, to make every execution
  84 | +    // deterministic and not depend on a previous unrelated input.
  85 | +    SeedRandomStateForTest(SeedRand::ZEROS);
    dergoegge commented at 11:01 AM on December 12, 2024:

    How cheap is it to to this? A lot of tests don't need it...

    maflcko commented at 11:06 AM on December 12, 2024:

    For testing the overhead, I used the fastest target (addition_overflow) and for me the iterations dropped by about 20%. I think this overall overhead is fine, even if not all fuzz targets need it. This is because any fast target will likely already be saturated, or can be saturated quickly, and any slow target will not see the overhead in the overall speed.

    maflcko commented at 11:19 AM on December 12, 2024:

    I guess an alternative would be to install a linter in one of the CI tasks to fail when any of the fuzz targets use randomness without SeedRand::ZEROS. Happy to do this, if you think it is worth it.

    maflcko commented at 12:39 PM on December 13, 2024:

    Alternative in https://github.com/bitcoin/bitcoin/pull/31486

  6. maflcko commented at 11:05 AM on December 12, 2024: member

    For reference, asan has leak detection enabled by default, and it is also set in test/fuzz/test_runner.py in ASAN_OPTIONS.

    For testing that leak detection still works, one can use a diff, something like:

    diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp
index 071e5fb029..a8c52d236a 100644
--- a/src/test/fuzz/addition_overflow.cpp
+++ b/src/test/fuzz/addition_overflow.cpp
@@ -44,6 +44,10 @@ void TestAdditionOverflow(FuzzedDataProvider& fuzzed_data_provider)
 
 FUZZ_TARGET(addition_overflow)
 {
+    const auto* leak = new uint8_t[777]{};
+    FuzzedDataProvider fdp(leak, 777);
+    TestAdditionOverflow<int64_t>(fdp);
+
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     TestAdditionOverflow<int64_t>(fuzzed_data_provider);
     TestAdditionOverflow<uint64_t>(fuzzed_data_provider);
  7. DrahtBot added the label CI failed on Dec 12, 2024
  8. DrahtBot removed the label CI failed on Dec 12, 2024
  9. DrahtBot added the label Needs rebase on Dec 17, 2024
  10. maflcko renamed this:
    fuzz: Faster leak check, and SeedRand::ZEROS before every input
    fuzz: Faster leak check
    on Dec 18, 2024
  11. fuzz: Speed up generation by running leak check less often eeee163513
  12. maflcko force-pushed on Dec 18, 2024
  13. maflcko commented at 8:23 AM on December 18, 2024: member

    rebased to drop the seed commit

  14. DrahtBot removed the label Needs rebase on Dec 18, 2024
  15. in test/fuzz/test_runner.py:278 in eeee163513 
     277 | @@ -278,6 +278,7 @@ def job(command, t, t_env):
 278 |          use_value_profile = int(random.random() < .3)
    hodlinator commented at 10:55 AM on December 20, 2024:

    (Comment about job-function further up)

    Suggested logging improvement in commit 3dd6ef5c174ce36119660cd2ccc82ab09d82300b, here tested with early leak-detection still enabled.

    Before

    Generating corpus to foo
Running '['/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz', '-rss_limit_mb=8000', '-max_total_time=10', '-reload=0', '-use_value_profile=0', PosixPath('foo/addition_overflow')]'
Traceback (most recent call last):
  File "/home/hodlinator/bitcoin/build_fuzz/test/fuzz/test_runner.py", line 412, in <module>
    main()
  File "/home/hodlinator/bitcoin/build_fuzz/test/fuzz/test_runner.py", line 180, in main
    return generate_corpus(
           ^^^^^^^^^^^^^^^^
  File "/home/hodlinator/bitcoin/build_fuzz/test/fuzz/test_runner.py", line 291, in generate_corpus
    future.result()
  File "/nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/lib/python3.12/concurrent/futures/_base.py", line 449, in result
    return self.__get_result()
           ^^^^^^^^^^^^^^^^^^^
  File "/nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/lib/python3.12/concurrent/futures/_base.py", line 401, in __get_result
    raise self._exception
  File "/nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/lib/python3.12/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hodlinator/bitcoin/build_fuzz/test/fuzz/test_runner.py", line 262, in job
    subprocess.run(
  File "/nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/lib/python3.12/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz', '-rss_limit_mb=8000', '-max_total_time=10', '-reload=0', '-use_value_profile=0', PosixPath('foo/addition_overflow')]' returned non-zero exit status 77.

    After

    Generating corpus to foo
Running '['/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz', '-rss_limit_mb=8000', '-max_total_time=10', '-reload=0', '-use_value_profile=0', PosixPath('foo/addition_overflow')]'
Command '['/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz', '-rss_limit_mb=8000', '-max_total_time=10', '-reload=0', '-use_value_profile=0', PosixPath('foo/addition_overflow')]' output:
'INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 168855887
INFO: Loaded 1 modules   (1305991 inline 8-bit counters): 1305991 [0x561fd5b16200, 0x561fd5c54f87), 
INFO: Loaded 1 PC tables (1305991 PCs): 1305991 [0x561fd5c54f88,0x561fd70427f8), 
INFO:        0 files found in foo/addition_overflow
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
[#2](/bitcoin-bitcoin/2/)	INITED cov: 797 ft: 798 corp: 1/1b exec/s: 0 rss: 230Mb
==3390108==WARNING: invalid path to external symbolizer!
==3390108==WARNING: Failed to use and restart external symbolizer!

=================================================================
==3390108==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 777 byte(s) in 1 object(s) allocated from:
    [#0](/bitcoin-bitcoin/0/) 0x561fca60f978  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da0978)
    [#1](/bitcoin-bitcoin/1/) 0x561fca6129c9  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da39c9)
    [#2](/bitcoin-bitcoin/2/) 0x561fca61c3c9  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad3c9)
    [#3](/bitcoin-bitcoin/3/) 0x561fca61c01f  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad01f)
    [#4](/bitcoin-bitcoin/4/) 0x561fca61b603  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dac603)
    [#5](/bitcoin-bitcoin/5/) 0x561fcc4984de  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c294de)
    [#6](/bitcoin-bitcoin/6/) 0x561fcc493c83  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c24c83)
    [#7](/bitcoin-bitcoin/7/) 0x561fcc49391f  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c2491f)
    [#8](/bitcoin-bitcoin/8/) 0x561fca4b3ab8  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c44ab8)
    [#9](/bitcoin-bitcoin/9/) 0x561fca4b66a0  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c476a0)
    [#10](/bitcoin-bitcoin/10/) 0x561fca4b7acb  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c48acb)
    [#11](/bitcoin-bitcoin/11/) 0x561fca4b7ce7  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c48ce7)
    [#12](/bitcoin-bitcoin/12/) 0x561fca4986d1  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c296d1)
    [#13](/bitcoin-bitcoin/13/) 0x561fca29cd72  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5a2dd72)
    [#14](/bitcoin-bitcoin/14/) 0x7ff7486dd27d  (/nix/store/sl141d1g77wvhr050ah87lcyz2czdxa3-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 6a788357de379aee7162f608d25a7692f7162b15)

Direct leak of 777 byte(s) in 1 object(s) allocated from:
    [#0](/bitcoin-bitcoin/0/) 0x561fca60f978  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da0978)
    [#1](/bitcoin-bitcoin/1/) 0x561fca6129c9  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da39c9)
    [#2](/bitcoin-bitcoin/2/) 0x561fca61c3c9  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad3c9)
    [#3](/bitcoin-bitcoin/3/) 0x561fca61c01f  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad01f)
    [#4](/bitcoin-bitcoin/4/) 0x561fca61b603  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dac603)
    [#5](/bitcoin-bitcoin/5/) 0x561fcc4984de  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c294de)
    [#6](/bitcoin-bitcoin/6/) 0x561fcc493c83  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c24c83)
    [#7](/bitcoin-bitcoin/7/) 0x561fcc49391f  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c2491f)
    [#8](/bitcoin-bitcoin/8/) 0x561fca4b3ab8  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c44ab8)
    [#9](/bitcoin-bitcoin/9/) 0x561fca4b66a0  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c476a0)
    [#10](/bitcoin-bitcoin/10/) 0x561fca4b72b0  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c482b0)
    [#11](/bitcoin-bitcoin/11/) 0x561fca4b7ee7  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c48ee7)
    [#12](/bitcoin-bitcoin/12/) 0x561fca4986d1  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c296d1)
    [#13](/bitcoin-bitcoin/13/) 0x561fca29cd72  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5a2dd72)
    [#14](/bitcoin-bitcoin/14/) 0x7ff7486dd27d  (/nix/store/sl141d1g77wvhr050ah87lcyz2czdxa3-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 6a788357de379aee7162f608d25a7692f7162b15)

Direct leak of 777 byte(s) in 1 object(s) allocated from:
    [#0](/bitcoin-bitcoin/0/) 0x561fca60f978  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da0978)
    [#1](/bitcoin-bitcoin/1/) 0x561fca6129c9  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da39c9)
    [#2](/bitcoin-bitcoin/2/) 0x561fca61c3c9  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad3c9)
    [#3](/bitcoin-bitcoin/3/) 0x561fca61c01f  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad01f)
    [#4](/bitcoin-bitcoin/4/) 0x561fca61b603  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dac603)
    [#5](/bitcoin-bitcoin/5/) 0x561fcc4984de  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c294de)
    [#6](/bitcoin-bitcoin/6/) 0x561fcc493c83  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c24c83)
    [#7](/bitcoin-bitcoin/7/) 0x561fcc49391f  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c2491f)
    [#8](/bitcoin-bitcoin/8/) 0x561fca4b3ab8  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c44ab8)
    [#9](/bitcoin-bitcoin/9/) 0x561fca4b76d4  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c486d4)
    [#10](/bitcoin-bitcoin/10/) 0x561fca4b7ce7  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c48ce7)
    [#11](/bitcoin-bitcoin/11/) 0x561fca4986d1  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c296d1)
    [#12](/bitcoin-bitcoin/12/) 0x561fca29cd72  (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5a2dd72)
    [#13](/bitcoin-bitcoin/13/) 0x7ff7486dd27d  (/nix/store/sl141d1g77wvhr050ah87lcyz2czdxa3-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 6a788357de379aee7162f608d25a7692f7162b15)

SUMMARY: AddressSanitizer: 2331 byte(s) leaked in 3 allocation(s).
INFO: to ignore leaks on libFuzzer side use -detect_leaks=0.

MS: 1 ChangeBit-; base unit: adc83b19e793491b1c6ea0fd8b46cd9f32e592fc
0x8,
\010
artifact_prefix='./'; Test unit written to ./leak-8d883f1577ca8c334b7c6d75ccb71209d71ced13
Base64: CA==
'
  16. hodlinator approved
  17. hodlinator commented at 11:02 AM on December 20, 2024: contributor

    ACK eeee163513918540f483925f36b3399430de1c57

    Performance increase

    Changed -max_total_time=60 beneath the line this PR is modifying and ran with & without -detect_leaks=0. The performance improvement is VERY stark. The number of fuzz inputs generated where either around 67 (2 tests) without, and with, it was a staggering 1937 (average from 2 runs). Around 28x faster.

    What it actually does

    Without this PR, fuzzing stops early after 3 leaking runs (using suggested diff adding intential leaks). With this PR, it completes before reporting leaks. This matches the docs at https://llvm.org/docs/LibFuzzer.html which state:

    -detect_leaks If 1 (default) and if LeakSanitizer is enabled try to detect memory leaks during fuzzing (i.e. not only at shut down).

    PR title

    Would prefer the title be renamed "fuzz: Faster leak check when generating corpus", or at least pointing out that it is this part which is optimized in the PR summary.

    Test diff improvement

    More correct size?

    FuzzedDataProvider fdp(leak, 777);
  18. maflcko commented at 11:44 AM on December 20, 2024: member

    Performance increase

    Changed -max_total_time=60 beneath the line this PR is modifying and ran with & without -detect_leaks=0. The performance improvement is VERY stark. The number of fuzz inputs generated where either around 67 (2 tests) without, and with, it was a staggering 1937 (average from 2 runs). Around 28x faster.

    Just for completeness, the performance increase should be mild overall, assuming that the leak detection is turned off automatically anyway for long runs, but I haven't confirmed this.

    If #31481 (review) is too controversial, an alternative could be to just disable it when empty_min_time is active, or just close this pull.

  19. maflcko renamed this:
    fuzz: Faster leak check
    fuzz: Faster leak check when generating corpus
    on Dec 20, 2024
  20. hodlinator commented at 1:01 PM on December 20, 2024: contributor

    Just for completeness, the performance increase should be mild overall, assuming that the leak detection is turned off automatically anyway for long runs, but I haven't confirmed this.

    If #31481 (comment) is too controversial, an alternative could be to just disable it when empty_min_time is active, or just close this pull.

    Yeah, if libfuzzer automatically stops leak detection after a while, I can see a case for closing this PR too, haven't seriously generated corpuses yet, so weigh my ACK accordingly.

  21. maflcko closed this on Jan 2, 2025
  22. maflcko deleted the branch on Jan 2, 2025
  23. bitcoin locked this on Jan 3, 2026
Contributors
maflckoDrahtBotmzumsandedergoeggehodlinator
Labels
Tests
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: 2026-04-21 18:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me