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/link: dwarf code badly confused by repeated local symbols #21566

Closed
rsc opened this issue Aug 23, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@rsc
Copy link
Contributor

commented Aug 23, 2017

When using -linkmode=internal, the linker's DWARF code gets badly confused if there are local (file-static) symbols in multiple objects with the same name. It creates a symbol go.info.name for each, but without distinct version numbers or any other distinguishing information. Then the code that walks the real symbols and builds a list of DWARF symbols puts the same DWARF symbol onto the list twice. Then the code that assigns DWARF symbols to sections gets confused because it stops when it finds a non-SDWARFINFO symbol, and when it reaches the second instance of a symbol, that symbol has already been converted to SRODATA, so the loop stops prematurely. That in turn causes missing DWARF info.

This only happens with internal linking, and nothing in the standard library happens to link against any code that tickles the bug today, but it certainly could in the future. I ran into this trying to use internal linking with BoringCrypto (much larger than our glibc cgo code).

The last time the linker correctly handled this situation (probably by not trying to collect that DWARF info at all) was Go 1.6.

$ cat x.c
static int f(int x) { return 1; }
int F(int x) { return f(x); }
$ cat y.c
static int f(int x) { return 1; }
int G(int x) { return f(x); }
$ cat z.go
package main

// #cgo CFLAGS: -g -O0
// int F(void);
// int G(void);
import "C"

func main() { println(C.F(), C.G()) }
$ go1.9 build -ldflags=-linkmode=internal 
# _/tmp/zz
.debug_pubnames: missing DWARF section for relocation target go.info.main._cgo_939471b576dc_Cfunc_F
.debug_pubnames: missing DWARF section for relocation target go.info.main._cgo_939471b576dc_Cfunc_G
.debug_pubnames: missing DWARF section for relocation target go.info.main.initdone·
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.algarray
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.support_aes
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.support_ssse3
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.support_sse41
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.useAeshash
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.writeBarrier
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.aeskeysched
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.hashkey
.debug_pubnames: missing DWARF section for relocation target go.info._cgo_mmap
.debug_pubnames: missing DWARF section for relocation target go.info._cgo_munmap
.debug_pubnames: missing DWARF section for relocation target go.info._cgo_sigaction
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.inForkedChild
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.iscgo
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.framepointer_enabled
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.main_init_done
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.extraMWaiters
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.debug
.debug_pubnames: missing DWARF section for relocation target go.info.runtime.mheap_
/home/rsc/go1.9/pkg/tool/linux_amd64/link: too many errors
$ go1.8 build -ldflags=-linkmode=internal
# _/tmp/zz
.debug_pubnames: missing DWARF section for relocation target go.info.main._cgo_939471b576dc_Cfunc_F
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x52947e]

goroutine 1 [running]:
cmd/link/internal/ld.relocsym(0xc4204c2000, 0xc420fce2d8)
	/usr/local/go/src/cmd/link/internal/ld/data.go:589 +0x26ae
cmd/link/internal/ld.(*Link).reloc(0xc4204c2000)
	/usr/local/go/src/cmd/link/internal/ld/data.go:739 +0x128
cmd/link/internal/ld.Main()
	/usr/local/go/src/cmd/link/internal/ld/main.go:207 +0x949
main.main()
	/usr/local/go/src/cmd/link/main.go:58 +0xdb
$ go1.7 build -ldflags=-linkmode=internal
# _/tmp/zz
2017/08/22 22:42:18 symbol go.dwarf.info.f listed multiple times
$ go1.6 build -ldflags=-linkmode=internal
$ ./zz
1 1
$ 

On dev.boringcrypto I will just skip over the duplicates, which works well enough, but it's not the right fix:

$ git diff
diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go
index 452332367c..0ad88603a5 100644
--- a/src/cmd/link/internal/ld/data.go
+++ b/src/cmd/link/internal/ld/data.go
@@ -1868,6 +1868,10 @@ func (ctxt *Link) dodata() {
 		datsize = Rnd(datsize, int64(sect.Align))
 		sect.Vaddr = uint64(datsize)
 		for _, s := range dwarfp[i:] {
+			// Syms can (incorrectly) appear twice on the list. Ignore repeats.
+			if s.Type == SRODATA {
+				continue
+			}
 			if s.Type != SDWARFINFO {
 				break
 			}

/cc @mwhudson @heschik @dr2chase

@rsc rsc added this to the Go1.10 milestone Aug 23, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Aug 23, 2017

Change https://golang.org/cl/57943 mentions this issue: [dev.boringcrypto] cmd/link: work around DWARF symbol bug

@gopherbot

This comment has been minimized.

Copy link

commented Aug 23, 2017

Change https://golang.org/cl/58070 mentions this issue: [dev.boringcrypto.go1.8] cmd/link: work around DWARF symbol bug

@heschik

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2017

I took a flying leap at this and moved the if s.FuncInfo == nil { continue } in writelines up a little bit. all.bash passes and the sample program now builds. Basically, I don't think we have any business creating the go.info symbols for non-Go functions, but I would want to get some more history/context before sending a change. If nobody else is interested I can do that.

@rsc

This comment has been minimized.

Copy link
Contributor Author

commented Aug 23, 2017

@heschik, I agree, that seems like a correct change to me. And if it fixes this (I don't quite see the connection to go.info, but I believe you), all the better. :-)

gopherbot pushed a commit that referenced this issue Aug 24, 2017

[dev.boringcrypto] cmd/link: work around DWARF symbol bug
The DWARF code is mishandling the case when the host object files
define multiple (distinct) symbols with the same name. They are mapped
to the same DWARF debug symbol, which then appears on the dwarfp
list multiple times, which then breaks the code that processes the list.
Detect duplicates and skip them, because that's trivial, instead of fixing
the underlying problem.

See #21566.

Change-Id: Ib5a34c891d7c15f4c7bb6239d8f31a1ec767b8bc
Reviewed-on: https://go-review.googlesource.com/57943
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot

This comment has been minimized.

Copy link

commented Aug 24, 2017

Change https://golang.org/cl/58630 mentions this issue: cmd/link: don't create go.info symbols for non-Go functions

@gopherbot gopherbot closed this in 6d7db25 Aug 24, 2017

gopherbot pushed a commit that referenced this issue Aug 26, 2017

[dev.boringcrypto.go1.8] cmd/link: work around DWARF symbol bug
The DWARF code is mishandling the case when the host object files
define multiple (distinct) symbols with the same name. They are mapped
to the same DWARF debug symbol, which then appears on the dwarfp
list multiple times, which then breaks the code that processes the list.
Detect duplicates and skip them, because that's trivial, instead of fixing
the underlying problem.

See #21566.

Change-Id: Ib5a34c891d7c15f4c7bb6239d8f31a1ec767b8bc
Reviewed-on: https://go-review.googlesource.com/57943
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/58070
Reviewed-by: Adam Langley <agl@golang.org>

@golang golang locked and limited conversation to collaborators Aug 24, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.