From 805f6b3f5db714ce8f7dae2776748f6df96f288b Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 24 Apr 2024 12:59:02 -0400 Subject: [PATCH] cmd/dist: reject accidental use of internal packages from bootstrap toolchain The compiler was accidentally using internal/godebug from the Go 1.20 bootstrap toolchain and didn't get the behavior it expected. Generalizing, we should never assume we know the behavior of an internal package from an earlier bootstrap toolchain, so disallow that case in cmd/dist. Change-Id: I41e079f6120f4081124619bbe2b30069c96b9f29 Reviewed-on: https://go-review.googlesource.com/c/go/+/581496 Reviewed-by: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Auto-Submit: Russ Cox --- src/cmd/dist/buildtool.go | 72 ++++++++++++++++++++++++++++++++++----- 1 file changed, 64 insertions(+), 8 deletions(-) diff --git a/src/cmd/dist/buildtool.go b/src/cmd/dist/buildtool.go index 1141981329..a47b7f90da 100644 --- a/src/cmd/dist/buildtool.go +++ b/src/cmd/dist/buildtool.go @@ -300,6 +300,11 @@ func rewriteBlock%s(b *Block) bool { panic("unused during bootstrap") } return bootstrapFixImports(srcFile) } +var ( + importRE = regexp.MustCompile(`\Aimport\s+(\.|[A-Za-z0-9_]+)?\s*"([^"]+)"\s*(//.*)?\n\z`) + importBlockRE = regexp.MustCompile(`\A\s*(?:(\.|[A-Za-z0-9_]+)?\s*"([^"]+)")?\s*(//.*)?\n\z`) +) + func bootstrapFixImports(srcFile string) string { text := readfile(srcFile) if !strings.Contains(srcFile, "/cmd/") && !strings.Contains(srcFile, `\cmd\`) { @@ -307,7 +312,17 @@ func bootstrapFixImports(srcFile string) string { } lines := strings.SplitAfter(text, "\n") inBlock := false + inComment := false for i, line := range lines { + if strings.HasSuffix(line, "*/\n") { + inComment = false + } + if strings.HasSuffix(line, "/*\n") { + inComment = true + } + if inComment { + continue + } if strings.HasPrefix(line, "import (") { inBlock = true continue @@ -316,15 +331,56 @@ func bootstrapFixImports(srcFile string) string { inBlock = false continue } - if strings.HasPrefix(line, `import `) || inBlock { - line = strings.Replace(line, `"cmd/`, `"bootstrap/cmd/`, -1) - for _, dir := range bootstrapDirs { - if strings.HasPrefix(dir, "cmd/") { - continue - } - line = strings.Replace(line, `"`+dir+`"`, `"bootstrap/`+dir+`"`, -1) + + var m []string + if !inBlock { + if !strings.HasPrefix(line, "import ") { + continue } - lines[i] = line + m = importRE.FindStringSubmatch(line) + if m == nil { + fatalf("%s:%d: invalid import declaration: %q", srcFile, i+1, line) + } + } else { + m = importBlockRE.FindStringSubmatch(line) + if m == nil { + fatalf("%s:%d: invalid import block line", srcFile, i+1) + } + if m[2] == "" { + continue + } + } + + path := m[2] + if strings.HasPrefix(path, "cmd/") { + path = "bootstrap/" + path + } else { + for _, dir := range bootstrapDirs { + if path == dir { + path = "bootstrap/" + dir + break + } + } + } + + // Rewrite use of internal/reflectlite to be plain reflect. + if path == "internal/reflectlite" { + lines[i] = strings.ReplaceAll(line, `"reflect"`, `reflectlite "reflect"`) + continue + } + + // Otherwise, reject direct imports of internal packages, + // since that implies knowledge of internal details that might + // change from one bootstrap toolchain to the next. + // There are many internal packages that are listed in + // bootstrapDirs and made into bootstrap copies based on the + // current repo's source code. Those are fine; this is catching + // references to internal packages in the older bootstrap toolchain. + if strings.HasPrefix(path, "internal/") { + fatalf("%s:%d: bootstrap-copied source file cannot import %s", srcFile, i+1, path) + } + if path != m[2] { + lines[i] = strings.ReplaceAll(line, `"`+m[2]+`"`, `"`+path+`"`) } }