cmd/asm: properly handle file scoped symbol names appearing in multiple files of a package

[calmh]

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]

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

[minux]

A minimal test case: $ mkdir t $ cd t $ cat > a.s <<EOF DATA ·constants<>+0x0(SB)/8,$0 GLOBL ·constants<>(SB),8,$8 EOF $ cp a.s b.s $ echo 'package t' > a.go $ go build # go tip # _/tmp/t duplicate "".constants asm: symbol "".constants listed multiple times $ go1.7 build # Go 1.7 $ Notice that a.s and b.s are using file scoped names, so cmd/asm shouldn't reject them.

[0xmohit]

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]

Even though cmd/asm has properly separated lexer and parser for each input file. The files are sharing the same cmd/internal/obj.Link in order to generate a single object file. Sharing Link means the files are sharing the same symbol table --- which shouldn't be a problem but cmd/asm didn't do anything to make file scoped symbols truly file scoped.

[minux]

A more comprehensive test case (for amd64): // t.go package main func A() int64 func B() int64 func main() { if A() != 1 || B() != 2 { panic("BUG") } } // a.s TEXT ·A(SB), 7, $0-8 MOVQ ·constants<>(SB), AX MOVQ AX, ret+0(FP) RET DATA ·constants<>+0x0(SB)/8,$1 GLOBL ·constants<>(SB),8,$8 // b.s TEXT ·B(SB), 7, $0-8 MOVQ ·constants<>(SB), AX MOVQ AX, ret+0(FP) RET DATA ·constants<>+0x0(SB)/8,$2 GLOBL ·constants<>(SB),8,$8 It passes under Go 1.7, but fails to build in Go 1.8. And if cmd/asm doesn't properly implement the semantics of file scoped symbols, it won't pass. As this is regression from Go 1.7, label this issue as Go1.8. There are multiple ways to fix this for Go 1.8, but I'm not sure if we want to preserve this property of Go 1.7: in the final binary, there are two symbols named "main.contants", that is, cmd/asm doesn't mangle the two symbols.

[bradfitz]

/cc @josharian

[josharian]

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.

[bradfitz]

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]

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