From: Bryan C. Mills <bcmills@google.com>
Date: Mon, 7 Feb 2022 16:33:30 +0000 (-0500)
Subject: misc/cgo: fix aliasing bugs in parallel tests that append to shared slices
X-Git-Tag: go1.18rc1~87
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=3c4c10ea8c87836a75c4065660c72c0929309dd7;p=gostls13.git

misc/cgo: fix aliasing bugs in parallel tests that append to shared slices

These tests use a slice to represent the base C compiler command (with
flags). Appending to that slice can cause subtle aliasing bugs, such
as commands that silently corrupt the arguments of other concurrent
commands in parallel tests.

In this change, we explicitly reduce the capacity of the command slice
to force appends to it to always allocate unique new slices.

Fixes #49693

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

diff --git a/misc/cgo/errors/badsym_test.go b/misc/cgo/errors/badsym_test.go
index fc687567bf..bc3ba2b489 100644
--- a/misc/cgo/errors/badsym_test.go
+++ b/misc/cgo/errors/badsym_test.go
@@ -201,6 +201,10 @@ func cCompilerCmd(t *testing.T) []string {
 	if !lastSpace {
 		cc = append(cc, s[start:])
 	}
+
+	// Force reallocation (and avoid aliasing bugs) for tests that append to cc.
+	cc = cc[:len(cc):len(cc)]
+
 	return cc
 }
 
diff --git a/misc/cgo/testcarchive/carchive_test.go b/misc/cgo/testcarchive/carchive_test.go
index a821396c77..d36b97b70e 100644
--- a/misc/cgo/testcarchive/carchive_test.go
+++ b/misc/cgo/testcarchive/carchive_test.go
@@ -11,7 +11,6 @@ import (
 	"flag"
 	"fmt"
 	"io"
-	"io/fs"
 	"log"
 	"os"
 	"os/exec"
@@ -141,6 +140,9 @@ func testMain(m *testing.M) int {
 	libgodir = filepath.Join(GOPATH, "pkg", libbase, "testcarchive")
 	cc = append(cc, "-I", libgodir)
 
+	// Force reallocation (and avoid aliasing bugs) for parallel tests that append to cc.
+	cc = cc[:len(cc):len(cc)]
+
 	if GOOS == "windows" {
 		exeSuffix = ".exe"
 	}
@@ -248,29 +250,6 @@ func testInstall(t *testing.T, exe, libgoa, libgoh string, buildcmd ...string) {
 
 var badLineRegexp = regexp.MustCompile(`(?m)^#line [0-9]+ "/.*$`)
 
-// checkIsExecutable verifies that exe exists and has execute permission.
-//
-// (https://golang.org/issue/49693 notes failures with "no such file or
-// directory", so we want to double-check that the executable actually exists
-// immediately after we build it in order to better understand that failure
-// mode.)
-func checkIsExecutable(t *testing.T, exe string) {
-	t.Helper()
-	fi, err := os.Stat(exe)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if runtime.GOOS == "windows" {
-		// os.File doesn't check the "execute" permission on Windows files
-		// and as a result doesn't set that bit in a file's permissions.
-		// Assume that if the file exists it is “executable enough”.
-		return
-	}
-	if fi.Mode()&0111 == 0 {
-		t.Fatalf("%s is not executable: %0o", exe, fi.Mode()&fs.ModePerm)
-	}
-}
-
 // checkLineComments checks that the export header generated by
 // -buildmode=c-archive doesn't have any absolute paths in the #line
 // comments. We don't want those paths because they are unhelpful for
@@ -964,7 +943,6 @@ func TestSIGPROF(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	checkIsExecutable(t, "./testp6"+exeSuffix)
 
 	argv := cmdToRun("./testp6")
 	cmd = exec.Command(argv[0], argv[1:]...)
@@ -1113,7 +1091,6 @@ func TestManyCalls(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	checkIsExecutable(t, "./testp7"+exeSuffix)
 
 	argv := cmdToRun("./testp7")
 	cmd = exec.Command(argv[0], argv[1:]...)
@@ -1170,7 +1147,6 @@ func TestPreemption(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	checkIsExecutable(t, "./testp8"+exeSuffix)
 
 	argv := cmdToRun("./testp8")
 	cmd = exec.Command(argv[0], argv[1:]...)
diff --git a/misc/cgo/testcshared/cshared_test.go b/misc/cgo/testcshared/cshared_test.go
index 13ec8761e8..c9e9e5fe63 100644
--- a/misc/cgo/testcshared/cshared_test.go
+++ b/misc/cgo/testcshared/cshared_test.go
@@ -117,6 +117,9 @@ func testMain(m *testing.M) int {
 	}
 	cc = append(cc, "-I", filepath.Join("pkg", libgodir))
 
+	// Force reallocation (and avoid aliasing bugs) for parallel tests that append to cc.
+	cc = cc[:len(cc):len(cc)]
+
 	if GOOS == "windows" {
 		exeSuffix = ".exe"
 	}