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: Using cgo and quic-go under certain circumstance causes multiple definition errors #19198

Closed
crvv opened this issue Feb 20, 2017 · 12 comments

Comments

@crvv
Copy link
Contributor

crvv commented Feb 20, 2017

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

go version go1.8 windows/amd64
go version go1.7.5 windows/amd64
go version go1.7.1 windows/amd64

What operating system and processor architecture are you using (go env)?

Windows, AMD64
Only Windows. I tested on Linux and it works fine.

What did you do?

go build -gcflags '-N' the following code. I add -gcflag '-N' so I can paste shorter code here.

package main
import (
	"C"
	"net"
	"github.com/lucas-clemente/quic-go"
)
type server struct{}
func (s *server) Accept() (net.Conn, error) {
	quic.NewServer("", nil, nil)
	return nil, nil
}
func Listen() net.Listener {
	_ = &server{}
	return nil
}
func main() {
	Listen()
}

Almost everything in these codes is necessary to reproduce.

What did you expect to see?

Build successfully.

What did you see instead?

Errors:

multiple definition of `bswapMask'
multiple definition of `gcmPoly'
multiple definition of `andMask'
multiple definition of `_expand_key_128'
multiple definition of `_expand_key_192a'
multiple definition of `_expand_key_192b'
multiple definition of `_expand_key_256a'
multiple definition of `_expand_key_256b'

"github.com/lucas-clemente/quic-go" is a pure Go package, so I think this is a bug in Go code.

I reproduced it on AppVeyor at https://ci.appveyor.com/project/crvv/gocryptoaes.

@crvv crvv changed the title cmd/go: Using cgo and quic-go under certain circumstance cause multiple definition errors cmd/go: Using cgo and quic-go under certain circumstance causes multiple definition errors Feb 20, 2017
@ianlancetaylor
Copy link
Contributor

Can you show us the complete output of go build -x? I can't figure out where that error is coming from.

@crvv
Copy link
Contributor Author

crvv commented Feb 21, 2017

go build -gcflags '-N' -x

WORK=C:\Users\appveyor\AppData\Local\Temp\1\go-build125884458
mkdir -p $WORK\github.com\crvv\gocryptoaes\_obj\
mkdir -p $WORK\github.com\crvv\gocryptoaes\_obj\exe\
cd c:\gopath\src\github.com\crvv\gocryptoaes
CGO_LDFLAGS="-g" "-O2" "C:\\go\\pkg\\tool\\windows_amd64\\cgo.exe" -objdir "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\" -importpath github.com/crvv/gocryptoaes -- -I "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\" main.go
cd $WORK
gcc -fdebug-prefix-map=a=b -c trivial.c
gcc -gno-record-gcc-switches -c trivial.c
cd c:\gopath\src\github.com\crvv\gocryptoaes
gcc -I "c:\\gopath\\src\\github.com\\crvv\\gocryptoaes" -m64 -mthreads -fmessage-length=0 "-fdebug-prefix-map=C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458=/tmp/go-build" -gno-record-gcc-switches -I "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\" -g -O2 -o "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_main.o" -c "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_main.c"
gcc -I "c:\\gopath\\src\\github.com\\crvv\\gocryptoaes" -m64 -mthreads -fmessage-length=0 "-fdebug-prefix-map=C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458=/tmp/go-build" -gno-record-gcc-switches -I "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\" -g -O2 -o "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_export.o" -c "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_export.c"
gcc -I "c:\\gopath\\src\\github.com\\crvv\\gocryptoaes" -m64 -mthreads -fmessage-length=0 "-fdebug-prefix-map=C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458=/tmp/go-build" -gno-record-gcc-switches -I "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\" -g -O2 -o "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\main.cgo2.o" -c "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\main.cgo2.c"
gcc -I "c:\\gopath\\src\\github.com\\crvv\\gocryptoaes" -m64 -mthreads -fmessage-length=0 "-fdebug-prefix-map=C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458=/tmp/go-build" -gno-record-gcc-switches -o "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_.o" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_main.o" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_export.o" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\main.cgo2.o" -g -O2
"C:\\go\\pkg\\tool\\windows_amd64\\cgo.exe" -objdir "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\" -dynpackage main -dynimport "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_.o" -dynout "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_import.go"
cd $WORK
gcc -no-pie -c trivial.c
cd c:\gopath\src\github.com\crvv\gocryptoaes
gcc -I "c:\\gopath\\src\\github.com\\crvv\\gocryptoaes" -m64 -mthreads -fmessage-length=0 "-fdebug-prefix-map=C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458=/tmp/go-build" -gno-record-gcc-switches -o "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_all.o" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_export.o" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\main.cgo2.o" -g -O2 -Wl,-r -nostdlib -Wl,--start-group -lmingwex -lmingw32 -Wl,--end-group
"C:\\go\\pkg\\tool\\windows_amd64\\compile.exe" -o "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes.a" -trimpath "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458" -N -p main -buildid d3c3673e87947861accc557f4168a6aa8b84c3ae -D _/c_/gopath/src/github.com/crvv/gocryptoaes -I "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458" -I "c:\\gopath\\pkg\\windows_amd64" -pack "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_gotypes.go" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\main.cgo1.go" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_cgo_import.go"
pack r "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes.a" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\_all.o" # internal
cd .
"C:\\go\\pkg\\tool\\windows_amd64\\link.exe" -o "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes\\_obj\\exe\\a.out.exe" -L "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458" -L "c:\\gopath\\pkg\\windows_amd64" -extld=gcc -buildmode=exe -buildid=d3c3673e87947861accc557f4168a6aa8b84c3ae "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\go-build125884458\\github.com\\crvv\\gocryptoaes.a"
# github.com/crvv/gocryptoaes
C:\go\pkg\tool\windows_amd64\link.exe: running gcc failed: exit status 1
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x18bb80): multiple definition of `bswapMask'
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x18bb90): first defined here
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x18bbe0): multiple definition of `gcmPoly'
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x18bbd0): first defined here
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x18c700): multiple definition of `andMask'
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x18c600): first defined here
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x10ed60): multiple definition of `_expand_key_128'
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0xe5160): first defined here
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x10ed90): multiple definition of `_expand_key_192a'
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0xe5190): first defined here
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x10ede0): multiple definition of `_expand_key_192b'
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0xe51e0): first defined here
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x10ee20): multiple definition of `_expand_key_256a'
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0xe5220): first defined here
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0x10ee30): multiple definition of `_expand_key_256b'
C:\Users\appveyor\AppData\Local\Temp\1\go-link-608199587\go.o:(.text+0xe5230): first defined here
collect2.exe: error: ld returned 1 exit status

@alexbrainman
Copy link
Member

I can reproduce it here on windows-amd64 with gcc 4.9.1. For example, I can see 2 different "bswapMask" symbols in go.o:

c:\dev\src\issue19198>go build -gcflags "-N" -ldflags "-tmpdir=%TMP%" & objdump -t %TMP%\go.o | grep bswapMask
# issue19198
C:\dev\go\pkg\tool\windows_amd64\link.exe: running gcc failed: exit status 1
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0x1b3a00): multiple definition of `bswapMask'
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0x1b39f0): first defined here
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0x1b3bf0): multiple definition of `gcmPoly'
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0x1b3c00): first defined here
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0x1b4fe0): multiple definition of `andMask'
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0x1b4ee0): first defined here
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xf5cd0): multiple definition of `_expand_key_128'
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xbc9f0): first defined here
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xf5d00): multiple definition of `_expand_key_192a'
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xbca20): first defined here
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xf5d50): multiple definition of `_expand_key_192b'
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xbca70): first defined here
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xf5d90): multiple definition of `_expand_key_256a'
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xbcab0): first defined here
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xf5da0): multiple definition of `_expand_key_256b'
C:\Users\brainman\AppData\Local\Temp\go.o:(.text+0xbcac0): first defined here
c:/dev/mingw64_4.9.1/bin/../lib/gcc/x86_64-w64-mingw32/4.9.1/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\brainman\AppData\Local\Temp\go.o: bad reloc address 0x110 in section `.data'
collect2.exe: error: ld returned 1 exit status

[1852](sec  1)(fl 0x00)(ty   0)(scl   2) (nx 0) 0x00000000001b39f0 bswapMask
[2027](sec  1)(fl 0x00)(ty   0)(scl   2) (nx 0) 0x00000000001b3a00 bswapMask

c:\dev\src\issue19198>

So I have changed cmd/link:

diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go
index f1d51ca..8a0b257 100644
--- a/src/cmd/link/internal/ld/pe.go
+++ b/src/cmd/link/internal/ld/pe.go
@@ -962,6 +962,9 @@ func writePESymTableRecords(ctxt *Link) int {
 	var symcnt int
 
 	writeOneSymbol := func(s *Symbol, addr int64, sectidx int, typ uint16, class uint8) {
+		if strings.Contains(s.Name, "bswapMask") {
+			fmt.Printf("ALEX: name=%s file=%s\n", s.Name, s.File)
+		}
 		// write COFF symbol table record
 		if len(s.Name) > 8 {
 			Lputl(0)

to see more

# issue19198
ALEX: name=bswapMask file=crypto/aes
ALEX: name=bswapMask file=github.com/lucas-clemente/aes12
C:\dev\go\pkg\tool\windows_amd64\link.exe: running gcc failed: exit status 1
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0x1b3a00): multiple definition of `bswapMask'
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0x1b39f0): first defined here
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0x1b3bf0): multiple definition of `gcmPoly'
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0x1b3c00): first defined here
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0x1b4fe0): multiple definition of `andMask'
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0x1b4ee0): first defined here
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xf5cd0): multiple definition of `_expand_key_128'
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xbc9f0): first defined here
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xf5d00): multiple definition of `_expand_key_192a'
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xbca20): first defined here
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xf5d50): multiple definition of `_expand_key_192b'
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xbca70): first defined here
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xf5d90): multiple definition of `_expand_key_256a'
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xbcab0): first defined here
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xf5da0): multiple definition of `_expand_key_256b'
C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o:(.text+0xbcac0): first defined here
c:/dev/mingw64_4.9.1/bin/../lib/gcc/x86_64-w64-mingw32/4.9.1/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\brainman\AppData\Local\Temp\go-link-750078823\go.o: bad reloc address 0x110 in section `.data'
collect2.exe: error: ld returned 1 exit status

I will let @ianlancetaylor decide what needs to be done here, if anything.

Alex

@ianlancetaylor
Copy link
Contributor

crypto/aes is being a bit dubious in declaring some unqualified symbols as global, but I think that ultimately the error is that writePESymTableRecords uses IMAGE_SYM_CLASS_EXTERNAL for everything. The corresponding code in putelfsym does this:

	bind := STB_GLOBAL

	if x.Version != 0 || (x.Type&obj.SHIDDEN != 0) || x.Attr.Local() {
		bind = STB_LOCAL
	}

Here STB_GLOBAL corresponds to IMAGE_SYM_CLASS_EXTERNAL, and STB_LOCAL ought to correspond to IMAGE_SYM_CLASS_STATIC. That is, some of the PE symbols should be marked as STATIC rather than EXTERNAL.

@alexbrainman Do you want try a patch to writePESymTableRecords to change the storage class for the PE symbols, and see what happens? I don't really do Windows so I don't have a way to test it.

@alexbrainman
Copy link
Member

try a patch to writePESymTableRecords to change the storage class for the PE symbols, and see what happens?

I tried this:

diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go
index f1d51ca..d54392e 100644
--- a/src/cmd/link/internal/ld/pe.go
+++ b/src/cmd/link/internal/ld/pe.go
@@ -1027,7 +1027,13 @@ func writePESymTableRecords(ctxt *Link) int {
 			typ = IMAGE_SYM_DTYPE_ARRAY<<8 + IMAGE_SYM_TYPE_STRUCT
 			typ = 0x0308 // "array of structs"
 		}
-		writeOneSymbol(s, value, sect, typ, IMAGE_SYM_CLASS_EXTERNAL)
+		class := IMAGE_SYM_CLASS_EXTERNAL
+		if Linkmode == LinkExternal {
+			if s.Version != 0 || (s.Type&obj.SHIDDEN != 0) || s.Attr.Local() {
+				class = IMAGE_SYM_CLASS_STATIC
+			}
+		}
+		writeOneSymbol(s, value, sect, typ, uint8(class))
 	}
 
 	if Linkmode == LinkExternal {

and the problem goes away. Should I send a CL? I think this needs a test. What would the test do? Where would the test live?

Thank you.

Alex

@ianlancetaylor
Copy link
Contributor

Sending a CL would be great, thanks.

In this case I would not bother to make it conditional on Linkmode == LinkExternal. I think it has always been wrong, it just doesn't matter in an executable.

I think the test can be a program with two packages that both have an asm file that both say

DATA sym<>(SB)/8, $0x0

A name like sym<> should be specific to the file, like a static variable in C, but if I'm right the error will show up as sym being passed twice to the external linker.

@alexbrainman
Copy link
Member

I think the test can be a program with two packages that both have an asm file

Where will this test live?

Alex

@ianlancetaylor
Copy link
Contributor

Since we have no good place for that kind of test, I would put it in cmd/go/go_test.go, as with TestConcurrentAsm.

@ianlancetaylor
Copy link
Contributor

In fact, it looks like TestConcurrentAsm is exactly the test you want, except add another build with -ldflags=-linkmode=external

@alexbrainman
Copy link
Member

TestConcurrentAsm is exactly the test you want,

I will have a go. Thank you.

Alex

@alexbrainman
Copy link
Member

add another build with -ldflags=-linkmode=external

I had to change too many things, so I created new test.
https://go-review.googlesource.com/37810

Alex

@gopherbot
Copy link

CL https://golang.org/cl/37810 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

4 participants