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

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

cmd/link: dwarf code badly confused by repeated local symbols #21566

rsc opened this issue Aug 23, 2017 · 5 comments
Labels
FrozenDueToAge
Milestone
Go1.10

Comments

@rsc
Copy link
Contributor

rsc 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
Copy link

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

@gopherbot
Copy link

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

@heschi
Copy link
Contributor

heschi 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
Copy link
Contributor Author

rsc 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
@rsc
[dev.boringcrypto] cmd/link: work around DWARF symbol bug
991652d 
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
Copy link

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

gopherbot pushed a commit that referenced this issue Aug 26, 2017
@rsc
[dev.boringcrypto.go1.8] cmd/link: work around DWARF symbol bug
4aff713 
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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
3 participants
@rsc @gopherbot @heschi