Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/link: Mismatch between hashed and actual linker config prevents reproducible builds #38989

Open
lxop opened this issue May 10, 2020 · 9 comments
Milestone

Comments

@lxop
Copy link

@lxop lxop commented May 10, 2020

What version of Go are you using (go version)?

$ go version
go version go1.13.8 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/alex/.cache/go-build"
GOENV="/home/alex/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/alex/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build419143820=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Built the same application multiple times with go in different locations, using the -trimpath flag.

What did you expect to see?

The same binary produced in each case (bit-for-bit equal).

What did you see instead?

The application is almost bit-for-bit equal, with the only difference being the first section of the buildID.

Based on the underlying cause below, setting GOROOT_FINAL for the application build (not for building the compiler) avoids this problem. However this is just a workaround, as my understanding is that GOROOT_FINAL is only supposed to be used for building the compiler.

The cause

With some rudimentary debugging, I have tracked this down to a difference in the linker configuration injected into the actionID hash by printLinkerConfig(), in particular the GOROOT=... line. This line reports the value of GOROOT_FINAL, however, with the -trimpath commandline flag set, the linker does not in fact use that GOROOT_FINAL value (see gc.go line 553 in go1.13.8). This problem could be fixed by applying the same logic (as in ld() in gc.go) to printLinkerConfig().

@lxop
Copy link
Author

@lxop lxop commented May 10, 2020

Suggested patch (from current master):

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 6ab3498c3e..b5d4a3c171 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -1161,8 +1161,13 @@ func (b *Builder) printLinkerConfig(h io.Writer, p *load.Package) {
 		key, val := cfg.GetArchEnv()
 		fmt.Fprintf(h, "%s=%s\n", key, val)
 
-		// The linker writes source file paths that say GOROOT_FINAL.
-		fmt.Fprintf(h, "GOROOT=%s\n", cfg.GOROOT_FINAL)
+		// The linker writes source file paths that say GOROOT_FINAL, but 
+		// only if -trimpath is not specified (see ld() in gc.go).
+		if cfg.BuildTrimpath {
+			fmt.Fprintf(h, "GOROOT=go\n")
+		} else {
+			fmt.Fprintf(h, "GOROOT=%s\n", cfg.GOROOT_FINAL)
+		}
 
 		// GO_EXTLINK_ENABLED controls whether the external linker is used.
 		fmt.Fprintf(h, "GO_EXTLINK_ENABLED=%s\n", cfg.Getenv("GO_EXTLINK_ENABLED"))
diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go
index 318d688d2e..55903ae27c 100644
--- a/src/cmd/go/internal/work/gc.go
+++ b/src/cmd/go/internal/work/gc.go
@@ -560,6 +560,7 @@ func (gcToolchain) ld(b *Builder, root *Action, out, importcfg, mainpkg string)
 
 	env := []string{}
 	if cfg.BuildTrimpath {
+		// Keep this up-to-date in printLinkerConfig() in exec.go.
 		env = append(env, "GOROOT_FINAL=go")
 	}
 	return b.run(root, dir, root.Package.ImportPath, env, cfg.BuildToolexec, base.Tool("link"), "-o", out, "-importcfg", importcfg, ldflags, mainpkg)
@toothrot toothrot changed the title Mismatch between hashed and actual linker config prevents reproducible builds cmd/link: Mismatch between hashed and actual linker config prevents reproducible builds May 11, 2020
@toothrot toothrot added this to the Backlog milestone May 11, 2020
@toothrot
Copy link
Contributor

@toothrot toothrot commented May 11, 2020

@thanm
Copy link
Member

@thanm thanm commented May 11, 2020

I think this is a cmd/go issue, not a cmd/link issue (please correct me if I am wrong here).

Also, I note that this is with go1.13.8 -- there have been a fair number of changes to the Go command since then -- does this problem also occur for Go 1.14?

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented May 11, 2020

cc @jayconrod

I think @jayconrod has been looking at -trimpath recently.

@ALTree
Copy link
Member

@ALTree ALTree commented May 11, 2020

#36118 ?

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented May 11, 2020

This does seem like #36118 (thanks @ALTree for finding that). There are a number of issues that make builds not fully reproducible, but I think this is the main one that affects pure Go builds.

I'm not sure whether the suggested patch would completely mitigate this issue. Seems like it might, but it's been a while since I looked at this.

@lxop
Copy link
Author

@lxop lxop commented May 11, 2020

@thanm Yes this still exists in Go 1.14 - the suggested patch is against master (as of yesterday).

@jayconrod There are certainly plenty of facets to fully reproducible builds. However this particular code is simply a mismatch between what is reported to be used (reported to the build ID hash) and what is actually used by the linker. As such, the fix simply makes the hashed path consistent with reality, and does indeed help towards reproducible builds (it was the only thing left in the way in my situation), so seems to me like a valid and valuable fix even if it doesn't fix all cases.

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented May 11, 2020

@lxop Sure, I didn't mean to argue this wasn't valid or valuable; as long as GOROOT_FINAL is around, we should account for it correctly in cache keys.

Do you want to submit a CL for this? I'd be happy to review later this week. I think you'll probably just need a test in src/cmd/go/testdata/script.

@lxop
Copy link
Author

@lxop lxop commented May 12, 2020

OK I'll look at getting one together some time soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.