-
Notifications
You must be signed in to change notification settings - Fork 73
Add fuzz inputs from my servers and oss-fuzz #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
murchandamus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, appears to just add 30k odd new fuzz inputs
ACK 0dbdd55
|
You are adding around 20k, so I wonder if there is an overlap. Should I redo with your folder added? |
|
Sure, feel free to pull it in, that’s what I pushed the branches for! :) |
|
Ok, running that now. Also, it looks like we are nearing the time limit here. So, I'd say to merge this and then right after open the pull request to delete-nonreduced.sh? |
Approach ACK |
|
Hmm, for some reason I am running out of disk space on my tmpfs |
|
Now the task is |
|
Ok, now it passed, with this patch applied to avoid out-of-memory and out-of-storage: diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py
index 9d64591111..591689b11c 100755
--- a/test/fuzz/test_runner.py
+++ b/test/fuzz/test_runner.py
@@ -278,6 +278,8 @@ def merge_inputs(*, fuzz_pool, corpus, test_list, src_dir, build_dir, merge_dirs
args = [
os.path.join(build_dir, 'src', 'test', 'fuzz', 'fuzz'),
'-set_cover_merge=1',
+ '-rss_limit_mb=8000',
+ f'-merge_control_file=/run/media/marco/scratch_fde/fuzz/A_m_{t}.txt',
# set_cover_merge is used instead of -merge=1 to reduce the overall
# size of the qa-assets git repository a bit, but more importantly,
# to cut the runtime to iterate over all fuzz inputs [0]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I verified that the second commit only modifies a bunch of I2P. Both the first and the second commit’s I2P seeds pass on my machine:
➜ qa-assets git:(main) ✗ git reset --hard maflcko/main
Updating files: 100% (36457/36457), done.
HEAD is now at c764d6c7842 Revert i2p fuzz inputs to work around crash for now
➜ build git:(2023-10-refactor-coinselector-tests) ✗ FUZZ=i2p src/test/fuzz/fuzz ../../qa-assets/fuzz_seed_corpus/i2p <<<
i2p: succeeded against 1247 files in 2s.
➜ qa-assets git:(main) ✗ git reset --hard 00c956e4a9c555da5640946ec28cdd5e501c0554
HEAD is now at 00c956e4a9c Add fuzz inputs from my servers, murch, and oss-fuzz
➜ build git:(2023-10-refactor-coinselector-tests) ✗ FUZZ=i2p src/test/fuzz/fuzz ../../qa-assets/fuzz_seed_corpus/i2p
i2p: succeeded against 1256 files in 2s.
🤷
The first commits seems to add a bunch of files into the fuzz_seed_corpus directly instead of one of the subfolders. Any idea what may have happened there?
Can you give an example? |
That also seems funny. You could reproduce yesterday (at least in your folder), I can reproduce locally, and both CI tasks failed on your pull requests with this error. |
|
In any case, let's move discussion to bitcoin/bitcoin#28665 |
|
Which fuzzing engine are you using? Can you try with libFuzzer? I've never seen this command and output: |
|
For reference, this increases the coverage from 63.6% (https://maflcko.github.io/b-c-cov/fuzz.coverage/index.html , unstable link) to 64.4% (https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/091d29c49590d31d/c764d6c7842481a2/fuzz.coverage/index.html) |
|
Could this PR cause CI timeouts? See: bitcoin/bitcoin#28703. |
fa21535 fuzz: Increase merge -rss_limit_mb (MarcoFalke) Pull request description: For some reason, the limit is hit. (Presumably due to `-set_cover_merge=1` eating more memory, or by simply having more fuzz inputs). Fix it by increasing it for the merge operation. ACKs for top commit: dergoegge: ACK fa21535 hebasto: ACK fa21535, considering the discussion in bitcoin-core/qa-assets#155. Tree-SHA512: 4fed0f254eccc6fe0b53656bc345ff898b13811dc39387387317d34b521ab77cee03d82b0896dd92d253b7546b6a7e4bdcd478749f47064374ab44ad759ab9ff
fa21535 fuzz: Increase merge -rss_limit_mb (MarcoFalke) Pull request description: For some reason, the limit is hit. (Presumably due to `-set_cover_merge=1` eating more memory, or by simply having more fuzz inputs). Fix it by increasing it for the merge operation. ACKs for top commit: dergoegge: ACK fa21535 hebasto: ACK fa21535, considering the discussion in bitcoin-core/qa-assets#155. Tree-SHA512: 4fed0f254eccc6fe0b53656bc345ff898b13811dc39387387317d34b521ab77cee03d82b0896dd92d253b7546b6a7e4bdcd478749f47064374ab44ad759ab9ff
fa21535 fuzz: Increase merge -rss_limit_mb (MarcoFalke) Pull request description: For some reason, the limit is hit. (Presumably due to `-set_cover_merge=1` eating more memory, or by simply having more fuzz inputs). Fix it by increasing it for the merge operation. ACKs for top commit: dergoegge: ACK fa21535 hebasto: ACK fa21535, considering the discussion in bitcoin-core/qa-assets#155. Tree-SHA512: 4fed0f254eccc6fe0b53656bc345ff898b13811dc39387387317d34b521ab77cee03d82b0896dd92d253b7546b6a7e4bdcd478749f47064374ab44ad759ab9ff

Doing this in a single commit to allow the fuzz engine to create a smaller set of files.