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/asm: properly handle file scoped symbol names appearing in multiple files of a package #18225

Closed
calmh opened this issue Dec 7, 2016 · 9 comments
Milestone

Comments

@calmh
Copy link
Contributor

calmh commented Dec 7, 2016

In go1.8beta, on Linux & Darwin amd64, the package github.com/minio/sha256-simd fails to build:

jb@unu:~ $ go get github.com/minio/sha256-simd
# github.com/minio/sha256-simd
duplicate constants
asm: symbol constants listed multiple times

The error message is initially a little confusing due to the name of the symbol, but the cause seems to be a GLOBL declaration that is the same in multiple files:

jb@unu:~/s/g/m/sha256-simd $ git grep 'GLOBL constants'
sha256blockAvx_amd64.s:GLOBL constants<>(SB), 8, $256
sha256blockSsse_amd64.s:GLOBL constants<>(SB), 8, $256

This was apparently fine in 1.7, and the issue on the other side minio/sha256-simd#11 was closed. I don't have a real opinion on which side of the fence the bug is on, if there is bug, but it would be good to get it clarified so we can build with 1.8beta. :)

@minux
Copy link
Member

minux commented Dec 7, 2016 via email

@minux
Copy link
Member

minux commented Dec 7, 2016 via email

@0xmohit
Copy link
Contributor

0xmohit commented Dec 7, 2016

I suspect it's because Go 1.8 cmd/asm compiles multiple assembly files together.

cmd/asm is now invoked only once per package (CL 27636). Also see #15680.

@minux
Copy link
Member

minux commented Dec 7, 2016 via email

@minux minux changed the title go1.8beta: asm "GLOBL" directive changes scope? cmd/asm: properly handle file scoped symbol names appearing in multiple files of a package Dec 7, 2016
@minux minux added this to the Go1.8 milestone Dec 7, 2016
@minux
Copy link
Member

minux commented Dec 7, 2016 via email

@bradfitz
Copy link
Contributor

bradfitz commented Dec 7, 2016

/cc @josharian

@josharian
Copy link
Contributor

Sounds like the right thing to do is add proper file-scoped symbol support to cmd/asm, including mangling, regardless of CL 27636. I don't know how hard or risky that is. In the meantime, if it helps enough, we can certainly revert CL 27636--it is for toolspeed only--and try it again in 1.9 after fixing cmd/asm.

FiloSottile added a commit to cloudflare/go that referenced this issue Dec 12, 2016
This reverts commit 9741d83.

Workaround golang/go#18225

Change-Id: I69322ae988b58e381213eb9cb93c5497004872e1
@bradfitz
Copy link
Contributor

Decision: let's just roll back the cmd/go part for Go 1.8 to call asm multiple times.

We'll fix it properly for Go 1.9.

@gopherbot
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants