os/exec: build TestExtraFiles subprocess without cgo

Fixes #25628

Change-Id: I8b69e59f9c0123c4f65b5931d7c6d7ecc1c720e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/228639
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

2 files changed, 114 insertions, 94 deletions

diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go
index d9c9812554..dafbc64a17 100644
--- a/src/os/exec/exec_test.go
+++ b/src/os/exec/exec_test.go
@@ -79,17 +79,6 @@ func helperCommandContext(t *testing.T, ctx context.Context, s ...string) (cmd *
 	} else {
 		cmd = exec.Command(os.Args[0], cs...)
 	}
-
-	// Temporary code to try to resolve #25628.
-	// TODO(iant): Remove this when we no longer need it.
-	if runtime.GOARCH == "386" && runtime.GOOS == "linux" && testenv.Builder() != "" && len(s) == 1 && s[0] == "read3" {
-		sctx := ctx
-		if sctx == nil {
-			sctx = context.Background()
-		}
-		cmd = exec.CommandContext(sctx, "/usr/bin/strace", append([]string{"-f", os.Args[0]}, cs...)...)
-	}
-
 	cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
 	return cmd
 }
@@ -499,25 +488,6 @@ func numOpenFDsAndroid(t *testing.T) (n int, lsof []byte) {
 	return bytes.Count(lsof, []byte("\n")), lsof
 }
 
-// basefds returns the number of expected file descriptors
-// to be present in a process at start.
-// stdin, stdout, stderr, epoll/kqueue, epoll/kqueue pipe, maybe testlog
-func basefds() uintptr {
-	n := os.Stderr.Fd() + 1
-	// The poll (epoll/kqueue) descriptor can be numerically
-	// either between stderr and the testlog-fd, or after
-	// testlog-fd.
-	for poll.IsPollDescriptor(n) {
-		n++
-	}
-	for _, arg := range os.Args {
-		if strings.HasPrefix(arg, "-test.testlogfile=") {
-			n++
-		}
-	}
-	return n
-}
-
 func TestExtraFilesFDShuffle(t *testing.T) {
 	t.Skip("flaky test; see https://golang.org/issue/5780")
 	switch runtime.GOOS {
@@ -633,6 +603,7 @@ func TestExtraFiles(t *testing.T) {
 	}
 
 	testenv.MustHaveExec(t)
+	testenv.MustHaveGoBuild(t)
 
 	if runtime.GOOS == "windows" {
 		t.Skipf("skipping test on %q", runtime.GOOS)
@@ -687,6 +658,18 @@ func TestExtraFiles(t *testing.T) {
 		t.Fatalf("Seek: %v", err)
 	}
 
+	tempdir := t.TempDir()
+	exe := filepath.Join(tempdir, "read3.exe")
+
+	c := exec.Command(testenv.GoToolPath(t), "build", "-o", exe, "read3.go")
+	// Build the test without cgo, so that C library functions don't
+	// open descriptors unexpectedly. See issue 25628.
+	c.Env = append(os.Environ(), "CGO_ENABLED=0")
+	if output, err := c.CombinedOutput(); err != nil {
+		t.Logf("go build -o %s read3.go\n%s", exe, output)
+		t.Fatalf("go build failed: %v", err)
+	}
+
 	// Use a deadline to try to get some output even if the program hangs.
 	ctx := context.Background()
 	if deadline, ok := t.Deadline(); ok {
@@ -698,7 +681,8 @@ func TestExtraFiles(t *testing.T) {
 		ctx, cancel = context.WithDeadline(ctx, deadline)
 		defer cancel()
 	}
-	c := helperCommandContext(t, ctx, "read3")
+
+	c = exec.CommandContext(ctx, exe)
 	var stdout, stderr bytes.Buffer
 	c.Stdout = &stdout
 	c.Stderr = &stderr
@@ -779,17 +763,6 @@ func TestHelperProcess(*testing.T) {
 	}
 	defer os.Exit(0)
 
-	// Determine which command to use to display open files.
-	ofcmd := "lsof"
-	switch runtime.GOOS {
-	case "dragonfly", "freebsd", "netbsd", "openbsd":
-		ofcmd = "fstat"
-	case "plan9":
-		ofcmd = "/bin/cat"
-	case "aix":
-		ofcmd = "procfiles"
-	}
-
 	args := os.Args
 	for len(args) > 0 {
 		if args[0] == "--" {
@@ -863,58 +836,6 @@ func TestHelperProcess(*testing.T) {
 			os.Exit(1)
 		}
 		os.Exit(0)
-	case "read3": // read fd 3
-		fd3 := os.NewFile(3, "fd3")
-		bs, err := ioutil.ReadAll(fd3)
-		if err != nil {
-			fmt.Printf("ReadAll from fd 3: %v", err)
-			os.Exit(1)
-		}
-		// Now verify that there are no other open fds.
-		var files []*os.File
-		for wantfd := basefds() + 1; wantfd <= 100; wantfd++ {
-			if poll.IsPollDescriptor(wantfd) {
-				continue
-			}
-			f, err := os.Open(os.Args[0])
-			if err != nil {
-				fmt.Printf("error opening file with expected fd %d: %v", wantfd, err)
-				os.Exit(1)
-			}
-			if got := f.Fd(); got != wantfd {
-				fmt.Printf("leaked parent file. fd = %d; want %d\n", got, wantfd)
-				fdfile := fmt.Sprintf("/proc/self/fd/%d", wantfd)
-				link, err := os.Readlink(fdfile)
-				fmt.Printf("readlink(%q) = %q, %v\n", fdfile, link, err)
-				var args []string
-				switch runtime.GOOS {
-				case "plan9":
-					args = []string{fmt.Sprintf("/proc/%d/fd", os.Getpid())}
-				case "aix":
-					args = []string{fmt.Sprint(os.Getpid())}
-				default:
-					args = []string{"-p", fmt.Sprint(os.Getpid())}
-				}
-				cmd := exec.Command(ofcmd, args...)
-				out, err := cmd.CombinedOutput()
-				if err != nil {
-					fmt.Fprintf(os.Stderr, "%s failed: %v\n", strings.Join(cmd.Args, " "), err)
-				}
-				fmt.Printf("%s", out)
-				os.Exit(1)
-			}
-			files = append(files, f)
-		}
-		for _, f := range files {
-			f.Close()
-		}
-		// Referring to fd3 here ensures that it is not
-		// garbage collected, and therefore closed, while
-		// executing the wantfd loop above. It doesn't matter
-		// what we do with fd3 as long as we refer to it;
-		// closing it is the easy choice.
-		fd3.Close()
-		os.Stdout.Write(bs)
 	case "exit":
 		n, _ := strconv.Atoi(args[0])
 		os.Exit(n)
diff --git a/src/os/exec/read3.go b/src/os/exec/read3.go
new file mode 100644
index 0000000000..25d732a991
--- /dev/null
+++ b/src/os/exec/read3.go
@@ -0,0 +1,99 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build ignore
+
+// This is a test program that verifies that it can read from
+// descriptor 3 and that no other descriptors are open.
+// This is not done via TestHelperProcess and GO_WANT_HELPER_PROCESS
+// because we want to ensure that this program does not use cgo,
+// because C libraries can open file descriptors behind our backs
+// and confuse the test. See issue 25628.
+package main
+
+import (
+	"fmt"
+	"internal/poll"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"runtime"
+	"strings"
+)
+
+func main() {
+	fd3 := os.NewFile(3, "fd3")
+	bs, err := ioutil.ReadAll(fd3)
+	if err != nil {
+		fmt.Printf("ReadAll from fd 3: %v\n", err)
+		os.Exit(1)
+	}
+
+	// Now verify that there are no other open fds.
+	// stdin == 0
+	// stdout == 1
+	// stderr == 2
+	// descriptor from parent == 3
+	// All descriptors 4 and up should be available,
+	// except for any used by the network poller.
+	var files []*os.File
+	for wantfd := uintptr(4); wantfd <= 100; wantfd++ {
+		if poll.IsPollDescriptor(wantfd) {
+			continue
+		}
+		f, err := os.Open(os.Args[0])
+		if err != nil {
+			fmt.Printf("error opening file with expected fd %d: %v", wantfd, err)
+			os.Exit(1)
+		}
+		if got := f.Fd(); got != wantfd {
+			fmt.Printf("leaked parent file. fd = %d; want %d\n", got, wantfd)
+			fdfile := fmt.Sprintf("/proc/self/fd/%d", wantfd)
+			link, err := os.Readlink(fdfile)
+			fmt.Printf("readlink(%q) = %q, %v\n", fdfile, link, err)
+			var args []string
+			switch runtime.GOOS {
+			case "plan9":
+				args = []string{fmt.Sprintf("/proc/%d/fd", os.Getpid())}
+			case "aix":
+				args = []string{fmt.Sprint(os.Getpid())}
+			default:
+				args = []string{"-p", fmt.Sprint(os.Getpid())}
+			}
+
+			// Determine which command to use to display open files.
+			ofcmd := "lsof"
+			switch runtime.GOOS {
+			case "dragonfly", "freebsd", "netbsd", "openbsd":
+				ofcmd = "fstat"
+			case "plan9":
+				ofcmd = "/bin/cat"
+			case "aix":
+				ofcmd = "procfiles"
+			}
+
+			cmd := exec.Command(ofcmd, args...)
+			out, err := cmd.CombinedOutput()
+			if err != nil {
+				fmt.Fprintf(os.Stderr, "%s failed: %v\n", strings.Join(cmd.Args, " "), err)
+			}
+			fmt.Printf("%s", out)
+			os.Exit(1)
+		}
+		files = append(files, f)
+	}
+
+	for _, f := range files {
+		f.Close()
+	}
+
+	// Referring to fd3 here ensures that it is not
+	// garbage collected, and therefore closed, while
+	// executing the wantfd loop above. It doesn't matter
+	// what we do with fd3 as long as we refer to it;
+	// closing it is the easy choice.
+	fd3.Close()
+
+	os.Stdout.Write(bs)
+}