Use nonzero_lit #2589

tokatoka · 2024-10-07T17:16:38Z

No description provided.

domenukk · 2024-10-08T16:55:21Z

docs/listings/baby_fuzzer/listing-04/src/main.rs

@@ -77,7 +77,7 @@ fn main() {

 /* ANCHOR: generator */
 // Generator of printable bytearrays of max size 32
- let mut generator = RandPrintablesGenerator::new(32).unwrap();
+ let mut generator = RandPrintablesGenerator::new(NonZeroUsize::new(32).unwrap());


I don't like this change, the mutator is not a performance-critical path, let's just keep it be a normal usize and check the value on creation

In any case, use the generic NonZero::new() instead - it's shorter.

I don't understand.
I thought the point of your previous PR is that you want compile-time check instead of debug_ssert so that users won't pass 0 in inappropriate place & get runtime errors.
but now you say the opposite.

Here, new is only called once. If the new "unwraps" internally or an external NonZero call does it makes no difference for the user nor for performance - but now the API is more ugly..

domenukk · 2024-10-08T16:56:54Z

libafl/src/mutators/mutations.rs

@@ -314,7 +311,7 @@ where
 Ok(MutationResult::Skipped)
 } else {
 let byte = state.rand_mut().choose(input.bytes_mut()).unwrap();
- *byte ^= 1 + state.rand_mut().below(NonZero::new(254).unwrap()) as u8;
+ *byte ^= 1 + state.rand_mut().below(nonzero_lit::usize!(254)) as u8;


It's basically the same length and we only have 30 usages in the whole code base.
I don't think it's worth it adding a new dependency for that (and adding compile time for the extra macro)
Rust optimizes the new away

libafl/src/mutators/mutations.rs

domenukk · 2024-10-08T16:59:30Z

libafl/src/stages/mutational.rs

- Self::transforming_with_max_iterations(mutator, DEFAULT_MUTATIONAL_MAX_ITERATIONS).unwrap()
+ Self::transforming_with_max_iterations(
+ mutator,
+ nonzero_lit::usize!(DEFAULT_MUTATIONAL_MAX_ITERATIONS),


same here, why change the API to be ugly?

domenukk · 2024-10-08T17:02:51Z

I have concerns with the changes:

Adding NonZero to constructors makes them way more ugly to use. We can just check the parameter on creation dynamically. I had this originally and changed it back.
I don't think the extra dependency for nonzero_lit is worth it. It makes the code no more legible and is an extra dependency and extra macro.

tokatoka · 2024-10-08T17:28:26Z

I don't think the extra dependency for nonzero_lit is worth it. It makes the code no more legible and is an extra dependency and extra macro.

unwrap() is more ugly.

domenukk · 2024-10-08T19:16:20Z

I don't think the extra dependency for nonzero_lit is worth it. It makes the code no more legible and is an extra dependency and extra macro.

unwrap() is more ugly.

It's the way the standard library does it, I didn't invent the language

tokatoka · 2024-10-14T22:05:26Z

Adapted the stuff

domenukk · 2024-10-15T00:17:51Z

libafl/src/generators/mod.rs

@@ -119,7 +106,7 @@ impl<S> RandBytesGenerator<S> {
 #[derive(Clone, Debug)]
 /// Generates random printable characters
 pub struct RandPrintablesGenerator<S> {
- max_size: NonZeroUsize,
+ max_size: usize,


Why would we allow a Generator for 0 bytes?

It'll be a bug in 99% of cases?

hmmm okay maybe this guy should have NonZero really

or i think i should just make this usize const

libafl/src/mutators/token_mutations.rs

domenukk · 2024-10-15T00:22:12Z

libafl/src/mutators/token_mutations.rs

@@ -376,7 +379,9 @@ where
 {
 fn mutate(&mut self, state: &mut S, input: &mut I) -> Result<MutationResult, Error> {
 let size = input.bytes().len();
- let Some(nonzero_size) = NonZero::new(size) else {
+ let off = if let Some(nz) = NonZero::new(size) {


Nothing changed here, right?

libafl/src/mutators/tuneable.rs

libafl_bolts/src/rands/mod.rs

libafl_bolts/Cargo.toml

domenukk · 2024-10-15T00:32:59Z

I'm not sure how much sense it makes to let users shoot themselves in the foot with 0 sized generators and 0 stacking.
I still don't see why we don't just keep checking this inside the constructor and return an IllegalValue, that is literally what the error is for...
With the nonzero! maybe using NonZero in the API isn't so ugly anymore?

I'll leave the choice up to you, just wanted to voice my opinion one last time, do what you think is best ;)

tokatoka · 2024-10-15T00:44:01Z

0 size generator is odd but 0 stacking makes sense. because that stacking is about power

libafl/src/generators/mod.rs

domenukk · 2024-10-16T08:48:07Z

libafl/src/generators/mod.rs

- if size == 0 {
- size = 1;
- }
+ let size = 1 + state.rand_mut().below(self.max_size);


This is not correct. You add 1 in new and here you add 1 again.
So a max size of 10 will lead to self.max_size = 11 -> below leads to a value up to 10.
In this line, you add 1 to size so worst case the result has 11 entries.

libafl_bolts/src/lib.rs

domenukk · 2024-10-16T22:46:48Z

libafl/src/generators/mod.rs

+ pub fn new(max_size: usize) -> Self {
+ // # Safety
+ // Saturating added 1 so it's always above 0
+ let max_size = unsafe { NonZeroUsize::new(max_size.saturating_add(1)).unwrap_unchecked() };


We should check for 0 here and return Error::IllegalArgument (or really have max_size be NonZero) - right now specifying a max_size of 0 will result in generated inputs of size 1.

Pick one of the two options (or otherwise return 0 sized inputs lol)

langston-barrett · 2024-10-18T20:02:06Z

libafl_bolts/src/lib.rs

@@ -1050,6 +1050,46 @@ pub unsafe fn set_error_print_panic_hook(new_stderr: RawFd) {
 }));
 }

+// Credit goes to https://github.com/thomcc/nonzero_lit


I don't think that all this complexity is needed, you can just do

use std::num::NonZeroUsize; const nz: NonZeroUsize = match NonZeroUsize::new(4) { Some(x) => x, None => panic!("was zero") };

This has been available since 2021: rust-lang/rust#89508

const Option::unwrap just landed, too: rust-lang/rust#67441

That's what we use currently but it's more ugly.
Maybe we could just do a

const fn nonzero(val: usize) -> NonZeroUsize { NonZero::new(val).unwrap() }

?

Maybe we could just do a

this is not compile time check

We want to use macro because i don't want to see bunches of unwrap() inside our lib

I understand the motivation for a macro, I just think this macro could be implemented in a simpler way: #2624

Marcondiro · 2024-10-21T12:35:40Z

libafl/src/generators/mod.rs

 let mut size = state.rand_mut().below(self.max_size);
- if size == 0 {
- size = 1;
- }
+ size = min(size, 1);


Hey @tokatoka, not sure I'm understanding this change. Now the function will generate only ByteInput of 1 byte?

yeah sorry i am dumb this should be max

tokatoka added 23 commits October 7, 2024 19:15

nonzero_lit

ef94534

nonzero

55d4f51

std instead of core

93aa8aa

a

c6e18b5

l

4c99770

test

e03def1

import

5f7a1e4

api

6bd662e

api

aad97bc

aaaaa

867ab26

apiapi

a98bfe0

Merge branch 'main' into nonzero_lit

2c9b751

api

b93871a

api

3178369

api

3d112d5

mm

41ed81c

api

5161702

non zero

e8b9076

FMT

a7e0e33

pls

841535d

nnnaaasdfadsfafdsa

4f3b107

pls

31cbbde

MM

b8eda99

domenukk reviewed Oct 8, 2024

View reviewed changes

libafl/src/mutators/mutations.rs Show resolved Hide resolved

domenukk reviewed Oct 8, 2024

View reviewed changes

libafl/src/mutators/mutations.rs Show resolved Hide resolved

domenukk reviewed Oct 8, 2024

View reviewed changes

tokatoka added 2 commits October 9, 2024 19:26

Merge branch 'main' into nonzero_lit

d0c5e97

bring back old API, delete added deps

b9becf9

domenukk reviewed Oct 15, 2024

View reviewed changes

libafl/src/mutators/token_mutations.rs Outdated Show resolved Hide resolved

domenukk reviewed Oct 15, 2024

View reviewed changes

libafl/src/mutators/tuneable.rs Show resolved Hide resolved

domenukk reviewed Oct 15, 2024

View reviewed changes

libafl_bolts/src/rands/mod.rs Show resolved Hide resolved

domenukk reviewed Oct 15, 2024

View reviewed changes

libafl_bolts/Cargo.toml Outdated Show resolved Hide resolved

tokatoka added 4 commits October 15, 2024 15:24

fix

6f11468

a

d352e00

sat add

cbdf3f5

aa

39d0c8a

domenukk reviewed Oct 15, 2024

View reviewed changes

libafl/src/generators/mod.rs Outdated Show resolved Hide resolved

domenukk reviewed Oct 16, 2024

View reviewed changes

libafl_bolts/src/lib.rs Outdated Show resolved Hide resolved

tokatoka added 3 commits October 16, 2024 21:47

mistake

b97b001

unreachable

66356c9

no generic

cacac93

domenukk reviewed Oct 16, 2024

View reviewed changes

tokatoka added 3 commits October 18, 2024 14:33

api change

c4ac518

Merge branch 'main' into nonzero_lit

e02d70b

a

06c973e

langston-barrett reviewed Oct 18, 2024

View reviewed changes

tokatoka merged commit fda1596 into main Oct 18, 2024
98 checks passed

tokatoka deleted the nonzero_lit branch October 18, 2024 21:33

Marcondiro reviewed Oct 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use nonzero_lit #2589

Use nonzero_lit #2589

tokatoka commented Oct 7, 2024

domenukk Oct 8, 2024

domenukk Oct 8, 2024

tokatoka Oct 8, 2024 •

edited

Loading

domenukk Oct 8, 2024

domenukk Oct 8, 2024

domenukk Oct 8, 2024

domenukk commented Oct 8, 2024

tokatoka commented Oct 8, 2024

domenukk commented Oct 8, 2024

tokatoka commented Oct 14, 2024

domenukk Oct 15, 2024

domenukk Oct 15, 2024

tokatoka Oct 15, 2024

tokatoka Oct 15, 2024

domenukk Oct 15, 2024

domenukk commented Oct 15, 2024 •

edited

Loading

tokatoka commented Oct 15, 2024

domenukk Oct 16, 2024 •

edited

Loading

tokatoka Oct 16, 2024

domenukk Oct 16, 2024

domenukk Oct 16, 2024

langston-barrett Oct 18, 2024

langston-barrett Oct 18, 2024

domenukk Oct 18, 2024

tokatoka Oct 18, 2024

tokatoka Oct 18, 2024

langston-barrett Oct 19, 2024

Marcondiro Oct 21, 2024

tokatoka Oct 21, 2024

Use nonzero_lit #2589

Use nonzero_lit #2589

Conversation

tokatoka commented Oct 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tokatoka Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenukk commented Oct 8, 2024

tokatoka commented Oct 8, 2024

domenukk commented Oct 8, 2024

tokatoka commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenukk commented Oct 15, 2024 • edited Loading

tokatoka commented Oct 15, 2024

domenukk Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tokatoka Oct 8, 2024 •

edited

Loading

domenukk commented Oct 15, 2024 •

edited

Loading

domenukk Oct 16, 2024 •

edited

Loading