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

internal/fuzz,testing: test_fuzz_mutator_repeat failures with "inputs are not equal" #49047

Closed
bcmills opened this issue Oct 18, 2021 · 13 comments
Labels
FrozenDueToAge fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Oct 18, 2021

greplogs --dashboard -md -l -e '(?ms)FAIL: TestScript/test_fuzz_mutator_repeat.*inputs are not equal'

2021-10-13T16:36:59-69041c7/windows-amd64-longtest
2021-10-08T00:18:29-7cef831/linux-amd64-longtest
2021-10-05T21:25:51-6ae3afa/windows-amd64-longtest

CC @katiehockman @rolandshoemaker

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker fuzz Issues related to native fuzzing support labels Oct 18, 2021
@bcmills bcmills added this to the Go1.18 milestone Oct 18, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Oct 18, 2021

The inputs in these failures look very similar to me.

(Maybe this has something to do with the Unicode replacement value, if the test input happens not to be valid UTF-8?)

@gopherbot
Copy link

Change https://golang.org/cl/356649 mentions this issue: cmd/go: skip flaky fuzz tests

gopherbot pushed a commit that referenced this issue Oct 19, 2021
(Temporarily, until they can be fixed.)

For #49046
For #49047

Change-Id: Ib580a5e45a0955aabdfc1899ed38a262a37f66ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/356649
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
@katiehockman katiehockman self-assigned this Oct 19, 2021
@katiehockman
Copy link
Contributor

katiehockman commented Oct 20, 2021

I don't think this is a UTF-8 issue. What this test is basically checking is that the coordinator can reconstruct the mutated input if it wasn't provided by the worker (e.g. because of a non-recoverable crash). It reconstructs it by using the count of mutations that is stored in shared memory. If that number is 1 off (either too big or too small), it would look a lot like these errors, since all of the incorrect inputs are only slightly different from the expected inputs.

I'm not able to reproduce this locally, unfortunately, which makes debugging this more difficult.

@katiehockman
Copy link
Contributor

I have a theory. It's a bit of a longshot, but seems plausible?

When fuzzing, if an unrecoverable error occurs, the coordinator uses the count written to memory to reconstruct the input that caused the failure. This count is increased inside fuzzOnce right before we execute fuzzFn (the fuzz function).
The fuzz test in test_fuzz_mutator_repeat writes to a file on the 100th execution of the fuzz target, and exits in a way that is unrecoverable. If an execution of fuzzFn triggers new code coverage, then we attempt to deflake to make sure the coverage is legitimate. This calls fuzzOnce, which would in turn increase count by one before fuzzFn is called.

I see one possible way this number could be off. If the 99th execution of fuzzFn increases coverage, and a crash occurs during the de-flake (i.e. the 100th execution), then the count will accurately describe how many times the fuzz function was called, but will be 1 bigger than the number of times mutate was called.

This would explain why it happens so rarely. The fix for this isn't totally straightforward. We can't for example just not increase the count when we're deflaking, because the count is used for -fuzztime=Xx. But I have a few other ideas that should work.

@gopherbot
Copy link

Change https://golang.org/cl/358094 mentions this issue: internal/fuzz: don't deflake coverage found while fuzzing

@bcmills
Copy link
Contributor Author

bcmills commented Nov 17, 2021

Unfortunately this test is still occasionally failing on linux-386-longtest. (Does it depend on instrumentation?)

greplogs --dashboard -md -l -e 'FAIL: TestScript/test_fuzz_mutator_repeat' --since=2021-10-23

2021-11-17T17:36:36-0555ea3/linux-386-longtest
2021-11-17T17:16:18-ab75484/linux-386-longtest

@bcmills bcmills reopened this Nov 17, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Nov 17, 2021

From some further builds, the recent failures are looking like a regression as of CL 364214.

@gopherbot
Copy link

Change https://golang.org/cl/364758 mentions this issue: cmd/go: skip broken fuzz test

gopherbot pushed a commit that referenced this issue Nov 17, 2021
For #49047

Change-Id: If06ce033f7bfd23d640311f1be261afab8332028
Reviewed-on: https://go-review.googlesource.com/c/go/+/364758
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/364954 mentions this issue: cmd/go: deflake test_fuzz_mutator_repeat

@gopherbot
Copy link

Change https://golang.org/cl/365175 mentions this issue: internal/fuzz: fix chunk swap mutator

@gopherbot
Copy link

Change https://golang.org/cl/365294 mentions this issue: internal/fuzz: compute correct number of mutations

@toothrot
Copy link
Contributor

@rolandshoemaker @katiehockman Is this okay to resolve after beta1? If so, feel free to apply the label for tracking.

gopherbot pushed a commit that referenced this issue Nov 19, 2021
When swapping two chunks of bytes in a slice, don't pick chunks which
extend beyond the end of the slice. Also don't pick chunks which
intersect with each other.

Fixes #49047

Change-Id: I070eb1888d05ae849ec6122d01c40c45e602019f
Reviewed-on: https://go-review.googlesource.com/c/go/+/365175
Trust: Roland Shoemaker <roland@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
Status: No status
Development

No branches or pull requests

4 participants