Commit Graph

18 Commits

Author SHA1 Message Date
Jay Conrod b178a81e1f [dev.fuzz] internal/fuzz: don't count time spent loading corpus
The -fuzztime flag tells us how much time to spend fuzzing, not
counting time spent running the seed corpus. We shouldn't count time
spent loading the cache either. If the cache is large, the time limit
may be exceeded before the coordinator starts the workers.

Change-Id: If00435faa5d24aabdb9003ebb9337fa2e47f22b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/307310
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2021-04-09 17:53:57 +00:00
Jay Conrod b975d0baa0 [dev.fuzz] internal/fuzz: reduce allocation in the mutator
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>
2021-04-05 17:04:08 +00:00
Jay Conrod 6ee1506769 [dev.fuzz] internal/fuzz: fix deadlock with multiple workers
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>
2021-03-09 18:38:07 +00:00
Jay Conrod d4825819fe [dev.fuzz] internal/fuzz: fix two bugs affecting windows
* 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>
2021-03-09 18:36:54 +00:00
Katie Hockman 621a81aba0 [dev.fuzz] testing,internal/fuzz: support structured inputs
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>
2021-02-23 21:10:06 +00:00
Katie Hockman 1c162b41c5 [dev.fuzz] internal/fuzz: remove duplicate read from testdata
We already read the seed corpus from testdata for the
fuzz target, and pass that corpus to the coordinator.
The coordinator doesn't need to read from testdata
again.

Change-Id: Ia7822e3e02b35d56f6918c7082a7b19901b36644
Reviewed-on: https://go-review.googlesource.com/c/go/+/293189
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>
2021-02-18 20:18:03 +00:00
Jay Conrod 7743f60b5a [dev.fuzz] testing: make F.Fuzz more similar to T.Run
This change rewrites much of the glue code in testing/fuzz.go to work
more analogously to T.Run. This results in improved behavior:

* If a fuzz target returns without calling F.Skip, F.Fail, or F.Fuzz,
  'go test' will report an error and exit non-zero.
* Functions registered with F.Cleanup are called.
* The user can re-run individual inputs using -run=FuzzTarget/name
  where name is the base name of the seed corpus file. We now print
  the 'go test' command after a crash.

This change doesn't correctly handle T.Parallel calls yet, but it
should be easier to do that in the future.

Highlighted parts of this change:

* Instead of creating one F for all targets, create an F for each
  target. F (actually common) holds the status, output, and cleanup
  function list for each target, so it's important to keep them
  separate.
* Run each target in its own goroutine via fRunner. fRunner is
  analogous to tRunner. It runs cleanups and catches inappropriate
  Goexits and panics.
* Run each input in its own goroutine via T.Run. This enables subtest
  filtering with -test.run and ensures functions registered with
  T.Cleanup (not F.Cleanup) are run at the appropriate time.

Change-Id: Iab1da14ead8bcb57746f8a76f4aebc625baa5792
Reviewed-on: https://go-review.googlesource.com/c/go/+/290693
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
2021-02-10 18:34:24 +00:00
Jay Conrod 5ef7357b50 [dev.fuzz] internal/fuzz: refactor CorpusEntry type
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>
2021-02-03 19:51:31 +00:00
Jay Conrod 671dba6c89 [dev.fuzz] internal/fuzz: guard concurrent access to shared memory
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>
2021-01-21 15:00:05 +00:00
Katie Hockman 06074108fa [dev.fuzz] internal/fuzz: fix context cancellation for coordinator
Previously, performing a SIGINT would cause the coordinator
to write a crash to testdata, and would continue to run
despite being interupted.

Also includes a few small cleanups.

Change-Id: Ia3cf7cd231c30ac9ad2a61f4935aa543e241f60d
Reviewed-on: https://go-review.googlesource.com/c/go/+/283634
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>
2021-01-14 14:40:52 +00:00
Katie Hockman 2f072cf8a9 [dev.fuzz] internal/fuzz: implement a more robust mutator
This change also allocates a larger capacity (100 MB) for the shared
memory at the start, rather than beginning as small as possible and
immediately needing to grow while mutating. This means that 100 MB is
the maximum size of a corpus entry currently, since growing the shared
memory is not yet supported.

The code in internal/fuzz/mutator.go and internal/fuzz/pcg.go are 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: I0d51d53976e23933072e760ff78e6c4ad9dcd862
Reviewed-on: https://go-review.googlesource.com/c/go/+/281972
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
2021-01-12 16:31:19 +00:00
Jay Conrod a1646595e6 [dev.fuzz] cmd/go: implement -fuzztime flag and support cancellation
fuzz.CoordinateFuzzing and RunFuzzWorker now accept a context.Context
parameter. They should terminate gracefully when the context is
cancelled. The worker should exit quickly without processing more
inputs. The coordinator should save interesting inputs to the cache.

The testing package can't import context directly, so it provides a
timeout argument to testdeps.CoordinateFuzzing instead. The testdeps
wrapper sets the timeout and installs an interrupt handler (for SIGINT
on POSIX and the equivalent on Windows) that cancels the context when
^C is pressed.

Note that on POSIX platforms, pressing ^C causes the shell to deliver
SIGINT to all processes in the active group: so 'go test', the
coordinator, and the workers should all react to that. On Windows,
pressing ^C only interrupts 'go test'. We may want to look at that
separately.

Change-Id: I924d3be2905f9685dae82ff3c047ca3d6b5e2357
Reviewed-on: https://go-review.googlesource.com/c/go/+/279487
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
2020-12-23 16:33:50 +00:00
Jay Conrod 0e21d5be37 [dev.fuzz] internal/fuzz: read and write interesting values in fuzz cache
'go test -fuzz' may now read and write interesting fuzzing values to
directories in $GOCACHE/fuzz. Files in this directory are named
$pkg/$test/$hash where $pkg is the package path containing the fuzz
target, $test is the target name, and $hash is the SHA-256 sum of the
data in the file.

Note that different versions of the same package or packages with the
same path from different modules may share the same directory.

Although files are written into a subdirectory of GOCACHE, they are
not removed automatically, nor are they removed by 'go clean -cache'.
Instead, they may be removed with 'go clean -fuzzcache'. We chose to
nest the fuzzing directory inside GOCACHE to avoid introducing a new
environment variable, since there's no real need for users to specify
a separate directory.

Change-Id: I2032cf8e6c92f715cf36a9fc6a550acf666d2382
Reviewed-on: https://go-review.googlesource.com/c/go/+/275534
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
2020-12-21 21:33:57 +00:00
Jay Conrod 97df3ba792 [dev.fuzz] internal/fuzz: refactor in preparation for GOFUZZCACHE
Several small changes, most related to GOFUZZCACHE.

* Use separate channels to send crashers and interesting values to the
  coordinator.
* Add a new type, crasherEntry, which is a corpusEntry with an
  error message.
* Workers now send fatal errors to the coordinator via errC instead of
  returning or closing doneC.
* In CoordinateFuzzing, defer code that closes doneC and waits for
  workers to stop. This is the only place where doneC is closed.
* In workerServer and workerClient, always pass input values through
  shared memory instead of RPC messages or arguments to avoid
  confusion.
* Rename sharedMem.value to valueRef and add valueCopy to make it
  clearer whether a reference or copy is needed.
* mutate now operates on shared memory directly.
* mutate will not panic on empty input.

Change-Id: I6e57354875508f0ac4483ed2728f3ba18dc938c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/275533
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
2020-12-15 15:34:59 +00:00
Katie Hockman 4651d6b267 [dev.fuzz] internal/fuzzing: handle and report crashers
Change-Id: Ie2a84c12f4991984974162e74f06cfd67e9bb4d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/274855
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2020-12-04 19:17:30 +01:00
Jay Conrod a01814975c [dev.fuzz] internal/fuzz: send inputs to workers with shared memory
The coordinator process creates a temporary file for each worker. Both
coordinator and worker map the file into memory and use it for input
values. Access is synchronized with RPC over pipes.

Change-Id: I43c10d7291a8760a616b472d11c017a3a7bb19cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/263153
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
2020-12-04 19:17:29 +01:00
Katie Hockman 9f3aa113a9 [dev.fuzz] testing: read corpus from testdata/corpus for each target
This change also includes a small cleanup of the run()
function and additional tests for error conditions
in fuzz targets.

Change-Id: I2b7722b25a0d071182a84f1dc4b92e82a7ea34d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/256978
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
2020-12-04 19:17:29 +01:00
Jay Conrod 8fabdcee8f [dev.fuzz] internal/fuzz: coordinate fuzzing across workers
Package fuzz provides common fuzzing functionality for tests built
with "go test" and for programs that use fuzzing functionality in the
testing package.

Change-Id: I3901c6a993a9adb8a93733ae1838b86dd78c7036
Reviewed-on: https://go-review.googlesource.com/c/go/+/259259
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
2020-12-04 19:17:29 +01:00