This change sets a timeout of 10 seconds on each
execution of the fuzz target, both during fuzzing
and during minimization. This is not currently
customizable by the user, but issue #48157 tracks
this work.
Deadlocks will be considered non-recoverable errors,
and as such, will not be minimizable.
Fixes#48591
Change-Id: Ic86e8e9e9a0255e7860f7cbf5654e832785d1cbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/363134
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This doesn't handle every possible scenario,
but improves the one we can control. For example,
if the worker panics for some reason, we have no
way of knowing whether the panic occurred in an
expected way (while executing the fuzz target) or
due to an internal error in the worker. So any
panic will still be treated as a crash.
However, if it fails due to some internal bug that
we know how to catch, then the error should be
reported to the user without a new crasher being
written to testdata.
This is very difficult to test. The reasons an
internal error would occur is because something went
very wrong, and we have a bug in our code (which is
why they were previously panics). So simulating
a problem like this in a test is not really feasible.
Fixes#48804
Change-Id: I334618f84eb4a994a8d17419551a510b1fdef071
Reviewed-on: https://go-review.googlesource.com/c/go/+/361115
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Previously, the worker would attempt to deflake
an input that was reported to have caused new
coverage. The chances of a flake causing new
coverage seem pretty low to me, and even if it
was a flake that caused it, adding that input to
the cache doesn't seem like a bad thing. The
input is already going to be deflaked during
minimization anyway. If by some off-chance the
code is causing a lot of flaky coverage
increases, and the user doesn't want minimization
to occur, then setting -fuzzminimizetime=1x will
deflake in the way they want without minimizing.
This can be documented as needed.
This fixes a bug where the mem.header().count
could have been one too large if an unrecoverable
crash occured while deflaking an input that
caused code coverage.
Fixes#49047
Change-Id: Ibdf893d7a89a46dd700702afb09e35623615390e
Reviewed-on: https://go-review.googlesource.com/c/go/+/358094
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This pulls in some code and tests from CL 353355.
This change makes some refactors for when we read
to and write from memory during minimization.
That fixes a bug when minimizing interesting inputs.
Now, if an error occurs while minimizing an interesting
input, that value will continue to be minimized as a
crash, and returned to the user.
This change also allows minimization of a crash that
occurred during the warmup phase. We don't want to
minimize failures in the seed corpus, but if an entry
in the cache causes a new failure, then there's no
compelling reason why we shouldn't try to minimize it.
Fixes#48731
Change-Id: I7262cecd8ea7ae6fdf932f3a36db55fb062a1f2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/355691
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
This change fixes some issues with -run, and
the subsequent command line output when running
in verbose mode. It replaces CorpusEntry.Name
with CorpusEntry.Path, and refactors the code
accordingly.
This change also adds a lot of additional tests
which check explicit command line output when
fuzz targets are run without fuzzing. This will
be important to avoid regressions.
Updates #48149
Change-Id: If34b1f51db646317b7b51c3c38ae53231d01f568
Reviewed-on: https://go-review.googlesource.com/c/go/+/354632
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This was useful for debugging while we were developing
the feature, but is now causing extraneous prints that
make the command output difficult to read.
This change also prevents the go command from printing
an extraneous "FAIL" when fuzzing is enabled.
Fixes#48633Fixes#46631
Change-Id: I636e65f305a20f6dcd843e62090ae228741a3725
Reviewed-on: https://go-review.googlesource.com/c/go/+/352892
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Don't add race detector instrumentation to the fuzzing counters,
allowing usage of -race without immediately triggering the
detector. Also fixes a minor race in contextReader.Read.
Fixes#48307
Change-Id: Idb2cfeaa4283f8a74473b4bac6cd68eed577e943
Reviewed-on: https://go-review.googlesource.com/c/go/+/351453
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
This change also makes it so that non-recoverable errors (which should
be pretty rare) will no longer be minimized as these failures can be
flakier and harder to minimize successfully.
Updates golang/go#48132
Change-Id: I991d837993ea1fb0304b3ec491cc725ef5265652
Reviewed-on: https://go-review.googlesource.com/c/go/+/351273
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This change refactors some of the code to support skipping a run
of the seed corpus by the go command before runFuzzing occurs.
Previously, the go command would run all seed corpus for all targets
that match the provided `run` argument. This will be redundant when
fuzzing a target. Now, the seed corpus is only run by targets other than
the one that's about to be fuzzed, and the worker handles running and
reporting issues with the seed corpus.
Part of the logic that needed close inspection is what to do if a
failure occurs during a testing-only or coverage-only fail. If the input
is already in the seed corpus, the fuzzing engine shouldn't add it. If
the input is currently in the cache, then it should be written to
testdata. In all cases, if an error occurs, we need to report this to
the user with enough information for them to debug it.
This uncovered some issues with our code when fuzzing without
instrumentation, and when -run=None was provided. There are some logic
fixes in this change, and some small refactors.
Fixesgolang/go#48327Fixesgolang/go#48296
Change-Id: I9ce2be0219c5b09277ddd308df8bc5a46d4558fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/349630
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
If the fuzz RPC finds no error and no new coverage, there's no point
in reconstructing the last value.
If the minimize RPC does not succeed in minimizing, either because the
error can't be reproduced, or new coverage can't be reproduced, or a
new error is found with the input, don't marshal or unmarshal the
input. Just use the original.
Change-Id: I3b0f89b8656a2b36066d82efefac0fff4a61fbee
Reviewed-on: https://go-review.googlesource.com/c/go/+/347233
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Change-Id: I70c0229e43dfe37f70b9c79c2e6fe88d7b8d7bd0
Reviewed-on: https://go-review.googlesource.com/c/go/+/347231
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Instead of holding all corpus data/values in memory, only store seed
inputs added via F.Add in memory, and only load corpus entries which
are written to disk when we need them. This should significantly reduce
the memory required by the coordinator process.
Additionally only load the corpus in the coordinator process, since the
worker has no need for it.
Fixes#46669.
Change-Id: Ic3b0c5e929fdb3e2877b963e6b0fa14e140c1e1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/345096
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
When a fuzz worker discovers an input that activates coverage counters
that weren't previously activated, it sends that input back to the
coordinator, as before. If the coordinator also finds that input
provides new coverage (that is, some other input hasn't won the race),
the coordinator now sends the input back to workers for minimization.
The minimization procedure now supports minimizing these interesting
inputs. It attempts to find smaller inputs that preserve at least one
new coverage bit. If minimization succeeds, the coordinator adds the
smaller input to the corpus instead of the original. If minimization
fails, the coordinator adds the original input. If minimization finds
that the original input didn't provide new coverage after all (for
example, a counter was activated by an unrelated background goroutine
and was considered flaky), the input is ignored and not recorded.
Change-Id: I81d98d6ec28abb0ac2a476f73480ceeaff674c08
Reviewed-on: https://go-review.googlesource.com/c/go/+/342997
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
workerServer.minimize now returns a response with Success = false when
the fuzz function run with the original input does not produce an
error. This may indicate flakiness.
The coordinator still records a crash, but it will use the unminimized
input with its original error message.
When minimization of interesting inputs is supported, Success = false
indicates that new coverage couldn't be reproduced, and the input will
be discarded.
Change-Id: I72c0e9808f0b0e5390dc7b64141cd0d653ee0af3
Reviewed-on: https://go-review.googlesource.com/c/go/+/342996
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
When we start fuzzing, we gather baseline coverage by calling the fuzz
function with each entry in the corpus (testdata, F.Add, and
cache). These calls should count toward -fuzztime when it specifies a
limited number of calls to the fuzz function.
Change-Id: I3ff57b6d0b25e2e22bb4ce24b10f5112fb116311
Reviewed-on: https://go-review.googlesource.com/c/go/+/345769
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Previously, when -fuzztime was given a number of executions like
-fuzztime=100x, this was a count for each minimization independent of
-fuzztime. Since there is no bound on the number of minimizations,
this was not a meaningful limit.
With this change, executions of the fuzz function during minimization
count toward the -fuzztime global limit. Executions are further
limited by -fuzzminimizetime.
This change also counts executions during the coverage-only run and
reports errors for those executions.
There is no change when -fuzztime specifies a duration or when
-fuzztime is not set.
Change-Id: Ibcf1b1982f28b28f6625283aa03ce66d4de0a26d
Reviewed-on: https://go-review.googlesource.com/c/go/+/342994
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
When taking a snapshot of coverage counters, round each counter down
to the nearest power of 2.
After coarsening, at most 1 bit per byte will be set. This lets the
coordinator use a coverage array as a mask that distinguish between
code that's executed many times for a given input and code that's
executed once or a few times. For example, if a byte in this array has
the value 12, it means the block has been executed at least 4 times
and at least 8 times with different inputs.
Also change the term "edge" to "bits" or just be more vague about how
coverage is represented.
Also add more code that may be "interesting" in test_fuzz_cache.
Change-Id: I67bf2adb298fb8efd7680b069a476c27e5fdbdae
Reviewed-on: https://go-review.googlesource.com/c/go/+/338829
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Previously, before each call to the fuzz function, the worker process
marshalled the mutated input into shared memory. If the worker process
terminates unexpectedly, it's important that the coordinator can find
the crashing input in shared memory.
Profiling shows this marshalling is very expensive though. This change
takes another strategy. Instead of marshaling each mutated input, the
worker process no longer modifies the input in shared memory at
all. Instead, it saves its PRNG state in shared memory and increments
a counter before each fuzz function call. If the worker process
terminates, the coordinator can reconstruct the crashing value using
this information.
This change gives a ~10x increase in execs/s for a trivial fuzz
function with -parallel=1.
Change-Id: I18cf326c252727385dc53ea2518922b1f6ae36b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/334149
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
* Benchmark{Marshal,Unmarshal}CorpusFile - measures time it takes to
serialize and deserialize byte slices of various lengths.
* BenchmarkWorkerPing - spins up a worker and measures time it takes
to ping it N times as a rough measure of RPC latency.
* BenchmarkWorkerFuzz - spins up a worker and measures time it takes
to mutate an input and call a trivial fuzz function N times.
Also a few small fixes to make this easier.
Change-Id: Id7f2dc6c6c05005cf286f30e6cc92a54bf44fbf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/333670
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
* The exec count must be set to 0 before calling
workerServer.fuzz. This was causing fuzz to run indefinitely after
the first benchmark iteration, since it wouldn't hit the termination
condition of being equal to fuzzArgs.Limit.
* Added an assertion that the count must be lower than fuzzArgs.Limit
at the beginning of workerServer.fuzz.
* Also closed and deleted shared memory at the end of each benchmark run.
Change-Id: Iab465f8a4997ebd652aec04d0ff9bb60b802829e
Reviewed-on: https://go-review.googlesource.com/c/go/+/334129
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
With this change, we'll no longer silently ignore terminations by
SIGKILL. We use SIGKILL to terminate unresponsive workers, but it can
also be delivered by the OOM killer.
When a worker is terminated by a signal not apparently due to a crash
or interruption (like SIGKILL or SIGHUP, as opposed to SIGSEGV), we'll
log a message, but we won't record a crash, since any given input is
not likely to reproduce this termination.
Fixesgolang/go#46576
Change-Id: I6ef18a7cf5a457c7b9bc44cf5416378271216bfd
Reviewed-on: https://go-review.googlesource.com/c/go/+/333190
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
We want worker RPCs to return as soon as the context is cancelled,
which happens if the user presses ^C, we hit the time limit, or
another worker discovers a crasher. RPCs typically block when reading
pipes: the server waits for call arguments from the client, and the
client waits for results from the server.
Since io.Reader.Read doesn't accept a context.Context and reads on
pipe file descriptors are difficult to reliably unblock, we've done
this by calling Read in a goroutine, and returning from the parent
function when ctx.Done() is closed, even if the underlying goroutine
isn't finished.
In workerServer.serve, we also called the fuzz function in the same
goroutine. This resulted in a bug: serve could return while the fuzz
function was still running. The fuzz function could observe side
effects from cleanup functions registered with F.Cleanup.
This change refactors read cancellation logic into contextReader. Only
the underlying Read is done in a goroutine. workerServe.serve won't
return while the fuzz function is running.
Fixes#46632
Change-Id: Id1ed31f6521155c7c8e76dd52a2d70aa93cab201
Reviewed-on: https://go-review.googlesource.com/c/go/+/329920
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
When GODEBUG=fuzzdebug=1, log additional debug level information about
what the fuzzer is doing. This provides useful information for
investigating the operation and performance of the fuzzing engine, and
is necessary for profiling new fuzzing strategies.
Change-Id: Ic3e24e7a128781377e62785767a218811c3c2030
Reviewed-on: https://go-review.googlesource.com/c/go/+/324972
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
When a worker process finds a crasher, it now sends that result
directly to the coordinator without attempting to minimize it
first. The coordinator stops sending new inputs and sends the
unminimized crasher back to a worker (any worker) for minimization.
This prevents wasted work during minimization and will help us
implement -keepfuzzing later on. We may also be able to minimize
interesting inputs with this approach later.
Since panics are recoverable errors (they don't terminate worker
processes), we no longer attempt to minimize non-recoverable errors.
This didn't work too well before: we lost too much state.
Change-Id: Id142c7e91a33f64584170b0d42d22cb1f22a92d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/321835
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Adds support for minimizing strings using the same logic as byte slices
as well as minimizing both signed and unsigned integers and floats using
extremely basic logic. A more complex approach is probably warranted in
the future, but for now this should be _good enough_.
Change-Id: Ibc6c3d6ae82685998f571aa2c1ecea2f85c2708b
Reviewed-on: https://go-review.googlesource.com/c/go/+/320669
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
And only log the last panic, not all of them, during minimization.
This change makes the worker processes quiet, so now the only
process that logs anything is the coordinator. This hides all of
the panics caused during minimization of an input which causes
a panic.
This change also alters the usage of tRunner such that we now
recover from recoverable panics instead of terminating the
process. This results in larger stack traces, since we include
a bit more of the trace within testing. There is a TODO to see
if it's possible to slice the stack up so that it is somewhat
more informative.
Change-Id: Ic85eabd2e70b078412fbb88adf424a8da25af876
Reviewed-on: https://go-review.googlesource.com/c/go/+/321230
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
* Introduced -fuzzminimizetime flag to control the number of time or
the number of calls to spend minimizing. Defaults to 60s. Only works
for unrecoverable crashes for now.
* Moved the count (used by -fuzztime=1000x) into shared
memory. Calling workerClient.fuzz resets it, but it will remain
after the worker processes crashes. workerClient.minimize resets it
once before restarting the worker the first time, but the total
number of runs should still be limited during minimization, even
after multiple terminations and restarts.
* Renamed fuzzArgs.Count to Limit to avoid confusion.
* Several other small fixes and refactorings.
Change-Id: I03faa4c94405041f6dfe48568e5ead502f8dbbd2
Reviewed-on: https://go-review.googlesource.com/c/go/+/320171
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
When instrumented packages intersect with the packages used by the
testing or internal/fuzz packages the coverage counters become noisier,
as counters will be triggered by non-fuzzed harness code.
Ideally counters would be deterministic, as there are many advanced
fuzzing strategies that require mutating the input while maintaining
static coverage.
The simplest way to mitigate this noise is to capture the coverage
counters as closely as possible to the invocation of the fuzz target
in the testing package. In order to do this add a new function which
captures the current values of the counters, SnapshotCoverage. This
function copies the current counters into a static buffer,
coverageSnapshot, which workerServer.fuzz can then inspect when it
comes time to check if new coverage has been found.
This method is not foolproof. As the fuzz target is called in a
goroutine, harness code can still cause counters to be incremented
while the target is being executed. Despite this we do see
significant reduction in churn via this approach. For example,
running a basic target that causes strconv to be instrumented for
500,000 iterations causes ~800 unique sets of coverage counters,
whereas by capturing the counters closer to the target we get ~40
unique sets.
It may be possible to make counters completely deterministic, but
likely this would require rewriting testing/F.Fuzz to not use tRunner
in a goroutine, and instead use it in a blocking manner (which I
couldn't figure out an obvious way to do), or by doing something even
more complex.
Change-Id: I95c2f3b1d7089c3e6885fc7628a0d3a8ac1a99cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/320329
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
This change updates the go command behavior when
fuzzing to instrument the binary for code coverage,
and uses this coverage in the fuzzing engine to
determine if an input is interesting.
Unfortunately, we can't store and use the coverage
data for a given run of `go test` and re-use it
the next time we fuzz, since the edges could have
changed between builds. Instead, every entry in
the seed corpus and the on-disk corpus is run
by the workers before fuzzing begins, so that the
coordinator can get the baseline coverage for what
the fuzzing engine has already found (or what
the developers have already provided).
Users should run `go clean -fuzzcache` before
using this change, to clear out any existing
"interesting" values that were in the cache.
Previously, every single non-crashing input was
written to the on-disk corpus. Now, only inputs
that actually expand coverage are written.
This change includes a small hack in
cmd/go/internal/load/pkg.go which ensures that the Gcflags
that were explicitly set in cmd/go/internal/test/test.go
don't get cleared out.
Tests will be added in a follow-up change, since
they will be a bit more involved.
Change-Id: Ie659222d44475c6d68fa4a35d37c37cab3619d71
Reviewed-on: https://go-review.googlesource.com/c/go/+/312009
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Assuming that this works for non-recoverable errors, there
will likely be a follow-up CL which refactors the minimization
for recoverable errors to use the same RPC flow (since that
more easily allows the worker to tell the coordinator that
it's minimizing and shouldn't send more inputs to other workers
to fuzz).
Change-Id: I32ac7cec4abe2d4c345c0ee77315233047efb1fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/309509
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This improves readability a bit, and it should help with compatibility
for future clients when arguments are added or reordered.
Unfortunately, testing still can't import internal/fuzz, so the
interface there can't use this type.
Change-Id: I4cda2347884defcbbfc2bd01ab5b4a901d91549c
Reviewed-on: https://go-review.googlesource.com/c/go/+/308192
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
While running the seed corpus, T.Parallel acts like it does in
subtests started with T.Run: it blocks until all other non-parallel
subtests have finished, then unblocks when the barrier chan is
closed. A semaphore (t.context.waitParallel) limits the number of
tests that run concurrently (determined by -test.parallel).
While fuzzing, T.Parallel has no effect, other than asserting that it
can't be called multiple times. We already run different inputs in
concurrent processes, but we can't run inputs concurrently in the same
process if we want to attribute crashes to specific inputs.
Change-Id: I2bac08e647e1d92ea410c83c3f3558a033fe3dd1
Reviewed-on: https://go-review.googlesource.com/c/go/+/300449
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
-fuzztime now works similarly to -benchtime: if it's given a string
with an "x" suffix (as opposed to "s" or some other unit of
duration), the fuzzing system will generate and run a maximum number
of values.
This CL also implements tracking and printing counts, since most of
the work was already done.
Change-Id: I013007984b5adfc1a751c379dc98c8d46b4a97e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/306909
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
worker.runFuzzing now accepts a Context, used for cancellation instead
of doneC (which is removed). This is passed down through workerClient
RPC methods (ping, fuzz).
workerClient RPC methods now wrap the call method, which handles
marshaling and cancellation.
Both workerClient.call and workerServer.serve should return quickly
when their contexts are cancelled. Turns out, closing the pipe won't
actually unblock a read on all platforms. Instead, we were falling
back to SIGKILL in worker.stop, which works but takes longer than
necessary.
Also fixed missing newline in log message.
Change-Id: I7b5ae54d6eb9afd6361a07759f049f048952e0cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/303429
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
This fixes a few issues that were being masked since
log statements weren't being printed to stdout. Now
that they are, fix the bugs, and update the tests.
Also includes a few small refactors which will make
minimizing non-recoverable errors easier.
Change-Id: Ie2fd2e5534b3980317e1e1f3fd8e04750988c17f
Reviewed-on: https://go-review.googlesource.com/c/go/+/307810
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
When mutating a byte slice, mutate in place, and only allocate once if
the slice's capacity is less than the maximum size.
mutateBytes already should not allocate; we check a post-condition
that the slice's data pointer does not change.
This speeds up the mutator from 4 ms per value to 200-600 ns. For
example:
goos: darwin
goarch: amd64
pkg: internal/fuzz
cpu: Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
BenchmarkMutatorBytes/1-8 5908735 275.3 ns/op
BenchmarkMutatorBytes/10-8 5198473 282.0 ns/op
BenchmarkMutatorBytes/100-8 4304750 233.9 ns/op
BenchmarkMutatorBytes/1000-8 4623988 295.2 ns/op
BenchmarkMutatorBytes/10000-8 4252104 458.5 ns/op
BenchmarkMutatorBytes/100000-8 1236751 950.8 ns/op
PASS
ok internal/fuzz 12.993s
Change-Id: I4bf2a04be6c648ef440af2c62bf0ffa3d310172c
Reviewed-on: https://go-review.googlesource.com/c/go/+/306675
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
Also improve the error messages for the use of
testing.F functions inside the Fuzz function.
Change-Id: I5fa48f8c7e0460a1da89a49a73e5af83c544e549
Reviewed-on: https://go-review.googlesource.com/c/go/+/298849
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This works by minimizing for a maximum of one minute. We may consider
making this customizable in the future.
This only minimizes []byte inputs which caused a recoverable error. In
the future, it should support minimizing other appopriate types, and
minimizing types which caused non-recoverable errors (though this is
much more expensive).
The code in internal/fuzz/worker.go is copied from, or heavily inspired
by, code originally authored by Dmitry Vyukov and Josh Bleecher Snyder
as part of the go-fuzz project. Thanks to them for their contributions.
See https://github.com/dvyukov/go-fuzz.
Change-Id: I93dbac7ff874d6d0c1b9b9dda23930ae9921480c
Reviewed-on: https://go-review.googlesource.com/c/go/+/298909
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
CoordinateFuzzing now continues to run after discovering a crasher. It
waits until all workers have terminated before returning.
This fixes a deadlock that occurred when multiple workers discovered
crashers concurrently. CoordinateFuzzing would receive one crasher,
close doneC (telling workers to stop), then wait for workers to stop
without receiving more crashers. Other workers would block sending
crashers.
Change-Id: I55a64aac0e6e43f5e36b9d03c15051c3d5debb20
Reviewed-on: https://go-review.googlesource.com/c/go/+/293369
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
* Appending to the worker environment slice should reallocate it. On
Windows, we pass handles through the environment, and concurrent
workers were writing to the same memory, resulting in
"The handle is invalid" errors.
* Instead of passing a handle to the temporary file, we pass its path
to each worker instead. The worker is responsible for opening and
closing the handle. Previously, all inheritable handles were
inherited by all workers, even though only one was used. This
prevented temporary files from being deleted after a worker stopped,
because other workers would still have open handles to it.
Change-Id: If8b8bcfa5b03fbcadd10ef923b036bb0ee5dc3f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/297034
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
If a worker process encounters an error communicating with the
coordinator, or if the setup code reports an error with F.Fail
before calling F.Fuzz, exit with status 70. The coordinator will report
these errors and 'go test' will exit non-zero, but the coordinator
won't record a crasher since the problem is not in the code being
fuzzed.
The coordinator also detects unexpected terminations before the worker
calls F.Fuzz by sending a ping RPC. If the worker terminates before
responding to the ping RPC, the coordinator won't record a crasher.
Exit codes are chosen somewhat arbitrary, but in the Advanced Bash
Scripting Guide, 70 is "internal software error" which is applicable
here. 70 is also ASCII 'F'.
Change-Id: I1e676e39a7b07c5664efaaa3221d055f55240fff
Reviewed-on: https://go-review.googlesource.com/c/go/+/297033
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Previously, the coordintor used the error string encoded by the worker
to determine whether or not a crash occurred. However, failures caused
by things like t.Fail() which have no output will have an empty error
string, so we can't rely on the error string alone to determine if
something is a crasher or not.
Change-Id: Idcf7f6b1210aa1dc4e8dab222642c87919595693
Reviewed-on: https://go-review.googlesource.com/c/go/+/298351
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This change makes several refactors to start supporting
structured fuzzing. The mutator can still only mutate
byte slices, and future changes will be made to support
mutating other types. However, it does now support
fuzzing more than one []byte.
This change also makes it so that corpus entries are
encoded in the new file format when being written to
testdata or GOCACHE. Any existing GOCACHE data should
be deleted from your local workstation to allow tests
to pass locally.
Change-Id: Iab8fe01a5dc870f0c53010b9d5b0b479bbdb310d
Reviewed-on: https://go-review.googlesource.com/c/go/+/293810
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
RunFuzzWorker now accepts a fuzz.CorpusEntry instead of []byte. This
may help us pass structured data in the future.
Change-Id: Idf7754cb890b6a835d887032fd23ade4d0713bcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/290692
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
If a worker process exits with status 0, treat it as a communication
error. Previously, we treated this as a crasher, but it seems more
likely to be caused by a bug in the fuzz function rather than a bug in
the code being tested.
Change-Id: I0c4efeaef85537f8a0e9c6def6aac41d75b2b307
Reviewed-on: https://go-review.googlesource.com/c/go/+/290690
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
CorpusEntry is now a struct type with Name and Data fields. In the
future, it may have more fields describing multiple values with
different types added with f.Add.
CorpusEntry must be the same type in testing and
internal/fuzz. However, we don't want to export it from testing, and
testing can't import internal/fuzz. We define it to be a type alias of
a struct type instead of a defined type. We need to define it to the
same thing in both places. We'll get a type error when building cmd/go
if there's a difference.
Change-Id: I9df6cd7aed67a6aa48b77ffb3a84bd302d2e5d94
Reviewed-on: https://go-review.googlesource.com/c/go/+/288534
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
This change moves the worker's *sharedMem into a buffered chan that
acts as a mutex. The mutex can be locked by receiving from the chan;
it can be unlocked by sending *sharedMem back to the chan. Multiple
objects (like worker, workerClient, workerServer) may have references
to the chan and may hold the lock across several operations.
This is intended to fix a segfault that occurred when
workerClient.fuzz accessed shared memory after it was already closed
and unmapped by the worker's goroutine. workerClient.fuzz is executed
in a separate goroutine so the worker can still receive messages from
the coordinator (like being told to stop and clean up).
Change-Id: I4eb9079ba9e5bfcfacfecd0fc8ad9bed17b33bba
Reviewed-on: https://go-review.googlesource.com/c/go/+/285054
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
A worker process may be terminated by SIGINT if it doesn't install the
signal handler before SIGINT is delivered. That's likely when TestMain
or the fuzz target setup take a long time. The coordinator now ignores
these errors.
Also, when testdeps.TestDeps.CoordinateFuzzing and RunFuzzWorker
return, they will send a value on the chan passed to signal.Notify
instead of closing it. This should have been obvious in hindsight, but
the signal handler could still send a value on that channel after
those functions return but before the process exits.
Change-Id: Iea2589115f1f9bb7415bb5e7911defee423e642e
Reviewed-on: https://go-review.googlesource.com/c/go/+/284292
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
