-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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: external linking can fail on Solaris 11.2+ #14957
Comments
Should I open a separate issue for the compile failure in |
@jtsylve Good idea, thanks. |
This is a regression compared to 1.5; I will add this to the list of things to investigate. |
CL https://golang.org/cl/21392 mentions this issue. |
Fixes a problem when using the external linker on Solaris. The Solaris external linker still doesn't work due to issue #14957. The problem is, for example, with `go test cmd/objdump`: objdump_test.go:71: go build fmthello.go: exit status 2 # command-line-arguments /var/gcc/iant/go/pkg/tool/solaris_amd64/link: running gcc failed: exit status 1 Undefined first referenced symbol in file x_cgo_callers /tmp/go-link-355600608/go.o ld: fatal: symbol referencing errors collect2: error: ld returned 1 exit status Change-Id: I54917cfd5c288ee77ea25c439489bd2c9124fe73 Reviewed-on: https://go-review.googlesource.com/21392 Run-TryBot: Ian Lance Taylor <[email protected]> Reviewed-by: David Crawshaw <[email protected]>
Might it be wise to add a Sun Solaris build bot? I'm not sure which distro the two bots are currently running, but it's surprising that the automated build tests didn't catch this regression. |
@jtsylve, are you volunteering to run it or help set it up? Both the current ones are SmartOS I think. |
Shoot me some info about the process and I might be able to help -- depending on how much work it involves. I'm currently on paternity leave, so I have my hands a little full these days, but I could eventually get there. |
Unfortunately, Go just happens to be the victim of a bug in the Solaris 11.2 and later linker. This issue is also trivially reproducible using gcc alone without Go:
The reason we are seeing this issue with Go is due to the changes applied for issue #13247 which added this seemingly innocuous logic to src/cmd/link/internal/ld/symtab.go:
This adds a NULL STT_FILE entry to the symtab; this seems to be allowed per the System V ABI, so probably is a valid thing to do. However, this in turn triggers a silent bug that has existed for a few years (since sometime in the Solaris 11.2 release cycle) in the Solaris linker; ld(1)'s local symbol counting is being thrown off in some manner. This in turn trips up logic added during the Solaris 11.2 release cycle to ensure that elf output sections are in the expected order:
In versions of Solaris that have this functionality, this can be worked around by using "-z relax=secadj". However, the simplest workaround is likely to simply have STT_FILE use a name of some sort:
...which avoids the issue. This issue will be resolved in a future update to Solaris, but a workaround seems appropriate. I will happily contribute a CL based on whatever the preferred solution is. |
That workaround sounds OK to me. |
I think the "-z relax=secadj" workaround is more preferable.
|
@minux why? that involves the linker working out what version of solaris it is on. |
@minux I did not choose the path of using "-z relax-secadj" for the following reasons:
|
Will setting the STT_FILE symbol table entry interfere with debuggers?
|
@minux not as far as I'm aware, and not in testing; at most, I suppose it might cause a warning, but I haven't encountered a case where it will |
@bradfitz I'd like to see it pass on a builder too, but I can't solve that problem at the moment; the best I can do is provide output of test results from a Solaris box and ensure that it hasn't broken any existing builders, as has been the case for all of my contributions thus far. Thanks. |
Can someone with editing privileges please update the bug title to: cmd/link: external linking can fail on Solaris 11.2+ ...so that it accurately reflects the issue? |
CL https://golang.org/cl/21636 mentions this issue. |
Patch is currently listed as DO NOT SUBMIT. Test builder is up and running now. Is there anything else we're waiting for? |
I expect to receive approval from my employer to submit my patches Monday. |
CL https://golang.org/cl/24461 mentions this issue. |
go version
)?go version devel +ba333a3 Fri Mar 25 01:09:28 2016 +0000 solaris/amd64
go env
)?SunOS solaris 5.11 11.3 i86pc i386 i86pc
GCC:
Bootstrapping with Go 1.4.3
The text was updated successfully, but these errors were encountered: