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/go: cgo #line directives cause non-reproducibility #36072

Open
jayconrod opened this issue Dec 10, 2019 · 7 comments
Open

cmd/go: cgo #line directives cause non-reproducibility #36072

jayconrod opened this issue Dec 10, 2019 · 7 comments
Assignees
Labels
Milestone

Comments

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Dec 10, 2019

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

$ go version
go version go1.13.5 darwin/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="/Users/jayconrod/Library/Caches/go-build"
GOENV="/Users/jayconrod/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jayconrod/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/go/installed"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/go/installed/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/jayconrod/Code/test/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/rq/x0692kqj6ml8cvrhcqh5bswc008xj1/T/go-build209284358=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Build an executable that depends on one or more cgo packages using -trimpath.

This can be reproduced by adding some cgo code to the program in build_trimpath.txt, the test case that covers -trimpath.

diff --git a/src/cmd/go/testdata/script/build_trimpath.txt b/src/cmd/go/testdata/script/build_trimpath.txt
index cfab80743e..363a01f156 100644
--- a/src/cmd/go/testdata/script/build_trimpath.txt
+++ b/src/cmd/go/testdata/script/build_trimpath.txt
@@ -100,6 +100,9 @@ import (
 	"strings"
 )
 
+// const int x = 42;
+import "C"
+
 func main() {
 	exe := os.Args[1]
 	data, err := ioutil.ReadFile(exe)

Also observe that strings $(go env GOROOT)/pkg/darwin_amd64/os/user.a prints file names in random temporary directories under /private/var, indicating these paths weren't completely scrubbed from standard library packages that include cgo.

Below is a longer test case. It builds a small cgo program, then lists file names in the DWARF data in the executable. The working directory should not be listed (but it is).

go build -trimpath -o hello .
go run dwarf-list-files.go hello
! stdout $WORK

-- go.mod --
module example.com/go-reproducibility

go 1.13

-- hello.go --
package main

// const int x = 42;
import "C"
import "fmt"

func main() {
	fmt.Println(C.x)
}

-- dwarf-list-files.go --
// +build tool

package main

import (
	"debug/dwarf"
	"debug/macho"
	"fmt"
	"io"
	"log"
	"os"
	"sort"
)

func main() {
	if len(os.Args) != 2 {
		fmt.Fprintf(os.Stderr, "usage: dwarf-list-files file\n")
		os.Exit(1)
	}
	files, err := run(os.Args[1])
	if err != nil {
		log.Fatal(err)
	}
	for _, file := range files {
		fmt.Println(file)
	}
}

func run(path string) ([]string, error) {
	machoFile, err := macho.Open(path)
	if err != nil {
		return nil, err
	}
	defer machoFile.Close()

	dwarfData, err := machoFile.DWARF()
	if err != nil {
		return nil, err
	}

	dwarfReader := dwarfData.Reader()
	files := make(map[string]bool)
	for {
		e, err := dwarfReader.Next()
		if err != nil {
			return nil, err
		}
		if e == nil {
			break
		}
		lr, err := dwarfData.LineReader(e)
		if err != nil {
			return nil, err
		}
		if lr == nil {
			continue
		}

		var le dwarf.LineEntry
		for {
			if err := lr.Next(&le); err != nil {
				if err == io.EOF {
					break
				}
				return nil, err
			}
			files[le.File.Name] = true
		}
	}

	sortedFiles := make([]string, 0, len(files))
	for file := range files {
		sortedFiles = append(sortedFiles, file)
	}
	sort.Strings(sortedFiles)

	return sortedFiles, nil
}

What did you expect to see?

When the -trimpath flag is used, the package root directories (including GOROOT, GOPATH roots, and module roots) should not appear in binaries produced by the go command or in any intermediate file.

What did you see instead?

.c files created by cgo include #line directives that point back to the original source files. These .c files are intermediate files, so with -trimpath, they shouldn't include absolute paths. The paths should just be derived from the import path, the module path, and the version.

When we compile these files, we use -fdebug-prefix-map to strip the temporary working directory, but we don't use that flag to remove the source directory. If #line directives are correctly stripped, we don't need this for intermediate .c files, but we still need it for source .c files. Without this flag, absolute paths to the source directory end up in the DWARF data in the compiled .o files.

I don't know much about the DWARF format, but I think paths are compressed in the linked binaries we're producing. strings doesn't show any inappropriate source paths, but dwarf-list-files.go in the test case above does. Also, it seems like the linker has special handling for GOROOT_FINAL for binaries in GOROOT, but I'm not sure how that works.

@jayconrod jayconrod added this to the Go1.14 milestone Dec 10, 2019
@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Dec 10, 2019

cc @bcmills

Marking this as a release blocker for Go 1.14. I'd like to put in at least a partial fix before then.

This should not block the beta.

@jayconrod jayconrod self-assigned this Dec 10, 2019
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 10, 2019

Is this new? I thought this was fixed for most systems by #13247.

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Dec 10, 2019

@ianlancetaylor Thanks for pointing out #13247. That didn't come up in my search.

I don't think it's quite the same issue though. That one is about eliding the temporary work directory, which we're doing. This issue is about eliding the source directory.

In any case, I don't think this is a new issue. Though it might be considered a bug with -trimpath, added in 1.13.

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Dec 11, 2019

A closely related problem: generated cgo files are cached for each package, and when -trimpath is specified, the cache key is independent of the source directory, even though the generated cgo files are not. So when an identical package is built in two different source directories, the generated cgo files with #line directives pointing to one directory will be reused in the other directory.

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 11, 2019

Change https://golang.org/cl/210943 mentions this issue: cmd/go: trim source paths when compiling C files

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 19, 2019

Change https://golang.org/cl/212101 mentions this issue: cmd/go: trim source paths when compiling C with -trimpath

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Dec 19, 2019

CL 210943 seems too big for 1.14 at this point. It also doesn't completely solve the problem. We should try to make linked cgo binaries completely reproducible when the host linker supports that. For linkers that don't support it, we should understand why.

CL 212101 is a smaller fix that should mitigate the issue. It will pass -fdebug-prefix-map to the C compiler (for compilers that support it) when building C/C++/etc files (either from source or generated by cgo). Generated cgo files will still have #line directives, but they shouldn't leave source paths in compiled .o files or the linked binary.

Removed the release-blocker label after discussion with @bcmills. I'd like for this to be mitigated with CL 212101, but it isn't severe enough to block the release.

@toothrot toothrot modified the milestones: Go1.14, Go1.15 Feb 25, 2020
gopherbot pushed a commit that referenced this issue Apr 29, 2020
When then go command is run with -trimpath, it will now use
-fdebug-prefix-map when invoking the C compiler (if supported) to
replace the source root directory with a dummy root directory.

This should prevent source directories from appearing either literally
or in compressed DWARF in linked binaries.

Updates #36072

Change-Id: Iedd08d5e886f81e981f11248a1be4ed4f58bdd29
Reviewed-on: https://go-review.googlesource.com/c/go/+/212101
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@jayconrod jayconrod modified the milestones: Go1.15, Go1.16 May 1, 2020
xujianhai666 added a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
When then go command is run with -trimpath, it will now use
-fdebug-prefix-map when invoking the C compiler (if supported) to
replace the source root directory with a dummy root directory.

This should prevent source directories from appearing either literally
or in compressed DWARF in linked binaries.

Updates golang#36072

Change-Id: Iedd08d5e886f81e981f11248a1be4ed4f58bdd29
Reviewed-on: https://go-review.googlesource.com/c/go/+/212101
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
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
4 participants
You can’t perform that action at this time.