cmd/fix: disallow cgo errors in tests

The 'cgo' command invoked by 'go fix' was not valid when built with
-trimpath, but the test was not failing because errors from the
command were being logged and ignored instead of causing tests to
fail. Changing the code and test not to ignore the errors revealed
that a number of existing tests were always, unconditionally
triggering cgo errors which were then ignored.

This change updates those tests to no longer produce cgo errors,
and to check their results when cgo is enabled.

For #51473.
Updates #51461.

Change-Id: Ib9d1ea93f26d30daa824d75ed634eaf530af086d
Reviewed-on: https://go-review.googlesource.com/c/go/+/450714
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

5 files changed, 100 insertions, 3 deletions

diff --git a/src/cmd/fix/cftype_test.go b/src/cmd/fix/cftype_test.go
index a18eb25261..cde47f28a3 100644
--- a/src/cmd/fix/cftype_test.go
+++ b/src/cmd/fix/cftype_test.go
@@ -13,6 +13,7 @@ var cftypeTests = []testCase{
 		Name: "cftype.localVariable",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 func f() {
@@ -23,6 +24,7 @@ func f() {
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 func f() {
@@ -36,6 +38,7 @@ func f() {
 		Name: "cftype.globalVariable",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef = nil
@@ -46,6 +49,7 @@ func f() {
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef = 0
@@ -59,6 +63,7 @@ func f() {
 		Name: "cftype.EqualArgument",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef
@@ -67,6 +72,7 @@ var z = x != nil
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef
@@ -78,6 +84,7 @@ var z = x != 0
 		Name: "cftype.StructField",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 type T struct {
@@ -88,6 +95,7 @@ var t = T{x: nil}
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 type T struct {
@@ -101,6 +109,7 @@ var t = T{x: 0}
 		Name: "cftype.FunctionArgument",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 func f(x C.CFTypeRef) {
@@ -112,6 +121,7 @@ func g() {
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 func f(x C.CFTypeRef) {
@@ -126,12 +136,14 @@ func g() {
 		Name: "cftype.ArrayElement",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = [3]C.CFTypeRef{nil, nil, nil}
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = [3]C.CFTypeRef{0, 0, 0}
@@ -141,12 +153,14 @@ var x = [3]C.CFTypeRef{0, 0, 0}
 		Name: "cftype.SliceElement",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = []C.CFTypeRef{nil, nil, nil}
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = []C.CFTypeRef{0, 0, 0}
@@ -156,12 +170,14 @@ var x = []C.CFTypeRef{0, 0, 0}
 		Name: "cftype.MapKey",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = map[C.CFTypeRef]int{nil: 0}
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = map[C.CFTypeRef]int{0: 0}
@@ -171,12 +187,14 @@ var x = map[C.CFTypeRef]int{0: 0}
 		Name: "cftype.MapValue",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = map[int]C.CFTypeRef{0: nil}
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x = map[int]C.CFTypeRef{0: 0}
@@ -186,6 +204,7 @@ var x = map[int]C.CFTypeRef{0: 0}
 		Name: "cftype.Conversion1",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef
@@ -193,6 +212,7 @@ var y = (*unsafe.Pointer)(&x)
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x C.CFTypeRef
@@ -203,6 +223,7 @@ var y = (*unsafe.Pointer)(unsafe.Pointer(&x))
 		Name: "cftype.Conversion2",
 		In: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x unsafe.Pointer
@@ -210,6 +231,7 @@ var y = (*C.CFTypeRef)(&x)
 `,
 		Out: `package main
 
+// typedef const void *CFTypeRef;
 import "C"
 
 var x unsafe.Pointer
diff --git a/src/cmd/fix/egltype_test.go b/src/cmd/fix/egltype_test.go
index 9b64a7c20b..c44525c053 100644
--- a/src/cmd/fix/egltype_test.go
+++ b/src/cmd/fix/egltype_test.go
@@ -17,6 +17,7 @@ func eglTestsFor(tname string) []testCase {
 			Name: "egl.localVariable",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 func f() {
@@ -27,6 +28,7 @@ func f() {
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 func f() {
@@ -40,6 +42,7 @@ func f() {
 			Name: "egl.globalVariable",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x C.$EGLTYPE = nil
@@ -50,6 +53,7 @@ func f() {
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x C.$EGLTYPE = 0
@@ -63,6 +67,7 @@ func f() {
 			Name: "egl.EqualArgument",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x C.$EGLTYPE
@@ -71,6 +76,7 @@ var z = x != nil
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x C.$EGLTYPE
@@ -82,6 +88,7 @@ var z = x != 0
 			Name: "egl.StructField",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 type T struct {
@@ -92,6 +99,7 @@ var t = T{x: nil}
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 type T struct {
@@ -105,6 +113,7 @@ var t = T{x: 0}
 			Name: "egl.FunctionArgument",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 func f(x C.$EGLTYPE) {
@@ -116,6 +125,7 @@ func g() {
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 func f(x C.$EGLTYPE) {
@@ -130,12 +140,14 @@ func g() {
 			Name: "egl.ArrayElement",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = [3]C.$EGLTYPE{nil, nil, nil}
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = [3]C.$EGLTYPE{0, 0, 0}
@@ -145,12 +157,14 @@ var x = [3]C.$EGLTYPE{0, 0, 0}
 			Name: "egl.SliceElement",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = []C.$EGLTYPE{nil, nil, nil}
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = []C.$EGLTYPE{0, 0, 0}
@@ -160,12 +174,14 @@ var x = []C.$EGLTYPE{0, 0, 0}
 			Name: "egl.MapKey",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = map[C.$EGLTYPE]int{nil: 0}
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = map[C.$EGLTYPE]int{0: 0}
@@ -175,12 +191,14 @@ var x = map[C.$EGLTYPE]int{0: 0}
 			Name: "egl.MapValue",
 			In: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = map[int]C.$EGLTYPE{0: nil}
 `,
 			Out: `package main
 
+// typedef void *$EGLTYPE;
 import "C"
 
 var x = map[int]C.$EGLTYPE{0: 0}
diff --git a/src/cmd/fix/jnitype_test.go b/src/cmd/fix/jnitype_test.go
index a6420f7b11..ecf01408c7 100644
--- a/src/cmd/fix/jnitype_test.go
+++ b/src/cmd/fix/jnitype_test.go
@@ -13,6 +13,7 @@ var jniTests = []testCase{
 		Name: "jni.localVariable",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 func f() {
@@ -23,6 +24,7 @@ func f() {
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 func f() {
@@ -36,6 +38,7 @@ func f() {
 		Name: "jni.globalVariable",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x C.jobject = nil
@@ -46,6 +49,7 @@ func f() {
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x C.jobject = 0
@@ -59,6 +63,7 @@ func f() {
 		Name: "jni.EqualArgument",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x C.jobject
@@ -67,6 +72,7 @@ var z = x != nil
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x C.jobject
@@ -78,6 +84,7 @@ var z = x != 0
 		Name: "jni.StructField",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 type T struct {
@@ -88,6 +95,7 @@ var t = T{x: nil}
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 type T struct {
@@ -101,6 +109,7 @@ var t = T{x: 0}
 		Name: "jni.FunctionArgument",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 func f(x C.jobject) {
@@ -112,6 +121,7 @@ func g() {
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 func f(x C.jobject) {
@@ -126,12 +136,14 @@ func g() {
 		Name: "jni.ArrayElement",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = [3]C.jobject{nil, nil, nil}
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = [3]C.jobject{0, 0, 0}
@@ -141,12 +153,14 @@ var x = [3]C.jobject{0, 0, 0}
 		Name: "jni.SliceElement",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = []C.jobject{nil, nil, nil}
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = []C.jobject{0, 0, 0}
@@ -156,12 +170,14 @@ var x = []C.jobject{0, 0, 0}
 		Name: "jni.MapKey",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = map[C.jobject]int{nil: 0}
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = map[C.jobject]int{0: 0}
@@ -171,12 +187,14 @@ var x = map[C.jobject]int{0: 0}
 		Name: "jni.MapValue",
 		In: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = map[int]C.jobject{0: nil}
 `,
 		Out: `package main
 
+// typedef struct _jobject* jobject;
 import "C"
 
 var x = map[int]C.jobject{0: 0}
diff --git a/src/cmd/fix/main_test.go b/src/cmd/fix/main_test.go
index 755007bc0d..837a5d72c7 100644
--- a/src/cmd/fix/main_test.go
+++ b/src/cmd/fix/main_test.go
@@ -5,13 +5,29 @@
 package main
 
 import (
+	"fmt"
 	"go/ast"
 	"go/parser"
 	"internal/diff"
+	"internal/testenv"
 	"strings"
 	"testing"
 )
 
+func init() {
+	// If cgo is enabled, enforce that cgo commands invoked by cmd/fix
+	// do not fail during testing.
+	if testenv.HasCGO() {
+		// The reportCgoError hook is global, so we can't set it per-test
+		// if we want to be able to run those tests in parallel.
+		// Instead, simply set it to panic on error: the goroutine dump
+		// from the panic should help us determine which test failed.
+		reportCgoError = func(err error) {
+			panic(fmt.Sprintf("unexpected cgo error: %v", err))
+		}
+	}
+}
+
 type testCase struct {
 	Name    string
 	Fn      func(*ast.File) bool
@@ -79,7 +95,13 @@ func TestRewrite(t *testing.T) {
 		tt := tt
 		t.Run(tt.Name, func(t *testing.T) {
 			if tt.Version == 0 {
-				t.Parallel()
+				if testing.Verbose() {
+					// Don't run in parallel: cmd/fix sometimes writes directly to stderr,
+					// and since -v prints which test is currently running we want that
+					// information to accurately correlate with the stderr output.
+				} else {
+					t.Parallel()
+				}
 			} else {
 				old := goVersion
 				goVersion = tt.Version
diff --git a/src/cmd/fix/typecheck.go b/src/cmd/fix/typecheck.go
index 27042e05a5..b115987390 100644
--- a/src/cmd/fix/typecheck.go
+++ b/src/cmd/fix/typecheck.go
@@ -170,7 +170,16 @@ func typecheck(cfg *TypeConfig, f *ast.File) (typeof map[any]string, assign map[
 			if err != nil {
 				return err
 			}
-			cmd := exec.Command(filepath.Join(runtime.GOROOT(), "bin", "go"), "tool", "cgo", "-objdir", dir, "-srcdir", dir, "in.go")
+			goCmd := "go"
+			if goroot := runtime.GOROOT(); goroot != "" {
+				goCmd = filepath.Join(goroot, "bin", "go")
+			}
+			cmd := exec.Command(goCmd, "tool", "cgo", "-objdir", dir, "-srcdir", dir, "in.go")
+			if reportCgoError != nil {
+				// Since cgo command errors will be reported, also forward the error
+				// output from the command for debugging.
+				cmd.Stderr = os.Stderr
+			}
 			err = cmd.Run()
 			if err != nil {
 				return err
@@ -206,7 +215,11 @@ func typecheck(cfg *TypeConfig, f *ast.File) (typeof map[any]string, assign map[
 			return nil
 		}()
 		if err != nil {
-			fmt.Fprintf(os.Stderr, "go fix: warning: no cgo types: %s\n", err)
+			if reportCgoError == nil {
+				fmt.Fprintf(os.Stderr, "go fix: warning: no cgo types: %s\n", err)
+			} else {
+				reportCgoError(err)
+			}
 		}
 	}
 
@@ -285,6 +298,10 @@ func typecheck(cfg *TypeConfig, f *ast.File) (typeof map[any]string, assign map[
 	return typeof, assign
 }
 
+// reportCgoError, if non-nil, reports a non-nil error from running the "cgo"
+// tool. (Set to a non-nil hook during testing if cgo is expected to work.)
+var reportCgoError func(err error)
+
 func makeExprList(a []*ast.Ident) []ast.Expr {
 	var b []ast.Expr
 	for _, x := range a {