Skip to content

Commit 805f6b3

Browse files
rscgopherbot
authored andcommitted
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 <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Russ Cox <[email protected]>
1 parent acda010 commit 805f6b3

File tree

1 file changed

+62
-6
lines changed

1 file changed

+62
-6
lines changed

src/cmd/dist/buildtool.go

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -300,14 +300,29 @@ func rewriteBlock%s(b *Block) bool { panic("unused during bootstrap") }
300300
return bootstrapFixImports(srcFile)
301301
}
302302

303+
var (
304+
importRE = regexp.MustCompile(`\Aimport\s+(\.|[A-Za-z0-9_]+)?\s*"([^"]+)"\s*(//.*)?\n\z`)
305+
importBlockRE = regexp.MustCompile(`\A\s*(?:(\.|[A-Za-z0-9_]+)?\s*"([^"]+)")?\s*(//.*)?\n\z`)
306+
)
307+
303308
func bootstrapFixImports(srcFile string) string {
304309
text := readfile(srcFile)
305310
if !strings.Contains(srcFile, "/cmd/") && !strings.Contains(srcFile, `\cmd\`) {
306311
text = regexp.MustCompile(`\bany\b`).ReplaceAllString(text, "interface{}")
307312
}
308313
lines := strings.SplitAfter(text, "\n")
309314
inBlock := false
315+
inComment := false
310316
for i, line := range lines {
317+
if strings.HasSuffix(line, "*/\n") {
318+
inComment = false
319+
}
320+
if strings.HasSuffix(line, "/*\n") {
321+
inComment = true
322+
}
323+
if inComment {
324+
continue
325+
}
311326
if strings.HasPrefix(line, "import (") {
312327
inBlock = true
313328
continue
@@ -316,15 +331,56 @@ func bootstrapFixImports(srcFile string) string {
316331
inBlock = false
317332
continue
318333
}
319-
if strings.HasPrefix(line, `import `) || inBlock {
320-
line = strings.Replace(line, `"cmd/`, `"bootstrap/cmd/`, -1)
334+
335+
var m []string
336+
if !inBlock {
337+
if !strings.HasPrefix(line, "import ") {
338+
continue
339+
}
340+
m = importRE.FindStringSubmatch(line)
341+
if m == nil {
342+
fatalf("%s:%d: invalid import declaration: %q", srcFile, i+1, line)
343+
}
344+
} else {
345+
m = importBlockRE.FindStringSubmatch(line)
346+
if m == nil {
347+
fatalf("%s:%d: invalid import block line", srcFile, i+1)
348+
}
349+
if m[2] == "" {
350+
continue
351+
}
352+
}
353+
354+
path := m[2]
355+
if strings.HasPrefix(path, "cmd/") {
356+
path = "bootstrap/" + path
357+
} else {
321358
for _, dir := range bootstrapDirs {
322-
if strings.HasPrefix(dir, "cmd/") {
323-
continue
359+
if path == dir {
360+
path = "bootstrap/" + dir
361+
break
324362
}
325-
line = strings.Replace(line, `"`+dir+`"`, `"bootstrap/`+dir+`"`, -1)
326363
}
327-
lines[i] = line
364+
}
365+
366+
// Rewrite use of internal/reflectlite to be plain reflect.
367+
if path == "internal/reflectlite" {
368+
lines[i] = strings.ReplaceAll(line, `"reflect"`, `reflectlite "reflect"`)
369+
continue
370+
}
371+
372+
// Otherwise, reject direct imports of internal packages,
373+
// since that implies knowledge of internal details that might
374+
// change from one bootstrap toolchain to the next.
375+
// There are many internal packages that are listed in
376+
// bootstrapDirs and made into bootstrap copies based on the
377+
// current repo's source code. Those are fine; this is catching
378+
// references to internal packages in the older bootstrap toolchain.
379+
if strings.HasPrefix(path, "internal/") {
380+
fatalf("%s:%d: bootstrap-copied source file cannot import %s", srcFile, i+1, path)
381+
}
382+
if path != m[2] {
383+
lines[i] = strings.ReplaceAll(line, `"`+m[2]+`"`, `"`+path+`"`)
328384
}
329385
}
330386

0 commit comments

Comments
 (0)