Revert "os/exec: avoid calling LookPath in cmd.Start for resolved paths"

This reverts CL 512155.

Reason for revert: CL 512155 introduced a race in that it caused
cmd.Start to set cmd.Path. Previously it was fine if code looked
at cmd.Path in one goroutine while calling cmd.Start in a different
goroutine.

A test case for this race is in CL 527495.

Change-Id: Ic18fdadf6763727f8ea748280d5f0e601b9bf374
Reviewed-on: https://go-review.googlesource.com/c/go/+/527337
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

5 files changed, 32 insertions, 65 deletions

diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go
index 2881345fb3..dfede0e7e2 100644
--- a/src/os/exec/exec.go
+++ b/src/os/exec/exec.go
@@ -590,6 +590,32 @@ func (c *Cmd) Run() error {
 	return c.Wait()
 }
 
+// lookExtensions finds windows executable by its dir and path.
+// It uses LookPath to try appropriate extensions.
+// lookExtensions does not search PATH, instead it converts `prog` into `.\prog`.
+func lookExtensions(path, dir string) (string, error) {
+	if filepath.Base(path) == path {
+		path = "." + string(filepath.Separator) + path
+	}
+	if dir == "" {
+		return LookPath(path)
+	}
+	if filepath.VolumeName(path) != "" {
+		return LookPath(path)
+	}
+	if len(path) > 1 && os.IsPathSeparator(path[0]) {
+		return LookPath(path)
+	}
+	dirandpath := filepath.Join(dir, path)
+	// We assume that LookPath will only add file extension.
+	lp, err := LookPath(dirandpath)
+	if err != nil {
+		return "", err
+	}
+	ext := strings.TrimPrefix(lp, dirandpath)
+	return path + ext, nil
+}
+
 // Start starts the specified command but does not wait for it to complete.
 //
 // If Start returns successfully, the c.Process field will be set.
@@ -623,11 +649,13 @@ func (c *Cmd) Start() error {
 		}
 		return c.Err
 	}
-	lp, err := lookExtensions(c.Path, c.Dir)
-	if err != nil {
-		return err
+	if runtime.GOOS == "windows" {
+		lp, err := lookExtensions(c.Path, c.Dir)
+		if err != nil {
+			return err
+		}
+		c.Path = lp
 	}
-	c.Path = lp
 	if c.Cancel != nil && c.ctx == nil {
 		return errors.New("exec: command with a non-nil Cancel was not created with CommandContext")
 	}
diff --git a/src/os/exec/lp_plan9.go b/src/os/exec/lp_plan9.go
index dffdbac35f..9344b14e8c 100644
--- a/src/os/exec/lp_plan9.go
+++ b/src/os/exec/lp_plan9.go
@@ -64,9 +64,3 @@ func LookPath(file string) (string, error) {
 	}
 	return "", &Error{file, ErrNotFound}
 }
-
-// lookExtensions is a no-op on non-Windows platforms, since
-// they do not restrict executables to specific extensions.
-func lookExtensions(path, dir string) (string, error) {
-	return path, nil
-}
diff --git a/src/os/exec/lp_unix.go b/src/os/exec/lp_unix.go
index 3787132078..fd2c6efbef 100644
--- a/src/os/exec/lp_unix.go
+++ b/src/os/exec/lp_unix.go
@@ -80,9 +80,3 @@ func LookPath(file string) (string, error) {
 	}
 	return "", &Error{file, ErrNotFound}
 }
-
-// lookExtensions is a no-op on non-Windows platforms, since
-// they do not restrict executables to specific extensions.
-func lookExtensions(path, dir string) (string, error) {
-	return path, nil
-}
diff --git a/src/os/exec/lp_wasm.go b/src/os/exec/lp_wasm.go
index 3c819049ba..f2c8e9c5de 100644
--- a/src/os/exec/lp_wasm.go
+++ b/src/os/exec/lp_wasm.go
@@ -21,9 +21,3 @@ func LookPath(file string) (string, error) {
 	// Wasm can not execute processes, so act as if there are no executables at all.
 	return "", &Error{file, ErrNotFound}
 }
-
-// lookExtensions is a no-op on non-Windows platforms, since
-// they do not restrict executables to specific extensions.
-func lookExtensions(path, dir string) (string, error) {
-	return path, nil
-}
diff --git a/src/os/exec/lp_windows.go b/src/os/exec/lp_windows.go
index 7f13347c50..066d38dfdb 100644
--- a/src/os/exec/lp_windows.go
+++ b/src/os/exec/lp_windows.go
@@ -63,45 +63,6 @@ func findExecutable(file string, exts []string) (string, error) {
 // As of Go 1.19, LookPath will instead return that path along with an error satisfying
 // errors.Is(err, ErrDot). See the package documentation for more details.
 func LookPath(file string) (string, error) {
-	return lookPath(file, pathExt())
-}
-
-// lookExtensions finds windows executable by its dir and path.
-// It uses LookPath to try appropriate extensions.
-// lookExtensions does not search PATH, instead it converts `prog` into `.\prog`.
-func lookExtensions(path, dir string) (string, error) {
-	if filepath.Base(path) == path {
-		path = "." + string(filepath.Separator) + path
-	}
-	exts := pathExt()
-	if ext := filepath.Ext(path); ext != "" {
-		for _, e := range exts {
-			if strings.EqualFold(ext, e) {
-				// Assume that path has already been resolved.
-				return path, nil
-			}
-		}
-	}
-	if dir == "" {
-		return lookPath(path, exts)
-	}
-	if filepath.VolumeName(path) != "" {
-		return lookPath(path, exts)
-	}
-	if len(path) > 1 && os.IsPathSeparator(path[0]) {
-		return lookPath(path, exts)
-	}
-	dirandpath := filepath.Join(dir, path)
-	// We assume that LookPath will only add file extension.
-	lp, err := lookPath(dirandpath, exts)
-	if err != nil {
-		return "", err
-	}
-	ext := strings.TrimPrefix(lp, dirandpath)
-	return path + ext, nil
-}
-
-func pathExt() []string {
 	var exts []string
 	x := os.Getenv(`PATHEXT`)
 	if x != "" {
@@ -117,11 +78,7 @@ func pathExt() []string {
 	} else {
 		exts = []string{".com", ".exe", ".bat", ".cmd"}
 	}
-	return exts
-}
 
-// lookPath implements LookPath for the given PATHEXT list.
-func lookPath(file string, exts []string) (string, error) {
 	if strings.ContainsAny(file, `:\/`) {
 		f, err := findExecutable(file, exts)
 		if err == nil {