]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/dist: reject accidental use of internal packages from bootstrap toolchain
authorRuss Cox <rsc@golang.org>
Wed, 24 Apr 2024 16:59:02 +0000 (12:59 -0400)
committerGopher Robot <gobot@golang.org>
Thu, 9 May 2024 13:50:06 +0000 (13:50 +0000)
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 <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>

src/cmd/dist/buildtool.go

index 1141981329adfdc1ef32efaaed7fb4359625d39c..a47b7f90da970d7dc95bed1addebcdaac136dd45 100644 (file)
@@ -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)
+
+               var m []string
+               if !inBlock {
+                       if !strings.HasPrefix(line, "import ") {
+                               continue
+                       }
+                       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 strings.HasPrefix(dir, "cmd/") {
-                                       continue
+                               if path == dir {
+                                       path = "bootstrap/" + dir
+                                       break
                                }
-                               line = strings.Replace(line, `"`+dir+`"`, `"bootstrap/`+dir+`"`, -1)
                        }
-                       lines[i] = line
+               }
+
+               // 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+`"`)
                }
        }