Skip to content
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

libafl_bolts: more rands improvements #2096

Merged
merged 2 commits into from
Apr 24, 2024
Merged

Conversation

flyingmutant
Copy link
Contributor

Continuing where #2086 left off.

As for faster 32-bit fallback for fast_bound, I realized that it will make the values you get different on 32-bit and 64-bit machines (using the same PRNG seed), which sounds bad.

We should either accept the slight speed penalty on machines without fast full-width 64-bit multiply, or say "gotta go fast" and accept the bias you get from 32-bit multiply version. I think favoring 64-bit platforms here is reasonable, since probably the vast majority of fuzzing is done on them.

@@ -92,7 +92,7 @@ where
fn mutator_mut(&mut self) -> &mut M;

/// Gets the number of iterations this mutator should run for.
fn iterations(&self, state: &mut Z::State) -> Result<u64, Error>;
fn iterations(&self, state: &mut Z::State) -> Result<usize, Error>;
Copy link
Member

Choose a reason for hiding this comment

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

This changes the API a little, @tokatoka you know this code the best, anything against it?

Copy link
Member

Choose a reason for hiding this comment

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

it's good

@domenukk
Copy link
Member

I personally think having 64 and 32 bit behave differently is okay, especially for our primary use case / fuzzing, if it improves performance. When would it be a problem?

@flyingmutant
Copy link
Contributor Author

I personally think having 64 and 32 bit behave differently is okay, especially for our primary use case / fuzzing, if it improves performance. When would it be a problem?

It is often convenient to exchange seeds between different machines/processes, or save them for later exact reproduction of what happened. "Coloring" PRNG behavior and seeds depending on the platform sounds like a footgun, I don't know of any PRNG library that does that.

Regarding speed, to put things in the perspective, taking an old AMD K7 from https://www.agner.org/optimize/instruction_tables.pdf: MUL has latency of 4 cycles and DIV has latency of 40. So 5 (or 4, if LLVM is really smart and sees that we only require the top bits of the product) 32-bit MULs required to emulate a full-width 64-bit one are still at least twice as fast as using modulo.

@domenukk
Copy link
Member

If we want to keep behavior consistent, why move the APIs to usize from u64?

@domenukk domenukk requested a review from tokatoka April 24, 2024 10:10
@tokatoka tokatoka merged commit 1e8667a into AFLplusplus:main Apr 24, 2024
99 checks passed
@flyingmutant flyingmutant deleted the rand branch April 24, 2024 13:41
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.

None yet

3 participants