aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRuss Cox <rsc@golang.org>2018-01-08 11:59:29 -0500
committerRuss Cox <rsc@golang.org>2018-01-09 21:46:18 +0000
commit8396015e80afa4ddd4ea3c3f00373d22c3b45d6c (patch)
tree692f255ede9aa9d8b4d7ed4522ebe2dd86b2020a
parent28639df158a19b2bbceda43fb67ca7cb685d8e34 (diff)
downloadgo-8396015e80afa4ddd4ea3c3f00373d22c3b45d6c.tar.gz
go-8396015e80afa4ddd4ea3c3f00373d22c3b45d6c.zip
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>
-rw-r--r--src/cmd/dist/buildruntime.go4
-rw-r--r--src/cmd/dist/test.go10
-rw-r--r--src/cmd/go/go_test.go21
-rw-r--r--src/cmd/go/internal/cfg/cfg.go19
-rw-r--r--src/cmd/go/internal/work/exec.go11
-rw-r--r--src/cmd/go/internal/work/gc.go5
-rw-r--r--src/cmd/internal/objabi/util.go2
-rw-r--r--src/cmd/link/dwarf_test.go3
-rw-r--r--src/cmd/link/internal/ld/main.go4
-rw-r--r--src/cmd/link/internal/ld/pcln.go14
-rw-r--r--src/runtime/internal/sys/stubs.go2
11 files changed, 48 insertions, 47 deletions
diff --git a/src/cmd/dist/buildruntime.go b/src/cmd/dist/buildruntime.go
index 2f10fd0237..5cbcd8191b 100644
--- a/src/cmd/dist/buildruntime.go
+++ b/src/cmd/dist/buildruntime.go
@@ -18,7 +18,6 @@ import (
// mkzversion writes zversion.go:
//
// package sys
-// var DefaultGoroot = <goroot>
//
// const TheVersion = <version>
// const Goexperiment = <goexperiment>
@@ -30,8 +29,6 @@ func mkzversion(dir, file string) {
fmt.Fprintln(&buf)
fmt.Fprintf(&buf, "package sys\n")
fmt.Fprintln(&buf)
- fmt.Fprintf(&buf, "var DefaultGoroot = `%s`\n", goroot_final)
- fmt.Fprintln(&buf)
fmt.Fprintf(&buf, "const TheVersion = `%s`\n", findgoversion())
fmt.Fprintf(&buf, "const Goexperiment = `%s`\n", os.Getenv("GOEXPERIMENT"))
fmt.Fprintf(&buf, "const StackGuardMultiplier = %d\n", stackGuardMultiplier())
@@ -71,7 +68,6 @@ func mkzbootstrap(file string) {
fmt.Fprintln(&buf)
fmt.Fprintf(&buf, "import \"runtime\"\n")
fmt.Fprintln(&buf)
- fmt.Fprintf(&buf, "const defaultGOROOT = `%s`\n", goroot_final)
fmt.Fprintf(&buf, "const defaultGO386 = `%s`\n", go386)
fmt.Fprintf(&buf, "const defaultGOARM = `%s`\n", goarm)
fmt.Fprintf(&buf, "const defaultGOMIPS = `%s`\n", gomips)
diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index 44971ecf17..6d76209e5d 100644
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -172,9 +172,13 @@ func (t *tester) run() {
return
}
- // we must unset GOROOT_FINAL before tests, because runtime/debug requires
+ // We must unset GOROOT_FINAL before tests, because runtime/debug requires
// correct access to source code, so if we have GOROOT_FINAL in effect,
// at least runtime/debug test will fail.
+ // If GOROOT_FINAL was set before, then now all the commands will appear stale.
+ // Nothing we can do about that other than not checking them below.
+ // (We call checkNotStale but only with "std" not "cmd".)
+ os.Setenv("GOROOT_FINAL_OLD", os.Getenv("GOROOT_FINAL")) // for cmd/link test
os.Unsetenv("GOROOT_FINAL")
for _, name := range t.runNames {
@@ -1044,7 +1048,7 @@ func (t *tester) cgoTest(dt *distTest) error {
// running in parallel with earlier tests, or if it has some other reason
// for needing the earlier tests to be done.
func (t *tester) runPending(nextTest *distTest) {
- checkNotStale("go", "std", "cmd")
+ checkNotStale("go", "std")
worklist := t.worklist
t.worklist = nil
for _, w := range worklist {
@@ -1097,7 +1101,7 @@ func (t *tester) runPending(nextTest *distTest) {
log.Printf("Failed: %v", w.err)
t.failed = true
}
- checkNotStale("go", "std", "cmd")
+ checkNotStale("go", "std")
}
if t.failed && !t.keepGoing {
log.Fatal("FAILED")
diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index 371296c72e..83c126e11e 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -102,6 +102,7 @@ func TestMain(m *testing.M) {
fmt.Printf("SKIP\n")
return
}
+ os.Unsetenv("GOROOT_FINAL")
if canRun {
args := []string{"build", "-tags", "testgo", "-o", "testgo" + exeSuffix}
@@ -4511,19 +4512,9 @@ func TestExecutableGOROOT(t *testing.T) {
newRoot := tg.path("new")
t.Run("RelocatedExe", func(t *testing.T) {
- t.Skip("TODO: skipping known broken test; see golang.org/issue/20284")
-
- // Should fall back to default location in binary.
- // No way to dig out other than look at source code.
- data, err := ioutil.ReadFile("../../runtime/internal/sys/zversion.go")
- if err != nil {
- t.Fatal(err)
- }
- m := regexp.MustCompile("var DefaultGoroot = `([^`]+)`").FindStringSubmatch(string(data))
- if m == nil {
- t.Fatal("cannot find DefaultGoroot in ../../runtime/internal/sys/zversion.go")
- }
- check(t, newGoTool, m[1])
+ // Should fall back to default location in binary,
+ // which is the GOROOT we used when building testgo.exe.
+ check(t, newGoTool, testGOROOT)
})
// 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) {
tg.must(os.RemoveAll(tg.path("new/pkg")))
// Binaries built in the new tree should report the
- // new tree when they call runtime.GOROOT().
- // This is implemented by having the go tool pass a -X option
- // to the linker setting runtime/internal/sys.DefaultGoroot.
+ // new tree when they call runtime.GOROOT.
t.Run("RuntimeGoroot", func(t *testing.T) {
// Build a working GOROOT the easy way, with symlinks.
testenv.MustHaveSymlink(t)
diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go
index dfab20a8de..1de4f0dc79 100644
--- a/src/cmd/go/internal/cfg/cfg.go
+++ b/src/cmd/go/internal/cfg/cfg.go
@@ -76,11 +76,12 @@ func init() {
}
var (
- GOROOT = findGOROOT()
- GOBIN = os.Getenv("GOBIN")
- GOROOTbin = filepath.Join(GOROOT, "bin")
- GOROOTpkg = filepath.Join(GOROOT, "pkg")
- GOROOTsrc = filepath.Join(GOROOT, "src")
+ GOROOT = findGOROOT()
+ GOBIN = os.Getenv("GOBIN")
+ GOROOTbin = filepath.Join(GOROOT, "bin")
+ GOROOTpkg = filepath.Join(GOROOT, "pkg")
+ GOROOTsrc = filepath.Join(GOROOT, "src")
+ GOROOT_FINAL = findGOROOT_FINAL()
// Used in envcmd.MkEnv and build ID computations.
GOARM = fmt.Sprint(objabi.GOARM)
@@ -129,6 +130,14 @@ func findGOROOT() string {
return def
}
+func findGOROOT_FINAL() string {
+ def := GOROOT
+ if env := os.Getenv("GOROOT_FINAL"); env != "" {
+ def = filepath.Clean(env)
+ }
+ return def
+}
+
// isSameDir reports whether dir1 and dir2 are the same directory.
func isSameDir(dir1, dir2 string) bool {
if dir1 == dir2 {
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index c5f0eb70bf..195437d220 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -790,15 +790,8 @@ func (b *Builder) printLinkerConfig(h io.Writer, p *load.Package) {
}
fmt.Fprintf(h, "GO$GOARCH=%s\n", os.Getenv("GO"+strings.ToUpper(cfg.BuildContext.GOARCH))) // GO386, GOARM, etc
- /*
- // TODO(rsc): Enable this code.
- // golang.org/issue/22475.
- goroot := cfg.BuildContext.GOROOT
- if final := os.Getenv("GOROOT_FINAL"); final != "" {
- goroot = final
- }
- fmt.Fprintf(h, "GOROOT=%s\n", goroot)
- */
+ // The linker writes source file paths that say GOROOT_FINAL.
+ fmt.Fprintf(h, "GOROOT=%s\n", cfg.GOROOT_FINAL)
// TODO(rsc): Convince linker team not to add more magic environment variables,
// or perhaps restrict the environment variables passed to subprocesses.
diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go
index d3bded6989..71b5337939 100644
--- a/src/cmd/go/internal/work/gc.go
+++ b/src/cmd/go/internal/work/gc.go
@@ -418,11 +418,6 @@ func (gcToolchain) ld(b *Builder, root *Action, out, importcfg, mainpkg string)
ldflags = append(ldflags, "-pluginpath", pluginPath(root))
}
- // TODO(rsc): This is probably wrong - see golang.org/issue/22155.
- if cfg.GOROOT != runtime.GOROOT() {
- ldflags = append(ldflags, "-X=runtime/internal/sys.DefaultGoroot="+cfg.GOROOT)
- }
-
// Store BuildID inside toolchain binaries as a unique identifier of the
// tool being run, for use by content-based staleness determination.
if root.Package.Goroot && strings.HasPrefix(root.Package.ImportPath, "cmd/") {
diff --git a/src/cmd/internal/objabi/util.go b/src/cmd/internal/objabi/util.go
index f8949e05a2..eafef6bfa7 100644
--- a/src/cmd/internal/objabi/util.go
+++ b/src/cmd/internal/objabi/util.go
@@ -19,6 +19,8 @@ func envOr(key, value string) string {
}
var (
+ defaultGOROOT string // set by linker
+
GOROOT = envOr("GOROOT", defaultGOROOT)
GOARCH = envOr("GOARCH", defaultGOARCH)
GOOS = envOr("GOOS", defaultGOOS)
diff --git a/src/cmd/link/dwarf_test.go b/src/cmd/link/dwarf_test.go
index f88aecc7c7..2b3771eabc 100644
--- a/src/cmd/link/dwarf_test.go
+++ b/src/cmd/link/dwarf_test.go
@@ -32,6 +32,9 @@ func TestDWARF(t *testing.T) {
t.Fatalf("go list: %v\n%s", err, out)
}
if string(out) != "false\n" {
+ if os.Getenv("GOROOT_FINAL_OLD") != "" {
+ t.Skip("cmd/link is stale, but $GOROOT_FINAL_OLD is set")
+ }
t.Fatalf("cmd/link is stale - run go install cmd/link")
}
diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go
index 4212562677..f86abbc6a6 100644
--- a/src/cmd/link/internal/ld/main.go
+++ b/src/cmd/link/internal/ld/main.go
@@ -110,6 +110,10 @@ func Main(arch *sys.Arch, theArch Arch) {
}
}
+ final := gorootFinal()
+ addstrdata1(ctxt, "runtime/internal/sys.DefaultGoroot="+final)
+ addstrdata1(ctxt, "cmd/internal/objabi.defaultGOROOT="+final)
+
// TODO(matloob): define these above and then check flag values here
if ctxt.Arch.Family == sys.AMD64 && objabi.GOOS == "plan9" {
flag.BoolVar(&Flag8, "8", false, "use 64-bit addresses in symbol table")
diff --git a/src/cmd/link/internal/ld/pcln.go b/src/cmd/link/internal/ld/pcln.go
index b954c05c81..9d82677059 100644
--- a/src/cmd/link/internal/ld/pcln.go
+++ b/src/cmd/link/internal/ld/pcln.go
@@ -421,14 +421,18 @@ func (ctxt *Link) pclntab() {
}
}
+func gorootFinal() string {
+ root := objabi.GOROOT
+ if final := os.Getenv("GOROOT_FINAL"); final != "" {
+ root = final
+ }
+ return root
+}
+
func expandGoroot(s string) string {
const n = len("$GOROOT")
if len(s) >= n+1 && s[:n] == "$GOROOT" && (s[n] == '/' || s[n] == '\\') {
- root := objabi.GOROOT
- if final := os.Getenv("GOROOT_FINAL"); final != "" {
- root = final
- }
- return filepath.ToSlash(filepath.Join(root, s[n:]))
+ return filepath.ToSlash(filepath.Join(gorootFinal(), s[n:]))
}
return s
}
diff --git a/src/runtime/internal/sys/stubs.go b/src/runtime/internal/sys/stubs.go
index 0a94502f32..5328023268 100644
--- a/src/runtime/internal/sys/stubs.go
+++ b/src/runtime/internal/sys/stubs.go
@@ -9,3 +9,5 @@ package sys
const PtrSize = 4 << (^uintptr(0) >> 63) // unsafe.Sizeof(uintptr(0)) but an ideal const
const RegSize = 4 << (^Uintreg(0) >> 63) // unsafe.Sizeof(uintreg(0)) but an ideal const
const SpAlign = 1*(1-GoarchArm64) + 16*GoarchArm64 // SP alignment: 1 normally, 16 for ARM64
+
+var DefaultGoroot string // set at link time