Skip to content

Add length range to FuzzInputOptions #20914

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AdamGoertz
Copy link
Contributor

@AdamGoertz AdamGoertz commented Aug 3, 2024

Closes #20816

Example test case, fuzzing f64 printing.

test "fuzz example" {
    const Context = struct {
        fn testOne(context: @This(), input: []const u8) anyerror!void {
            _ = context;
            // Try passing `--fuzz` to `zig build test` and see if it manages to fail this test case!
            try std.testing.expect(!std.mem.eql(u8, "canyoufindme", input));
        }
    };
    try std.testing.fuzz(Context{}, Context.testOne, .{ .len_range = .{
        .min = 12,
        .max = 12,
    } });
}

The first run of the fuzzer is seeded with random bytes, with length equal to the minimum allowed length.

@andrewrk
Copy link
Member

andrewrk commented Aug 3, 2024

I'll look at this in a couple days - I have a rather large body of work sitting in the fuzz branch right now.

@AdamGoertz
Copy link
Contributor Author

No problem, I figured. As of yesterday looks like the conflicts would be pretty minimal, so hopefully it'll stay easy.

@andrewrk
Copy link
Member

andrewrk commented Aug 7, 2024

Looks like there were indeed a couple of conflicts.

@AdamGoertz AdamGoertz force-pushed the fuzzer-len-range branch 2 times, most recently from b38cb3a to 2885f0b Compare August 8, 2024 02:38
@AdamGoertz
Copy link
Contributor Author

Conflicts resolved 👍

@andrewrk
Copy link
Member

andrewrk commented Aug 8, 2024

If the minimum length is greater than 0, the corpus must contain at least 1 entry, so that we can use it for the initial run/smoke test of the fuzzer.

No need, it can generate random bytes in this case.

@AdamGoertz
Copy link
Contributor Author

If the minimum length is greater than 0, the corpus must contain at least 1 entry, so that we can use it for the initial run/smoke test of the fuzzer.

No need, it can generate random bytes in this case.

I considered this. The concern I had was with ownership of the memory for holding the input. Since the length range is runtime known we need to allocate it dynamically. Under normal circumstances the fuzzer owns all of the returned memory, but on the initial run it's not clear how to de-allocate the slice returned from fuzzInput. A few options I considered:

  • Use a comptime-known length range:
    This would force the FuzzInputOptions parameter to be comptime known. Being able to load corpus entries from a file (without '@embedfile') would be nice, which this option would preclude. The length range could also be moved to a second parameter.

  • Reorganize the fuzzer code so it is still able to manage the returned input even when not linked to libfuzzer: This might be the best option, but I haven't explored how difficult it would be. My objective was to minimize changes to the current fuzzing architecture.

  • add a 'deinit' function for the fuzzer input: totally possible, just annoying because it would do nothing while actually fuzzing; its only responsibility would be cleaning up this one allocation for the smoke test.

Does one of these (or something else I haven't considered) seem like a better approach?

@AdamGoertz
Copy link
Contributor Author

Ok, a pretty easy solution by adding another global variable to track the bytes allocated for the smoke test. I think that should work.

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

I think I've addressed your objection in #21370. After those changes, you can use std.testing.allocator to allocate the input buffer, and free it after the call to testOne.

@AdamGoertz
Copy link
Contributor Author

Sounds good! Once that lands I'll take another look 👍

@AdamGoertz AdamGoertz force-pushed the fuzzer-len-range branch 2 times, most recently from 845c71f to 7ade963 Compare September 14, 2024 15:21
@AdamGoertz
Copy link
Contributor Author

Looks like the updated version is now green 👍🏻

@AdamGoertz AdamGoertz requested a review from andrewrk October 26, 2024 15:50
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.

std.testing.fuzzInput: introduce a length range option
2 participants