dvyukov / go-fuzz

Randomized testing for Go
Apache License 2.0
4.78k stars 279 forks source link

go-fuzz-build: fix two minor bugs #349

Closed josharian closed 1 year ago

josharian commented 1 year ago
josharian commented 1 year ago

I wanted to use go-fuzz (I needed good fuzzing) and was sad to find it had actually bit-rotted for me. This revives it, if only a little longer.

@thepudds I don't suppose I could prevail upon you to review? The changes are pretty small.

josharian commented 1 year ago

Thanks! Going to go ahead and merge. Dmitry, if that doesn’t sit well with you, do but let me know. :)

dvyukov commented 1 year ago

Looks good to me.

dvyukov commented 1 year ago

I needed good fuzzing

What's wrong with the standard library fuzzing?

thepudds commented 1 year ago

Hi Dmitry,

What's wrong with the standard library fuzzing?

I think in general the cmd/go fuzzer is still missing some basics, including it does not yet have anything similar to sonar. One quick example -- dvyukov/go-fuzz finds this ~instantly (thanks to sonar, I believe), but the builtin cmd/go fuzzer would take a very long time to find it:

func FuzzFoo(f *testing.F) {
        f.Fuzz(func(t *testing.T, data []byte) {
                if len(data) < 8 {
                        return
                }
                num := binary.LittleEndian.Uint64(data)
                if num == 0xDEADBEEF {
                        panic("bingo")
                }
        })
}

Some of the type-specific mutations for the cmd/go fuzzer are still too restrictive, I think, including numbers can mutate very slowly and might get better results with a []byte rather than say an int.

There is also not yet any deduplication logic for crashes (which I suspect is one reason -keepfuzzing might not be implemented yet; it currently just stops on first crasher).

My comments here were true previously I think, and I suspect they are still true, but I did not confirm just now. (I've tried to keep an eye on it, but maybe I've missed some improvements).

I think the cmd/go fuzzer is very well designed, especially in terms of the API and general user experience, but in short I think it would benefit from some more TLC, though of course time is in finite supply.

(For me personally, I've had it on my TODO list for a while to contribute more on it, but right now trying to get some other Go contributions off my plate. And quick side note while we are here: one area is trying to take better advantage of your very old CL https://go.dev/cl/3503 😅 ).

josharian commented 1 year ago

What's wrong with the standard library fuzzing?

I haven't done as much analysis as @thepudds (nice!), but the bottom line is the same: go-fuzz finds lots of bugs that std doesn't and it does it much faster.

I use std for most things because it is easier to share with others, but when I really care about shaking out the bugs, particularly deep/rare bugs, I still reach for go-fuzz.

I keep hoping std will get better, but it doesn't appear it is being worked on at all.

dvyukov commented 1 year ago

Oh, interesting, thanks for the detailed info. I had to use go-fuzz in my project for other reasons (support for older Go versions), so did not have chance to learn about the standard fuzzer in detail. But I was considering switching to the standard version since the minimum supported Go version was bumped. Need to consider that more carefully :)