fuzz: Faster leak check when generating corpus #31481

maflcko · 2024-12-12T10:55:29Z

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.

DrahtBot · 2024-12-12T10:55:32Z

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/31481.

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.

Conflicts

No conflicts as of last run.

src/test/fuzz/fuzz.cpp

maflcko

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);

maflcko · 2024-12-12T10:58:57Z

test/fuzz/test_runner.py

@@ -278,6 +278,7 @@ def job(command, t, t_env):
        use_value_profile = int(random.random() < .3)
        command = [
            fuzz_bin,
+            "-detect_leaks=0",  # Disable leak detection on every iteration, it is still run on shutdown.


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.

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

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?

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

maflcko · 2024-12-18T08:23:39Z

rebased to drop the seed commit

hodlinator

ACK eeee163

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);

hodlinator · 2024-12-20T10:55:44Z

test/fuzz/test_runner.py

@@ -278,6 +278,7 @@ def job(command, t, t_env):
        use_value_profile = int(random.random() < .3)


(Comment about job-function further up)

Suggested logging improvement in commit 3dd6ef5, 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 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 0x561fca60f978 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da0978) #1 0x561fca6129c9 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da39c9) #2 0x561fca61c3c9 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad3c9) #3 0x561fca61c01f (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad01f) #4 0x561fca61b603 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dac603) #5 0x561fcc4984de (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c294de) #6 0x561fcc493c83 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c24c83) #7 0x561fcc49391f (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c2491f) #8 0x561fca4b3ab8 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c44ab8) #9 0x561fca4b66a0 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c476a0) #10 0x561fca4b7acb (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c48acb) #11 0x561fca4b7ce7 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c48ce7) #12 0x561fca4986d1 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c296d1) #13 0x561fca29cd72 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5a2dd72) #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 0x561fca60f978 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da0978) #1 0x561fca6129c9 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da39c9) #2 0x561fca61c3c9 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad3c9) #3 0x561fca61c01f (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad01f) #4 0x561fca61b603 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dac603) #5 0x561fcc4984de (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c294de) #6 0x561fcc493c83 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c24c83) #7 0x561fcc49391f (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c2491f) #8 0x561fca4b3ab8 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c44ab8) #9 0x561fca4b66a0 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c476a0) #10 0x561fca4b72b0 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c482b0) #11 0x561fca4b7ee7 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c48ee7) #12 0x561fca4986d1 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c296d1) #13 0x561fca29cd72 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5a2dd72) #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 0x561fca60f978 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da0978) #1 0x561fca6129c9 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5da39c9) #2 0x561fca61c3c9 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad3c9) #3 0x561fca61c01f (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dad01f) #4 0x561fca61b603 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5dac603) #5 0x561fcc4984de (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c294de) #6 0x561fcc493c83 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c24c83) #7 0x561fcc49391f (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x7c2491f) #8 0x561fca4b3ab8 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c44ab8) #9 0x561fca4b76d4 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c486d4) #10 0x561fca4b7ce7 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c48ce7) #11 0x561fca4986d1 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5c296d1) #12 0x561fca29cd72 (/home/hodlinator/bitcoin/build_fuzz/src/test/fuzz/fuzz+0x5a2dd72) #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== '

maflcko · 2024-12-20T11:44:07Z

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 (comment) is too controversial, an alternative could be to just disable it when empty_min_time is active, or just close this pull.

hodlinator · 2024-12-20T13:01:17Z

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.

DrahtBot added the Tests label Dec 12, 2024

maflcko mentioned this pull request Dec 12, 2024

fuzz: Speed up *_package_eval fuzz targets a bit #31457

Merged

dergoegge reviewed Dec 12, 2024

View reviewed changes

src/test/fuzz/fuzz.cpp Outdated Show resolved Hide resolved

maflcko commented Dec 12, 2024

View reviewed changes

DrahtBot added CI failed and removed CI failed labels Dec 12, 2024

dergoegge mentioned this pull request Dec 13, 2024

fuzz: Abort when using global PRNG without re-seed #31486

Merged

DrahtBot added the Needs rebase label Dec 17, 2024

maflcko changed the title ~~fuzz: Faster leak check, and SeedRand::ZEROS before every input~~ fuzz: Faster leak check Dec 18, 2024

fuzz: Speed up generation by running leak check less often

eeee163

maflcko force-pushed the 2412-fuzz-stable-fast branch from fa10785 to eeee163 Compare December 18, 2024 08:22

DrahtBot removed the Needs rebase label Dec 18, 2024

hodlinator approved these changes Dec 20, 2024

View reviewed changes

maflcko changed the title ~~fuzz: Faster leak check~~ fuzz: Faster leak check when generating corpus Dec 20, 2024

maflcko closed this Jan 2, 2025

maflcko deleted the 2412-fuzz-stable-fast branch January 2, 2025 17:34

maflcko mentioned this pull request Jun 5, 2025

ci: libFuzzer disabled leak detection after every mutation #32681

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fuzz: Faster leak check when generating corpus #31481

fuzz: Faster leak check when generating corpus #31481

Uh oh!

maflcko commented Dec 12, 2024 •

edited

Loading

Uh oh!

DrahtBot commented Dec 12, 2024 •

edited

Loading

Uh oh!

Uh oh!

maflcko left a comment •

edited

Loading

Uh oh!

maflcko Dec 12, 2024

Uh oh!

mzumsande Dec 12, 2024 •

edited

Loading

Uh oh!

maflcko Dec 13, 2024

Uh oh!

maflcko Dec 20, 2024

Uh oh!

maflcko commented Dec 18, 2024

Uh oh!

hodlinator left a comment •

edited

Loading

Uh oh!

hodlinator Dec 20, 2024

Uh oh!

maflcko commented Dec 20, 2024

Performance increase

Uh oh!

hodlinator commented Dec 20, 2024

Uh oh!

Uh oh!

		@@ -278,6 +278,7 @@ def job(command, t, t_env):
		use_value_profile = int(random.random() < .3)

fuzz: Faster leak check when generating corpus #31481

fuzz: Faster leak check when generating corpus #31481

Uh oh!

Conversation

maflcko commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

Uh oh!

maflcko left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

mzumsande Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko Dec 13, 2024

Choose a reason for hiding this comment

Uh oh!

maflcko Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

maflcko commented Dec 18, 2024

Uh oh!

hodlinator left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Performance increase

What it actually does

PR title

Test diff improvement

Uh oh!

hodlinator Dec 20, 2024

Choose a reason for hiding this comment

Before

After

Uh oh!

maflcko commented Dec 20, 2024

Performance increase

Uh oh!

hodlinator commented Dec 20, 2024

Uh oh!

Uh oh!

maflcko commented Dec 12, 2024 •

edited

Loading

DrahtBot commented Dec 12, 2024 •

edited

Loading

maflcko left a comment •

edited

Loading

mzumsande Dec 12, 2024 •

edited

Loading

hodlinator left a comment •

edited

Loading