Skip to content

Add fuzzing to compare against croaring-rs#321

Merged
Kerollmops merged 7 commits intoRoaringBitmap:mainfrom
Dr-Emann:push-npzxvmvqlorl
Apr 22, 2025
Merged

Add fuzzing to compare against croaring-rs#321
Kerollmops merged 7 commits intoRoaringBitmap:mainfrom
Dr-Emann:push-npzxvmvqlorl

Conversation

@Dr-Emann
Copy link
Copy Markdown
Member

This also adds some extra validation which will reject a few more incorrect bitmaps when deserializing, to reject inputs also rejected by croaring, and some tests mirroring the unit tests for corrupted deserialization added in RoaringBitmap/CRoaring#665

I figure this might also be helpful for use in #320 (@lucascool12 FYI).

This has been running for a while on my machine (about 300 million executions over 19 hours) and hasn't found anything new in that time.

@Dr-Emann Dr-Emann requested a review from Kerollmops April 17, 2025 01:46
@Dr-Emann Dr-Emann force-pushed the push-npzxvmvqlorl branch from fb761f5 to d102d74 Compare April 17, 2025 01:49
@Kerollmops
Copy link
Copy Markdown
Member

Wow! That is impressive work. Thank you very much!

Don't we want to run it in place of the current benchmarks in the CI for something like 30 minutes?
What do you think?

@Dr-Emann Dr-Emann force-pushed the push-npzxvmvqlorl branch from d102d74 to f6c70ac Compare April 19, 2025 00:59
@Dr-Emann Dr-Emann force-pushed the push-npzxvmvqlorl branch from 40b9966 to b3cd52f Compare April 21, 2025 02:53
@Dr-Emann
Copy link
Copy Markdown
Member Author

@Kerollmops gave a try at adding 30 minutes of fuzzing to CI instead of the benchmarks, not a github actions expert.

@Kerollmops
Copy link
Copy Markdown
Member

Kerollmops commented Apr 22, 2025

Thank you very much @Dr-Emann 🙏

It seems to me that it worked perfectly in the CI: Continuous integration / fuzz pull_request Successful in 31m

Are the following lines expected?

WARNING: Failed to find function "__sanitizer_acquire_crash_state".
WARNING: Failed to find function "__sanitizer_print_stack_trace".
WARNING: Failed to find function "__sanitizer_set_death_callback".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for those improvements, too!

@Kerollmops Kerollmops added this pull request to the merge queue Apr 22, 2025
Merged via the queue into RoaringBitmap:main with commit a015529 Apr 22, 2025
15 checks passed
@Dr-Emann
Copy link
Copy Markdown
Member Author

Yeah, that's expected. It's fuzzing without a sanitizer (-s none), because it's faster, and we don't expect any memory unsafety issues, the fuzzer has hooks into sanitizers, but it can't use them if we're not running with a sanitizer

@Dr-Emann Dr-Emann deleted the push-npzxvmvqlorl branch April 22, 2025 12:44
@Dr-Emann Dr-Emann mentioned this pull request Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants