[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>

src/internal/fuzz/fuzz.go

@@ -96,7 +96,7 @@ func CoordinateFuzzing(ctx context.Context, parallel int, seed []CorpusEntry, ty
 			dir:         dir,
 			binPath:     binPath,
 			args:        args,
-			env:         env,
+			env:         env[:len(env):len(env)], // copy on append to ensure workers don't overwrite each other.
 			coordinator: c,
 			memMu:       memMu,
 		}, nil

src/internal/fuzz/sys_windows.go

@@ -9,8 +9,6 @@ import (
 	"os"
 	"os/exec"
 	"reflect"
-	"strconv"
-	"strings"
 	"syscall"
 	"unsafe"
 )
@@ -19,7 +17,13 @@ type sharedMemSys struct {
 	mapObj syscall.Handle
 }
 
-func sharedMemMapFile(f *os.File, size int, removeOnClose bool) (*sharedMem, error) {
+func sharedMemMapFile(f *os.File, size int, removeOnClose bool) (mem *sharedMem, err error) {
+	defer func() {
+		if err != nil {
+			err = fmt.Errorf("mapping temporary file %s: %w", f.Name(), err)
+		}
+	}()
+
 	// Create a file mapping object. The object itself is not shared.
 	mapObj, err := syscall.CreateFileMapping(
 		syscall.Handle(f.Fd()), // fhandle
@@ -86,12 +90,11 @@ func (m *sharedMem) Close() error {
 // run a worker process.
 func setWorkerComm(cmd *exec.Cmd, comm workerComm) {
 	mem := <-comm.memMu
-	memFD := mem.f.Fd()
+	memName := mem.f.Name()
 	comm.memMu <- mem
 	syscall.SetHandleInformation(syscall.Handle(comm.fuzzIn.Fd()), syscall.HANDLE_FLAG_INHERIT, 1)
 	syscall.SetHandleInformation(syscall.Handle(comm.fuzzOut.Fd()), syscall.HANDLE_FLAG_INHERIT, 1)
-	syscall.SetHandleInformation(syscall.Handle(memFD), syscall.HANDLE_FLAG_INHERIT, 1)
-	cmd.Env = append(cmd.Env, fmt.Sprintf("GO_TEST_FUZZ_WORKER_HANDLES=%x,%x,%x", comm.fuzzIn.Fd(), comm.fuzzOut.Fd(), memFD))
+	cmd.Env = append(cmd.Env, fmt.Sprintf("GO_TEST_FUZZ_WORKER_HANDLES=%x,%x,%q", comm.fuzzIn.Fd(), comm.fuzzOut.Fd(), memName))
 }
 
 // getWorkerComm returns communication channels in the worker process.
@@ -100,27 +103,21 @@ func getWorkerComm() (comm workerComm, err error) {
 	if v == "" {
 		return workerComm{}, fmt.Errorf("GO_TEST_FUZZ_WORKER_HANDLES not set")
 	}
-	parts := strings.Split(v, ",")
-	if len(parts) != 3 {
-		return workerComm{}, fmt.Errorf("GO_TEST_FUZZ_WORKER_HANDLES has invalid value")
-	}
-	base := 16
-	bitSize := 64
-	handles := make([]syscall.Handle, len(parts))
-	for i, s := range parts {
-		h, err := strconv.ParseInt(s, base, bitSize)
-		if err != nil {
-			return workerComm{}, fmt.Errorf("GO_TEST_FUZZ_WORKER_HANDLES has invalid value: %v", err)
-		}
-		handles[i] = syscall.Handle(h)
+	var fuzzInFD, fuzzOutFD uintptr
+	var memName string
+	if _, err := fmt.Sscanf(v, "%x,%x,%q", &fuzzInFD, &fuzzOutFD, &memName); err != nil {
+		return workerComm{}, fmt.Errorf("parsing GO_TEST_FUZZ_WORKER_HANDLES=%s: %v", v, err)
 	}
 
-	fuzzIn := os.NewFile(uintptr(handles[0]), "fuzz_in")
-	fuzzOut := os.NewFile(uintptr(handles[1]), "fuzz_out")
-	tmpFile := os.NewFile(uintptr(handles[2]), "fuzz_mem")
+	fuzzIn := os.NewFile(fuzzInFD, "fuzz_in")
+	fuzzOut := os.NewFile(fuzzOutFD, "fuzz_out")
+	tmpFile, err := os.OpenFile(memName, os.O_RDWR, 0)
+	if err != nil {
+		return workerComm{}, fmt.Errorf("worker opening temp file: %w", err)
+	}
 	fi, err := tmpFile.Stat()
 	if err != nil {
-		return workerComm{}, err
+		return workerComm{}, fmt.Errorf("worker checking temp file size: %w", err)
 	}
 	size := int(fi.Size())
 	if int64(size) != fi.Size() {

src/internal/fuzz/worker.go

@@ -219,7 +219,7 @@ func (w *worker) start() (err error) {
 
 	cmd := exec.Command(w.binPath, w.args...)
 	cmd.Dir = w.dir
-	cmd.Env = w.env
+	cmd.Env = w.env[:len(w.env):len(w.env)] // copy on append to ensure workers don't overwrite each other.
 	// TODO(jayconrod): set stdout and stderr to nil or buffer. A large number
 	// of workers may be very noisy, but for now, this output is useful for
 	// debugging.