Commit Graph

25 Commits

Author SHA1 Message Date
Katie Hockman 6ffb027483 [dev.fuzz] internal/fuzz: use coverage instrumentation while fuzzing
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>
2021-05-11 21:19:22 +00:00
Roland Shoemaker 510e711dd3 [dev.fuzz] testing,internal/fuzz: prevent unbounded memory growth
Usage of f.testContext.match.fullName to generate the test name causes
unbounded memory growth, eventually causing the fuzzer to slow down
as memory pressure increases.

Each time fuzzFn is invoked it generates a unique string and stores it
in a map. With the fuzzer running at around 100k executions per second
this consumed around ~30GB of memory in a handful of minutes.

Instead just use the base name of the test for mutated inputs, a special
name for seeded inputs, and the filename for inputs from the input
corpus.

Change-Id: I083f47df7e82f0c6b0bda244f158233784a13029
Reviewed-on: https://go-review.googlesource.com/c/go/+/316030
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>
2021-05-07 14:24:10 +00:00
Katie Hockman af3237eaf9 [dev.fuzz] internal/fuzz: don't panic if types change
There was a bug where if the types to fuzz were
different from the types in a file in the on-disk
corpus, then the code would panic. We thought
this case was handled, but the final `continue`
in the nested loop still allowed the invalid
entry to be added to the corpus. Pulling the
validation into a helper function makes this
less brittle.

Change-Id: I401346f890ea30ab7cff9640cb555da2e3ff8cc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/313810
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-05-04 18:57:18 +00:00
Katie Hockman 7a7eee8ab7 [dev.fuzz] internal/fuzz: minimize non-recoverable errors
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>
2021-04-16 20:50:55 +00:00
Jay Conrod 88c0c2d9ea [dev.fuzz] internal/fuzz: move CoordinateFuzzing args into struct type
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>
2021-04-09 19:53:06 +00:00
Jay Conrod 4baa39ca22 [dev.fuzz] testing: let -fuzztime specify a number of executions
-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>
2021-04-09 19:51:56 +00:00
Jay Conrod 4cde035a72 [dev.fuzz] internal/fuzz: improve cancellation in worker event loops
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>
2021-04-09 19:50:15 +00:00
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