Skip to content

Commit 8396015

Browse files
committed
cmd/link: set runtime.GOROOT default during link
Suppose you build the Go toolchain in directory A, move the whole thing to directory B, and then use it from B to build a new program hello.exe, and then run hello.exe, and hello.exe crashes with a stack trace into the standard library. Long ago, you'd have seen hello.exe print file names in the A directory tree, even though the files had moved to the B directory tree. About two years ago we changed the compiler to write down these files with the name "$GOROOT" (that literal string) instead of A, so that the final link from B could replace "$GOROOT" with B, so that hello.exe's crash would show the correct source file paths in the stack trace. (golang.org/cl/18200) Now suppose that you do the same thing but hello.exe doesn't crash: it prints fmt.Println(runtime.GOROOT()). And you run hello.exe after clearing $GOROOT from the environment. Long ago, you'd have seen hello.exe print A instead of B. Before this CL, you'd still see hello.exe print A instead of B. This case is the one instance where a moved toolchain still divulges its origin. Not anymore. After this CL, hello.exe will print B, because the linker sets runtime/internal/sys.DefaultGoroot with the effective GOROOT from link time. This makes the default result of runtime.GOROOT once again match the file names recorded in the binary, after two years of divergence. With that cleared up, we can reintroduce GOROOT into the link action ID and also reenable TestExecutableGOROOT/RelocatedExe. When $GOROOT_FINAL is set during link, it is used in preference to $GOROOT, as always, but it was easier to explain the behavior above without introducing that complication. Fixes #22155. Fixes #20284. Fixes #22475. Change-Id: Ifdaeb77fd4678fdb337cf59ee25b2cd873ec1016 Reviewed-on: https://go-review.googlesource.com/86835 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
1 parent 28639df commit 8396015

File tree

11 files changed

+48
-47
lines changed

11 files changed

+48
-47
lines changed

src/cmd/dist/buildruntime.go

-4
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
// mkzversion writes zversion.go:
1919
//
2020
// package sys
21-
// var DefaultGoroot = <goroot>
2221
//
2322
// const TheVersion = <version>
2423
// const Goexperiment = <goexperiment>
@@ -30,8 +29,6 @@ func mkzversion(dir, file string) {
3029
fmt.Fprintln(&buf)
3130
fmt.Fprintf(&buf, "package sys\n")
3231
fmt.Fprintln(&buf)
33-
fmt.Fprintf(&buf, "var DefaultGoroot = `%s`\n", goroot_final)
34-
fmt.Fprintln(&buf)
3532
fmt.Fprintf(&buf, "const TheVersion = `%s`\n", findgoversion())
3633
fmt.Fprintf(&buf, "const Goexperiment = `%s`\n", os.Getenv("GOEXPERIMENT"))
3734
fmt.Fprintf(&buf, "const StackGuardMultiplier = %d\n", stackGuardMultiplier())
@@ -71,7 +68,6 @@ func mkzbootstrap(file string) {
7168
fmt.Fprintln(&buf)
7269
fmt.Fprintf(&buf, "import \"runtime\"\n")
7370
fmt.Fprintln(&buf)
74-
fmt.Fprintf(&buf, "const defaultGOROOT = `%s`\n", goroot_final)
7571
fmt.Fprintf(&buf, "const defaultGO386 = `%s`\n", go386)
7672
fmt.Fprintf(&buf, "const defaultGOARM = `%s`\n", goarm)
7773
fmt.Fprintf(&buf, "const defaultGOMIPS = `%s`\n", gomips)

src/cmd/dist/test.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,13 @@ func (t *tester) run() {
172172
return
173173
}
174174

175-
// we must unset GOROOT_FINAL before tests, because runtime/debug requires
175+
// We must unset GOROOT_FINAL before tests, because runtime/debug requires
176176
// correct access to source code, so if we have GOROOT_FINAL in effect,
177177
// at least runtime/debug test will fail.
178+
// If GOROOT_FINAL was set before, then now all the commands will appear stale.
179+
// Nothing we can do about that other than not checking them below.
180+
// (We call checkNotStale but only with "std" not "cmd".)
181+
os.Setenv("GOROOT_FINAL_OLD", os.Getenv("GOROOT_FINAL")) // for cmd/link test
178182
os.Unsetenv("GOROOT_FINAL")
179183

180184
for _, name := range t.runNames {
@@ -1044,7 +1048,7 @@ func (t *tester) cgoTest(dt *distTest) error {
10441048
// running in parallel with earlier tests, or if it has some other reason
10451049
// for needing the earlier tests to be done.
10461050
func (t *tester) runPending(nextTest *distTest) {
1047-
checkNotStale("go", "std", "cmd")
1051+
checkNotStale("go", "std")
10481052
worklist := t.worklist
10491053
t.worklist = nil
10501054
for _, w := range worklist {
@@ -1097,7 +1101,7 @@ func (t *tester) runPending(nextTest *distTest) {
10971101
log.Printf("Failed: %v", w.err)
10981102
t.failed = true
10991103
}
1100-
checkNotStale("go", "std", "cmd")
1104+
checkNotStale("go", "std")
11011105
}
11021106
if t.failed && !t.keepGoing {
11031107
log.Fatal("FAILED")

src/cmd/go/go_test.go

+5-16
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ func TestMain(m *testing.M) {
102102
fmt.Printf("SKIP\n")
103103
return
104104
}
105+
os.Unsetenv("GOROOT_FINAL")
105106

106107
if canRun {
107108
args := []string{"build", "-tags", "testgo", "-o", "testgo" + exeSuffix}
@@ -4511,19 +4512,9 @@ func TestExecutableGOROOT(t *testing.T) {
45114512
newRoot := tg.path("new")
45124513

45134514
t.Run("RelocatedExe", func(t *testing.T) {
4514-
t.Skip("TODO: skipping known broken test; see golang.org/issue/20284")
4515-
4516-
// Should fall back to default location in binary.
4517-
// No way to dig out other than look at source code.
4518-
data, err := ioutil.ReadFile("../../runtime/internal/sys/zversion.go")
4519-
if err != nil {
4520-
t.Fatal(err)
4521-
}
4522-
m := regexp.MustCompile("var DefaultGoroot = `([^`]+)`").FindStringSubmatch(string(data))
4523-
if m == nil {
4524-
t.Fatal("cannot find DefaultGoroot in ../../runtime/internal/sys/zversion.go")
4525-
}
4526-
check(t, newGoTool, m[1])
4515+
// Should fall back to default location in binary,
4516+
// which is the GOROOT we used when building testgo.exe.
4517+
check(t, newGoTool, testGOROOT)
45274518
})
45284519

45294520
// If the binary is sitting in a bin dir next to ../pkg/tool, that counts as a GOROOT,
@@ -4548,9 +4539,7 @@ func TestExecutableGOROOT(t *testing.T) {
45484539
tg.must(os.RemoveAll(tg.path("new/pkg")))
45494540

45504541
// Binaries built in the new tree should report the
4551-
// new tree when they call runtime.GOROOT().
4552-
// This is implemented by having the go tool pass a -X option
4553-
// to the linker setting runtime/internal/sys.DefaultGoroot.
4542+
// new tree when they call runtime.GOROOT.
45544543
t.Run("RuntimeGoroot", func(t *testing.T) {
45554544
// Build a working GOROOT the easy way, with symlinks.
45564545
testenv.MustHaveSymlink(t)

src/cmd/go/internal/cfg/cfg.go

+14-5
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,12 @@ func init() {
7676
}
7777

7878
var (
79-
GOROOT = findGOROOT()
80-
GOBIN = os.Getenv("GOBIN")
81-
GOROOTbin = filepath.Join(GOROOT, "bin")
82-
GOROOTpkg = filepath.Join(GOROOT, "pkg")
83-
GOROOTsrc = filepath.Join(GOROOT, "src")
79+
GOROOT = findGOROOT()
80+
GOBIN = os.Getenv("GOBIN")
81+
GOROOTbin = filepath.Join(GOROOT, "bin")
82+
GOROOTpkg = filepath.Join(GOROOT, "pkg")
83+
GOROOTsrc = filepath.Join(GOROOT, "src")
84+
GOROOT_FINAL = findGOROOT_FINAL()
8485

8586
// Used in envcmd.MkEnv and build ID computations.
8687
GOARM = fmt.Sprint(objabi.GOARM)
@@ -129,6 +130,14 @@ func findGOROOT() string {
129130
return def
130131
}
131132

133+
func findGOROOT_FINAL() string {
134+
def := GOROOT
135+
if env := os.Getenv("GOROOT_FINAL"); env != "" {
136+
def = filepath.Clean(env)
137+
}
138+
return def
139+
}
140+
132141
// isSameDir reports whether dir1 and dir2 are the same directory.
133142
func isSameDir(dir1, dir2 string) bool {
134143
if dir1 == dir2 {

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

+2-9
Original file line numberDiff line numberDiff line change
@@ -790,15 +790,8 @@ func (b *Builder) printLinkerConfig(h io.Writer, p *load.Package) {
790790
}
791791
fmt.Fprintf(h, "GO$GOARCH=%s\n", os.Getenv("GO"+strings.ToUpper(cfg.BuildContext.GOARCH))) // GO386, GOARM, etc
792792

793-
/*
794-
// TODO(rsc): Enable this code.
795-
// golang.org/issue/22475.
796-
goroot := cfg.BuildContext.GOROOT
797-
if final := os.Getenv("GOROOT_FINAL"); final != "" {
798-
goroot = final
799-
}
800-
fmt.Fprintf(h, "GOROOT=%s\n", goroot)
801-
*/
793+
// The linker writes source file paths that say GOROOT_FINAL.
794+
fmt.Fprintf(h, "GOROOT=%s\n", cfg.GOROOT_FINAL)
802795

803796
// TODO(rsc): Convince linker team not to add more magic environment variables,
804797
// or perhaps restrict the environment variables passed to subprocesses.

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

-5
Original file line numberDiff line numberDiff line change
@@ -418,11 +418,6 @@ func (gcToolchain) ld(b *Builder, root *Action, out, importcfg, mainpkg string)
418418
ldflags = append(ldflags, "-pluginpath", pluginPath(root))
419419
}
420420

421-
// TODO(rsc): This is probably wrong - see golang.org/issue/22155.
422-
if cfg.GOROOT != runtime.GOROOT() {
423-
ldflags = append(ldflags, "-X=runtime/internal/sys.DefaultGoroot="+cfg.GOROOT)
424-
}
425-
426421
// Store BuildID inside toolchain binaries as a unique identifier of the
427422
// tool being run, for use by content-based staleness determination.
428423
if root.Package.Goroot && strings.HasPrefix(root.Package.ImportPath, "cmd/") {

src/cmd/internal/objabi/util.go

+2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ func envOr(key, value string) string {
1919
}
2020

2121
var (
22+
defaultGOROOT string // set by linker
23+
2224
GOROOT = envOr("GOROOT", defaultGOROOT)
2325
GOARCH = envOr("GOARCH", defaultGOARCH)
2426
GOOS = envOr("GOOS", defaultGOOS)

src/cmd/link/dwarf_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ func TestDWARF(t *testing.T) {
3232
t.Fatalf("go list: %v\n%s", err, out)
3333
}
3434
if string(out) != "false\n" {
35+
if os.Getenv("GOROOT_FINAL_OLD") != "" {
36+
t.Skip("cmd/link is stale, but $GOROOT_FINAL_OLD is set")
37+
}
3538
t.Fatalf("cmd/link is stale - run go install cmd/link")
3639
}
3740

src/cmd/link/internal/ld/main.go

+4
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ func Main(arch *sys.Arch, theArch Arch) {
110110
}
111111
}
112112

113+
final := gorootFinal()
114+
addstrdata1(ctxt, "runtime/internal/sys.DefaultGoroot="+final)
115+
addstrdata1(ctxt, "cmd/internal/objabi.defaultGOROOT="+final)
116+
113117
// TODO(matloob): define these above and then check flag values here
114118
if ctxt.Arch.Family == sys.AMD64 && objabi.GOOS == "plan9" {
115119
flag.BoolVar(&Flag8, "8", false, "use 64-bit addresses in symbol table")

src/cmd/link/internal/ld/pcln.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -421,14 +421,18 @@ func (ctxt *Link) pclntab() {
421421
}
422422
}
423423

424+
func gorootFinal() string {
425+
root := objabi.GOROOT
426+
if final := os.Getenv("GOROOT_FINAL"); final != "" {
427+
root = final
428+
}
429+
return root
430+
}
431+
424432
func expandGoroot(s string) string {
425433
const n = len("$GOROOT")
426434
if len(s) >= n+1 && s[:n] == "$GOROOT" && (s[n] == '/' || s[n] == '\\') {
427-
root := objabi.GOROOT
428-
if final := os.Getenv("GOROOT_FINAL"); final != "" {
429-
root = final
430-
}
431-
return filepath.ToSlash(filepath.Join(root, s[n:]))
435+
return filepath.ToSlash(filepath.Join(gorootFinal(), s[n:]))
432436
}
433437
return s
434438
}

src/runtime/internal/sys/stubs.go

+2
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,5 @@ package sys
99
const PtrSize = 4 << (^uintptr(0) >> 63) // unsafe.Sizeof(uintptr(0)) but an ideal const
1010
const RegSize = 4 << (^Uintreg(0) >> 63) // unsafe.Sizeof(uintreg(0)) but an ideal const
1111
const SpAlign = 1*(1-GoarchArm64) + 16*GoarchArm64 // SP alignment: 1 normally, 16 for ARM64
12+
13+
var DefaultGoroot string // set at link time

0 commit comments

Comments
 (0)