Skip to content

cmd/go: TestCgoConsistentResults fails with clang-3.9 #19964

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.

Sign up for GitHub

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

Jump to bottom
Closed
bradfitz opened this issue Apr 13, 2017 · 9 comments
Closed

cmd/go: TestCgoConsistentResults fails with clang-3.9 #19964

bradfitz opened this issue Apr 13, 2017 · 9 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.9

Comments

@bradfitz
Copy link
Contributor

@jessfraz and I upgraded the linux-*-clang builders for the first time in ages.

They now have clang-3.9 and now builds are failing with:

https://build.golang.org/log/e66348d8581d2f80e7d801961cf8814d9af1bed4

--- FAIL: TestCgoConsistentResults (4.47s)
	go_test.go:260: running testgo [build -o /tmp/gotest555473365/cgotest1 cgotest]
	go_test.go:260: running testgo [build -x -o /tmp/gotest555473365/cgotest2 cgotest]
	go_test.go:279: standard error:
	go_test.go:280: WORK=/tmp/go-build283999844
		mkdir -p $WORK/cgotest/_obj/
		mkdir -p $WORK/
		cd /tmp/workdir/go/src/cmd/go/testdata/src/cgotest
		CGO_LDFLAGS="-g" "-O2" /tmp/workdir/go/pkg/tool/linux_amd64/cgo -objdir $WORK/cgotest/_obj/ -importpath cgotest -- -I $WORK/cgotest/_obj/ -g -O2 m.go
		cd $WORK
		/usr/bin/clang -fdebug-prefix-map=a=b -c trivial.c
		/usr/bin/clang -gno-record-gcc-switches -c trivial.c
		cd /tmp/workdir/go/src/cmd/go/testdata/src/cgotest
		/usr/bin/clang -I . -fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=$WORK=/tmp/go-build -gno-record-gcc-switches -I $WORK/cgotest/_obj/ -g -O2 -o $WORK/cgotest/_obj/_cgo_export.o -c $WORK/cgotest/_obj/_cgo_export.c
		/usr/bin/clang -I . -fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=$WORK=/tmp/go-build -gno-record-gcc-switches -I $WORK/cgotest/_obj/ -g -O2 -o $WORK/cgotest/_obj/m.cgo2.o -c $WORK/cgotest/_obj/m.cgo2.c
		/usr/bin/clang -I . -fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=$WORK=/tmp/go-build -gno-record-gcc-switches -I $WORK/cgotest/_obj/ -g -O2 -o $WORK/cgotest/_obj/_cgo_main.o -c $WORK/cgotest/_obj/_cgo_main.c
		/usr/bin/clang -I . -fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=$WORK=/tmp/go-build -gno-record-gcc-switches -o $WORK/cgotest/_obj/_cgo_.o $WORK/cgotest/_obj/_cgo_main.o $WORK/cgotest/_obj/_cgo_export.o $WORK/cgotest/_obj/m.cgo2.o -g -O2
		/tmp/workdir/go/pkg/tool/linux_amd64/cgo -dynpackage cgotest -dynimport $WORK/cgotest/_obj/_cgo_.o -dynout $WORK/cgotest/_obj/_cgo_import.go
		cd $WORK
		/usr/bin/clang -no-pie -c trivial.c
		cd /tmp/workdir/go/src/cmd/go/testdata/src/cgotest
		/usr/bin/clang -I . -fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=$WORK=/tmp/go-build -gno-record-gcc-switches -o $WORK/cgotest/_obj/_all.o $WORK/cgotest/_obj/_cgo_export.o $WORK/cgotest/_obj/m.cgo2.o -g -O2 -Wl,-r -nostdlib -Wl,--build-id=none
		/tmp/workdir/go/pkg/tool/linux_amd64/compile -o $WORK/cgotest.a -trimpath $WORK -p cgotest -buildid da01d0527cc40b26d8afc2e889ecf3cc74ca325a -D _/tmp/workdir/go/src/cmd/go/testdata/src/cgotest -I $WORK -pack $WORK/cgotest/_obj/_cgo_gotypes.go $WORK/cgotest/_obj/m.cgo1.go $WORK/cgotest/_obj/_cgo_import.go
		pack r $WORK/cgotest.a $WORK/cgotest/_obj/_all.o # internal
		mkdir -p /tmp/gotest555473365/
		mv $WORK/cgotest.a /tmp/gotest555473365/cgotest2
		
	go_test.go:3398: building cgotest twice did not produce the same output
FAIL
FAIL	cmd/go	60.574s

/cc @ianlancetaylor
@bradfitz bradfitz added help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure. labels Apr 13, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Apr 13, 2017
@jessfraz
Copy link
Contributor

jessfraz commented Apr 13, 2017 via email

@jessfraz
Copy link
Contributor

ya I can reproduce with 3.9 only

@ianlancetaylor
Copy link
Member

CC @neild

@n
Copy link

n commented Apr 14, 2017 •
edited
Loading

Looks like clang-3.9 is including path information in the binary.

82,83c82,83
< /tmp/go-build662498961/cgotest/_obj/_cgo_export.c
< /tmp/go-build662498961/cgotest/_obj/m.cgo2.c
---
> /tmp/go-build714166219/cgotest/_obj/_cgo_export.c
> /tmp/go-build714166219/cgotest/_obj/m.cgo2.c

Stripping cgotest seems to resolve the issue.

3427c3427
< 	ldflags = append(ldflags, "-Wl,-s,-r", "-nostdlib")
---
> 	ldflags = append(ldflags, "-Wl,-r", "-nostdlib")

go test -run TestCgoConsistentResults go_test.go
ok  	command-line-arguments	23.184s

Add: Probably not the solution. Just more info on the problem.

@jessfraz
Copy link
Contributor

jessfraz commented Apr 14, 2017 via email

@amenzhinsky
Copy link

amenzhinsky commented Apr 17, 2017 •
edited
Loading

I think using the CGO_LDFLAGS env var would suffice. Changing default ldflags is not a good idea because we wouldn't be able to turn stripping down.

@bradfitz
Copy link
Contributor Author

We're using -fdebug-prefix-map though, right?

I checked that our image supports that flag, so this should be true:

        if b.gccSupportsFlag("-fdebug-prefix-map=a=b") {                                                             | 
                a = append(a, "-fdebug-prefix-map="+b.WorkDir+"=/tmp/go-build")                                      | 
        }                                                                                                            |

(Ignore the gcc part of the method name... it just checks the C compiler supporting the given flag.`

But you're saying that the linker is adding filenames?

Does the gcc linker not add filenames to its output? Which option is that? Maybe we're just missing the clang equivalent flags in the place where we ask gcc to not add filenames?

It seems that stripping the whole binary is overkill. Is that what we do with gcc?

/cc @ianlancetaylor

@ianlancetaylor
Copy link
Member

A filename that ends in .c would not be added by the linker. It's possible that -fdebug-prefix-map only affects debug info, and that this file name is appearing because the compiler is emitting a .file pseudo-op. In GCC -fdebug-prefix-map is applied to the .file pseudo-op, but perhaps in clang it is not. That's just a guess, though.

I only have clang 3.6 installed myself, and it does not seem to support -fdebug-prefix-map at all. According to a web search the option was added to clang in version 3.8. Note that the test always passes with a compiler that does not support that option--the test checks for that.

Hmmm, I just realized that I can use gomote to test clang 3.9. And I am correct: the -fdebug-prefix-map option is being applied to the debug info but not to the initial .file pseudo-op.

My only current thought for how to fix this is that instead of compiling $WORK/a/b/foo.c, we should change the command to (cd $WORK/a/b && clang -g -c foo.c). We'll still want -fdebug-prefix-map, I think, but that should fix the .file pseudo-op.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/41629 mentions this issue.

@golang golang locked and limited conversation to collaborators Apr 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
6 participants
@bradfitz @n @amenzhinsky @jessfraz @ianlancetaylor @gopherbot