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

Jump to bottom

cmd/link: issues handling duplicate weak symbols #29563

Closed
4a6f656c opened this issue Jan 4, 2019 · 4 comments
Closed

cmd/link: issues handling duplicate weak symbols #29563

4a6f656c opened this issue Jan 4, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-OpenBSD
Milestone
Go1.13

Comments

@4a6f656c
Copy link
Contributor

4a6f656c commented Jan 4, 2019

OpenBSD -current now enables -mretpoline by default, which results in the compiler emitting a weak __llvm_retpoline_r11 symbol per compilation unit. This results in a cgo package containing multiple files that have the same weak symbol reference, which cmd/link/internal/loadelf then fails on:

$ CC=clang ./make.bash                                                                                                                                                                          
Building Go cmd/dist using /home/joel/src/go1.4.                                                                                                                                                                                
Building Go toolchain1 using /home/joel/src/go1.4.                                                                                                                                                                              
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.                                                                                                                                                                
Building Go toolchain2 using go_bootstrap and Go toolchain1.                                                                                                                                                                    
Building Go toolchain3 using go_bootstrap and Go toolchain2.                                                                                                                                                                    
Building packages and commands for openbsd/amd64.
# cmd/go
loadelf: /home/joel/src/go/pkg/openbsd_amd64/net.a(_x005.o): duplicate symbol reference: __llvm_retpoline_r11 in both net(.text.__llvm_retpoline_r11) and net(.text.__llvm_retpoline_r11)                                       
loadelf: /home/joel/src/go/pkg/openbsd_amd64/runtime/cgo.a(_x003.o): duplicate symbol reference: __llvm_retpoline_r11 in both net(.text.__llvm_retpoline_r11) and runtime/cgo(.text.__llvm_retpoline_r11)                       
loadelf: /home/joel/src/go/pkg/openbsd_amd64/runtime/cgo.a(_x004.o): duplicate symbol reference: __llvm_retpoline_r11 in both net(.text.__llvm_retpoline_r11) and runtime/cgo(.text.__llvm_retpoline_r11)                       
loadelf: /home/joel/src/go/pkg/openbsd_amd64/runtime/cgo.a(_x005.o): duplicate symbol reference: __llvm_retpoline_r11 in both net(.text.__llvm_retpoline_r11) and runtime/cgo(.text.__llvm_retpoline_r11)                       
loadelf: /home/joel/src/go/pkg/openbsd_amd64/runtime/cgo.a(_x006.o): duplicate symbol reference: __llvm_retpoline_r11 in both net(.text.__llvm_retpoline_r11) and runtime/cgo(.text.__llvm_retpoline_r11)                       
loadelf: /home/joel/src/go/pkg/openbsd_amd64/runtime/cgo.a(_x007.o): duplicate symbol reference: __llvm_retpoline_r11 in both net(.text.__llvm_retpoline_r11) and runtime/cgo(.text.__llvm_retpoline_r11)                       
_cgo_init: relocation target x_cgo_init not defined
_cgo_notify_runtime_init_done: relocation target x_cgo_notify_runtime_init_done not defined
_cgo_thread_start: relocation target x_cgo_thread_start not defined
net._cgo_bc9ef0f70a97_C2func_getaddrinfo: relocation target _cgo_bc9ef0f70a97_C2func_getaddrinfo not defined
net._cgo_bc9ef0f70a97_Cfunc_freeaddrinfo: relocation target _cgo_bc9ef0f70a97_Cfunc_freeaddrinfo not defined
net._cgo_bc9ef0f70a97_Cfunc_gai_strerror: relocation target _cgo_bc9ef0f70a97_Cfunc_gai_strerror not defined
runtime._cgo_setenv: relocation target x_cgo_setenv not defined
runtime._cgo_unsetenv: relocation target x_cgo_unsetenv not defined
runtime.cgo_yield: relocation target _cgo_yield not defined

This can also occur with retguard on OpenBSD clang as it emits similar weak symbols per compilation unit, which can then have matching names across files.

I've attempted to construct a test case for this, however it is rather challenging since it really has to be done at the compiler level. The net.a case from the above compilation is a useful reference however:

$ objdump -t /home/joel/src/go/pkg/openbsd_amd64/net.a
In archive /home/joel/src/go/pkg/openbsd_amd64/net.a:

_x001.o:     file format elf64-x86-64

SYMBOL TABLE:
0000000000000000 l    df *ABS*  0000000000000000 _cgo_export.c



_x002.o:     file format elf64-x86-64

SYMBOL TABLE:
0000000000000000 l    df *ABS*  0000000000000000 cgo_openbsd.cgo2.c



_x003.o:     file format elf64-x86-64

SYMBOL TABLE:
0000000000000000 l    df *ABS*  0000000000000000 cgo_resold.cgo2.c
0000000000000000 l    d  .text  0000000000000000 .text
0000000000000000 l    d  .text.__llvm_retpoline_r11     0000000000000000 .text.__llvm_retpoline_r11
0000000000000000 l    d  .debug_str     0000000000000000 .debug_str
0000000000000000 l    d  .debug_loc     0000000000000000 .debug_loc
0000000000000000 l    d  .debug_abbrev  0000000000000000 .debug_abbrev
0000000000000000 l    d  .debug_info    0000000000000000 .debug_info
0000000000000000 l    d  .debug_line    0000000000000000 .debug_line
0000000000000000         *UND*  0000000000000000 __errno
0000000000000000  w    F .text.__llvm_retpoline_r11     0000000000000015 .hidden __llvm_retpoline_r11
0000000000000008  w    O .openbsd.randomdata.retguard   0000000000000008 .hidden __retguard_1627
0000000000000000  w    O .openbsd.randomdata.retguard   0000000000000008 .hidden __retguard_3721
0000000000000000 g     F .text  000000000000008d _cgo_bc9ef0f70a97_C2func_getnameinfo
0000000000000090 g     F .text  0000000000000076 _cgo_bc9ef0f70a97_Cfunc_getnameinfo
0000000000000000         *UND*  0000000000000000 _cgo_topofstack
0000000000000000         *UND*  0000000000000000 getnameinfo



_x004.o:     file format elf64-x86-64

SYMBOL TABLE:
0000000000000000 l    df *ABS*  0000000000000000 cgo_sockold.cgo2.c

_x005.o:     file format elf64-x86-64

SYMBOL TABLE:
0000000000000000 l    df *ABS*  0000000000000000 cgo_unix.cgo2.c
0000000000000000 l    d  .text  0000000000000000 .text
0000000000000000 l    d  .text.__llvm_retpoline_r11     0000000000000000 .text.__llvm_retpoline_r11
0000000000000000 l    d  .debug_str     0000000000000000 .debug_str
0000000000000000 l    d  .debug_loc     0000000000000000 .debug_loc
0000000000000000 l    d  .debug_abbrev  0000000000000000 .debug_abbrev
0000000000000000 l    d  .debug_info    0000000000000000 .debug_info
0000000000000000 l    d  .debug_line    0000000000000000 .debug_line
0000000000000000         *UND*  0000000000000000 __errno
0000000000000000  w    F .text.__llvm_retpoline_r11     0000000000000015 .hidden __llvm_retpoline_r11
0000000000000008  w    O .openbsd.randomdata.retguard   0000000000000008 .hidden __retguard_1319
0000000000000018  w    O .openbsd.randomdata.retguard   0000000000000008 .hidden __retguard_2000
0000000000000000  w    O .openbsd.randomdata.retguard   0000000000000008 .hidden __retguard_2526
0000000000000010  w    O .openbsd.randomdata.retguard   0000000000000008 .hidden __retguard_3761
0000000000000000 g     F .text  000000000000007e _cgo_bc9ef0f70a97_C2func_getaddrinfo
0000000000000080 g     F .text  000000000000000d _cgo_bc9ef0f70a97_Cfunc_freeaddrinfo
0000000000000090 g     F .text  0000000000000053 _cgo_bc9ef0f70a97_Cfunc_gai_strerror
00000000000000f0 g     F .text  000000000000005f _cgo_bc9ef0f70a97_Cfunc_getaddrinfo
0000000000000000         *UND*  0000000000000000 _cgo_topofstack
0000000000000000         *UND*  0000000000000000 freeaddrinfo
0000000000000000         *UND*  0000000000000000 gai_strerror
0000000000000000         *UND*  0000000000000000 getaddrinfo

I suspect that if the symbols are weak and identical then they should be permitted as duplicates, however I'm not entirely sure what the correct/expected behaviour is here. For the time being I have a diff that matches weak symbols with a prefix of __llvm_retpoline_ and marks them as DuplicateOK - this resolves the issue and allows Go to build/pass tests, however it is not really something I'd like to push upstream...
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 4, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.13 milestone Jan 4, 2019
@ianlancetaylor ianlancetaylor changed the title cmd/link/internal/loadelf: issues handling duplicate weak symbols cmd/link: issues handling duplicate weak symbols Jan 4, 2019
@ianlancetaylor
Copy link
Contributor

Most (though not all) ELF linkers ignore weak defined symbols if they have already seen a definition of the symbol. So it would likely be correct to set AttrDuplicateOK for ElfSymBindWeak if the symbol is defined.

@4a6f656c
Copy link
Contributor Author

It was also pointed out to me that one of these cases (not both) actually have the symbols defined in a COMDAT (or LINK_ONCE_DISCARD) section. This is probably something that the Go linker needs to consider handling in the future.

@gopherbot
Copy link

Change https://golang.org/cl/169658 mentions this issue: cmd/link: permit duplicate weak symbols

@gopherbot
Copy link

Change https://golang.org/cl/170015 mentions this issue: cmd/link: allow duplicated weak symbols on Mach-O

gopherbot pushed a commit that referenced this issue Mar 30, 2019
@cherrymui
cmd/link: allow duplicated weak symbols on Mach-O
637f34f 
This fixes cgo test issue29563 on Darwin.

Updates #29563.

Change-Id: If480078461247cd7c95931ae3ad4ca89736dd550
Reviewed-on: https://go-review.googlesource.com/c/go/+/170015
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-OpenBSD
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
3 participants
@ianlancetaylor @4a6f656c @gopherbot