cmd/go: add detection of toolchain switch loops

This happens mainly during testing because the virtual
toolchain switch is not terribly robust, and if you accidentally
try to exec "1.23" instead of "go1.23" it will let you, but it
won't work right.

Of course, although we feel pretty good about the non-test
implementation, perhaps it has a toolchain switch loop lurking too,
or perhaps one will be introduced in the future.

To handle the test bug, and just in case we have a real bug later,
add detection of toolchain switch loops with clear messages.

Also fixes a bug in setting the -lang flag properly when invoking
the Go compiler: this is the first test using 'go 1.21.x' lines
during a build.

For #57001.

Change-Id: I0ece3dd718596689a23b677cf08ddf32ea97bc57
Reviewed-on: https://go-review.googlesource.com/c/go/+/498436
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/go/go_test.go

@@ -36,6 +36,7 @@ import (
 	"cmd/go/internal/gover"
 	"cmd/go/internal/robustio"
 	"cmd/go/internal/search"
+	"cmd/go/internal/toolchain"
 	"cmd/go/internal/vcs"
 	"cmd/go/internal/vcweb/vcstest"
 	"cmd/go/internal/web"
@@ -108,10 +109,8 @@ func TestMain(m *testing.M) {
 	// We set CMDGO_TEST_RUN_MAIN via os.Setenv and testScript.setup.
 	if os.Getenv("CMDGO_TEST_RUN_MAIN") != "" {
 		cfg.SetGOROOT(cfg.GOROOT, true)
-
-		if v := os.Getenv("TESTGO_VERSION"); v != "" {
-			gover.TestVersion = v
-		}
+		gover.TestVersion = os.Getenv("TESTGO_VERSION")
+		toolchain.TestVersionSwitch = os.Getenv("TESTGO_VERSION_SWITCH")
 		if v := os.Getenv("TESTGO_TOOLCHAIN_VERSION"); v != "" {
 			work.ToolchainVersion = v
 		}

src/cmd/go/internal/gover/local.go

@@ -17,14 +17,26 @@ var TestVersion string
 
 // Local returns the local Go version, the one implemented by this go command.
 func Local() string {
-	v := runtime.Version()
-	if TestVersion != "" {
-		v = TestVersion
-	}
-	if v := FromToolchain(v); v != "" {
-		return v
-	}
-
-	// Development branch. Use "Dev" version with just 1.N, no rc1 or .0 suffix.
-	return "1." + strconv.Itoa(goversion.Version)
+	v, _ := local()
+	return v
+}
+
+// LocalToolchain returns the local toolchain name, the one implemented by this go command.
+func LocalToolchain() string {
+	_, t := local()
+	return t
+}
+
+func local() (goVers, toolVers string) {
+	toolVers = runtime.Version()
+	if TestVersion != "" {
+		toolVers = TestVersion
+	}
+	goVers = FromToolchain(toolVers)
+	if goVers == "" {
+		// Development branch. Use "Dev" version with just 1.N, no rc1 or .0 suffix.
+		goVers = "1." + strconv.Itoa(goversion.Version)
+		toolVers = "go" + goVers
+	}
+	return goVers, toolVers
 }

src/cmd/go/internal/toolchain/exec.go

@@ -20,7 +20,7 @@ import (
 // The GOROOT directory is empty if we are invoking a command named
 // gotoolchain found in $PATH.
 func execGoToolchain(gotoolchain, dir, exe string) {
-	os.Setenv(gotoolchainSwitchEnv, "1")
+	os.Setenv(targetEnv, gotoolchain)
 	if dir == "" {
 		os.Unsetenv("GOROOT")
 	} else {

src/cmd/go/internal/toolchain/toolchain.go

@@ -16,6 +16,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"sort"
+	"strconv"
 	"strings"
 
 	"cmd/go/internal/base"
@@ -40,16 +41,46 @@ const (
 	gotoolchainModule  = "golang.org/toolchain"
 	gotoolchainVersion = "v0.0.1"
 
-	// gotoolchainSwitchEnv is a special environment variable
-	// set to 1 during the toolchain switch by the parent process
-	// and cleared in the child process. When set, that indicates
-	// to the child not to do its own toolchain switch logic,
-	// to avoid an infinite recursion if for some reason a toolchain
-	// did not believe it could handle its own version and then
-	// reinvoked itself.
-	gotoolchainSwitchEnv = "GOTOOLCHAIN_INTERNAL_SWITCH"
+	// targetEnv is a special environment variable set to the expected
+	// toolchain version during the toolchain switch by the parent
+	// process and cleared in the child process. When set, that indicates
+	// to the child to confirm that it provides the expected toolchain version.
+	targetEnv = "GOTOOLCHAIN_INTERNAL_SWITCH_VERSION"
+
+	// countEnv is a special environment variable
+	// that is incremented during each toolchain switch, to detect loops.
+	// It is cleared before invoking programs in 'go run', 'go test', 'go generate', and 'go tool'
+	// by invoking them in an environment filtered with FilterEnv,
+	// so user programs should not see this in their environment.
+	countEnv = "GOTOOLCHAIN_INTERNAL_SWITCH_COUNT"
+
+	// maxSwitch is the maximum toolchain switching depth.
+	// Most uses should never see more than three.
+	// (Perhaps one for the initial GOTOOLCHAIN dispatch,
+	// a second for go get doing an upgrade, and a third if
+	// for some reason the chosen upgrade version is too small
+	// by a little.)
+	// When the count reaches maxSwitch - 10, we start logging
+	// the switched versions for debugging before crashing with
+	// a fatal error upon reaching maxSwitch.
+	// That should be enough to see the repetition.
+	maxSwitch = 100
 )
 
+// FilterEnv returns a copy of env with internal GOTOOLCHAIN environment
+// variables filtered out.
+func FilterEnv(env []string) []string {
+	// Note: Don't need to filter out targetEnv because Switch does that.
+	var out []string
+	for _, e := range env {
+		if strings.HasPrefix(e, countEnv+"=") {
+			continue
+		}
+		out = append(out, e)
+	}
+	return out
+}
+
 // Switch invokes a different Go toolchain if directed by
 // the GOTOOLCHAIN environment variable or the user's configuration
 // or go.mod file.
@@ -58,10 +89,6 @@ func Switch() {
 	log.SetPrefix("go: ")
 	defer log.SetPrefix("")
 
-	sw := os.Getenv(gotoolchainSwitchEnv)
-	os.Unsetenv(gotoolchainSwitchEnv)
-	// The sw == "1" check is delayed until later so that we still fill in gover.Startup for use in errors.
-
 	if !modload.WillBeEnabled() {
 		return
 	}
@@ -78,20 +105,19 @@ func Switch() {
 		return
 	}
 
-	var minToolchain, minVers string
-	if x, y, ok := strings.Cut(gotoolchain, "+"); ok { // go1.2.3+auto
-		orig := gotoolchain
-		minToolchain, gotoolchain = x, y
-		minVers = gover.FromToolchain(minToolchain)
-		if minVers == "" {
-			base.Fatalf("invalid GOTOOLCHAIN %q: invalid minimum toolchain %q", orig, minToolchain)
+	minToolchain := gover.LocalToolchain()
+	minVers := gover.Local()
+	if min, mode, ok := strings.Cut(gotoolchain, "+"); ok { // go1.2.3+auto
+		v := gover.FromToolchain(min)
+		if v == "" {
+			base.Fatalf("invalid GOTOOLCHAIN %q: invalid minimum toolchain %q", gotoolchain, min)
 		}
-		if gotoolchain != "auto" && gotoolchain != "path" {
-			base.Fatalf("invalid GOTOOLCHAIN %q: only version suffixes are +auto and +path", orig)
+		minToolchain = min
+		minVers = v
+		if mode != "auto" && mode != "path" {
+			base.Fatalf("invalid GOTOOLCHAIN %q: only version suffixes are +auto and +path", gotoolchain)
 		}
-	} else {
-		minVers = gover.Local()
-		minToolchain = "go" + minVers
+		gotoolchain = mode
 	}
 
 	if gotoolchain == "auto" || gotoolchain == "path" {
@@ -153,7 +179,34 @@ func Switch() {
 		}
 	}
 
-	if sw == "1" || gotoolchain == "local" || gotoolchain == "go"+gover.Local() {
+	// If we are invoked as a target toolchain, confirm that
+	// we provide the expected version and then run.
+	// This check is delayed until after the handling of auto and path
+	// so that we have initialized gover.Startup for use in error messages.
+	if target := os.Getenv(targetEnv); target != "" && TestVersionSwitch != "loop" {
+		if gover.LocalToolchain() != target {
+			base.Fatalf("toolchain %v invoked to provide %v", gover.LocalToolchain(), target)
+		}
+		os.Unsetenv(targetEnv)
+
+		// Note: It is tempting to check that if gotoolchain != "local"
+		// then target == gotoolchain here, as a sanity check that
+		// the child has made the same version determination as the parent.
+		// This turns out not always to be the case. Specifically, if we are
+		// running Go 1.21 with GOTOOLCHAIN=go1.22+auto, which invokes
+		// Go 1.22, then 'go get go@1.23.0' or 'go get needs_go_1_23'
+		// will invoke Go 1.23, but as the Go 1.23 child the reason for that
+		// will not be apparent here: it will look like we should be using Go 1.22.
+		// We rely on the targetEnv being set to know not to downgrade.
+		// A longer term problem with the sanity check is that the exact details
+		// may change over time: there may be other reasons that a future Go
+		// version might invoke an older one, and the older one won't know why.
+		// Best to just accept that we were invoked to provide a specific toolchain
+		// (which we just checked) and leave it at that.
+		return
+	}
+
+	if gotoolchain == "local" || gotoolchain == gover.LocalToolchain() {
 		// Let the current binary handle the command.
 		return
 	}
@@ -287,6 +340,14 @@ func HasPath() bool {
 	return env == "path" || strings.HasSuffix(env, "+path")
 }
 
+// TestVersionSwitch is set in the test go binary to the value in $TESTGO_VERSION_SWITCH.
+// Valid settings are:
+//
+//	"switch" - simulate version switches by reinvoking the test go binary with a different TESTGO_VERSION.
+//	"mismatch" - like "switch" but forget to set TESTGO_VERSION, so it looks like we invoked a mismatched toolchain
+//	"loop" - like "switch" but
+var TestVersionSwitch string
+
 // SwitchTo invokes the specified Go toolchain or else prints an error and exits the process.
 // If $GOTOOLCHAIN is set to path or min+path, SwitchTo only considers the PATH
 // as a source of Go toolchains. Otherwise SwitchTo tries the PATH but then downloads
@@ -294,16 +355,32 @@ func HasPath() bool {
 func SwitchTo(gotoolchain string) {
 	log.SetPrefix("go: ")
 
+	count, _ := strconv.Atoi(os.Getenv(countEnv))
+	if count >= maxSwitch-10 {
+		fmt.Fprintf(os.Stderr, "go: switching from go%v to %v [depth %d]\n", gover.Local(), gotoolchain, count)
+	}
+	if count >= maxSwitch {
+		base.Fatalf("too many toolchain switches")
+	}
+	os.Setenv(countEnv, fmt.Sprint(count+1))
+
 	env := cfg.Getenv("GOTOOLCHAIN")
 	pathOnly := env == "path" || strings.HasSuffix(env, "+path")
 
 	// For testing, if TESTGO_VERSION is already in use
 	// (only happens in the cmd/go test binary)
-	// and TESTGO_VERSION_SWITCH=1 is set,
+	// and TESTGO_VERSION_SWITCH=switch is set,
 	// "switch" toolchains by changing TESTGO_VERSION
 	// and reinvoking the current binary.
-	if gover.TestVersion != "" && os.Getenv("TESTGO_VERSION_SWITCH") == "1" {
+	// The special cases =loop and =mismatch skip the
+	// setting of TESTGO_VERSION so that it looks like we
+	// accidentally invoked the wrong toolchain,
+	// to test detection of that failure mode.
+	switch TestVersionSwitch {
+	case "switch":
 		os.Setenv("TESTGO_VERSION", gotoolchain)
+		fallthrough
+	case "loop", "mismatch":
 		exe, err := os.Executable()
 		if err != nil {
 			base.Fatalf("%v", err)
@@ -422,7 +499,7 @@ func modGoToolchain() (file, goVers, toolchain string) {
 
 // goInstallVersion looks at the command line to see if it is go install m@v or go run m@v.
 // If so, it returns the m@v and the go version from that module's go.mod.
-func goInstallVersion() (m module.Version, goVers string, ok bool) {
+func goInstallVersion() (m module.Version, goVers string, found bool) {
 	// Note: We assume there are no flags between 'go' and 'install' or 'run'.
 	// During testing there are some debugging flags that are accepted
 	// in that position, but in production go binaries there are not.
@@ -503,10 +580,15 @@ func goInstallVersion() (m module.Version, goVers string, ok bool) {
 	}
 	noneSelected := func(path string) (version string) { return "none" }
 	_, err := modload.QueryPackages(ctx, m.Path, m.Version, noneSelected, allowed)
-	tooNew, ok := err.(*gover.TooNewError)
-	if !ok {
-		return module.Version{}, "", false
+	if tooNew, ok := err.(*gover.TooNewError); ok {
+		m.Path, m.Version, _ = strings.Cut(tooNew.What, "@")
+		return m, tooNew.GoVersion, true
 	}
-	m.Path, m.Version, _ = strings.Cut(tooNew.What, "@")
-	return m, tooNew.GoVersion, true
+
+	// QueryPackages succeeded, or it failed for a reason other than
+	// this Go toolchain being too old for the modules encountered.
+	// Either way, we identified the m@v on the command line,
+	// so return found == true so the caller does not fall back to
+	// consulting go.mod.
+	return m, "", true
 }

src/cmd/go/internal/work/exec.go

@@ -35,6 +35,7 @@ import (
 	"cmd/go/internal/cache"
 	"cmd/go/internal/cfg"
 	"cmd/go/internal/fsys"
+	"cmd/go/internal/gover"
 	"cmd/go/internal/load"
 	"cmd/go/internal/modload"
 	"cmd/go/internal/slices"
@@ -431,17 +432,7 @@ func allowedVersion(v string) bool {
 	if v == "" {
 		return true
 	}
-	// Special case "1.0" means "go1", which is OK.
-	if v == "1.0" {
-		return true
-	}
-	// Otherwise look through release tags of form "go1.23" for one that matches.
-	for _, tag := range cfg.BuildContext.ReleaseTags {
-		if strings.HasPrefix(tag, "go") && tag[2:] == v {
-			return true
-		}
-	}
-	return false
+	return gover.Compare(gover.Local(), v) >= 0
 }
 
 const (

src/cmd/go/internal/work/gc.go

@@ -19,6 +19,7 @@ import (
 	"cmd/go/internal/base"
 	"cmd/go/internal/cfg"
 	"cmd/go/internal/fsys"
+	"cmd/go/internal/gover"
 	"cmd/go/internal/load"
 	"cmd/go/internal/str"
 	"cmd/internal/objabi"
@@ -99,7 +100,7 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg
 			v = "1.16"
 		}
 		if allowedVersion(v) {
-			defaultGcFlags = append(defaultGcFlags, "-lang=go"+v)
+			defaultGcFlags = append(defaultGcFlags, "-lang=go"+gover.Lang(v))
 		}
 	}
 	if p.Standard {

src/cmd/go/main.go

@@ -209,7 +209,7 @@ func invoke(cmd *base.Command, args []string) {
 	// the same default computation of these as we do,
 	// but in practice there might be skew
 	// This makes sure we all agree.
-	cfg.OrigEnv = os.Environ()
+	cfg.OrigEnv = toolchain.FilterEnv(os.Environ())
 	cfg.CmdEnv = envcmd.MkEnv()
 	for _, env := range cfg.CmdEnv {
 		if os.Getenv(env.Name) != env.Value {

src/cmd/go/testdata/script/gotoolchain_local.txt

@@ -3,7 +3,7 @@
 # See gotoolchain_net.txt and gotoolchain_path.txt for tests of network and PATH toolchains.
 
 env TESTGO_VERSION=go1.500
-env TESTGO_VERSION_SWITCH=1
+env TESTGO_VERSION_SWITCH=switch
 
 # Default setting should be auto
 env GOTOOLCHAIN=

src/cmd/go/testdata/script/gotoolchain_loop.txt

@@ -0,0 +1,65 @@
+env GOTOOLCHAIN=auto
+env TESTGO_VERSION=go1.21.1
+
+# Basic switch should work.
+env TESTGO_VERSION_SWITCH=switch
+go version
+stdout go1.21.99
+
+# Toolchain target mismatch should be detected.
+env TESTGO_VERSION_SWITCH=mismatch
+! go version
+stderr '^go: toolchain go1.21.1 invoked to provide go1.21.99$'
+
+# Toolchain loop should be detected.
+env TESTGO_VERSION_SWITCH=loop
+! go version
+stderr -count=10 '^go: switching from go1.21.1 to go1.21.99 \[depth 9[0-9]\]$'
+stderr -count=1 '^go: switching from go1.21.1 to go1.21.99 \[depth 100\]$'
+stderr '^go: too many toolchain switches$'
+
+[short] skip
+
+# Internal env vars should not leak to go test or go run.
+env TESTGO_VERSION_SWITCH=switch
+go version
+stdout go1.21.99
+go test
+stdout clean
+go run .
+stdout clean
+
+-- go.mod --
+module m
+go 1.21.99
+
+-- m_test.go --
+package main
+
+import "testing"
+
+func TestEnv(t *testing.T) {
+	// the check is in func init in m.go
+}
+
+-- m.go --
+package main
+
+import "os"
+
+func init() {
+	envs := []string{
+		"GOTOOLCHAIN_INTERNAL_SWITCH_COUNT",
+		"GOTOOLCHAIN_INTERNAL_SWITCH_VERSION",
+	}
+	for _, e := range envs {
+		if v := os.Getenv(e); v != "" {
+			panic("$"+e+"="+v)
+		}
+	}
+	os.Stdout.WriteString("clean\n")
+}
+
+func main() {
+}
+

src/cmd/go/testdata/script/mod_get_exec_toolchain.txt

@@ -1,5 +1,5 @@
 env TESTGO_VERSION=go1.21
-env TESTGO_VERSION_SWITCH=1
+env TESTGO_VERSION_SWITCH=switch
 
 # GOTOOLCHAIN=auto should run the newer toolchain
 env GOTOOLCHAIN=auto

src/cmd/go/testdata/script/mod_toolchain.txt

@@ -1,5 +1,5 @@
 env TESTGO_VERSION=go1.100
-env TESTGO_VERSION_SWITCH=1
+env TESTGO_VERSION_SWITCH=switch
 
 go get toolchain@go1.22.1
 stderr '^go: added toolchain go1.22.1$'