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: .debug_pubnames and .debug_pubtypes not following DWARF4 spec #30573

Closed
jasonborg opened this issue Mar 4, 2019 · 9 comments

Comments

Projects
None yet
6 participants
@jasonborg
Copy link

commented Mar 4, 2019

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

$ go version

go version go1.12rc1 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go/go12_rc1"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/go12_rc1/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build829564919=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I ran dwarfdump on the compiled binary of the following sample code:

https://play.golang.org/p/CVBXqakTEye

What did you expect to see?

A full normal dwarfdump output, including the .debug_pubnames and .debug_pubtypes sections.

What did you see instead?

Dwarfdump exits early instead, emitting a failure.

The following is the tail end of the dwarfdump output before it fails. The failure occurs while processing the first .debug_pubnames entry found in the second CU:

.debug_pubnames
global die-in-sect 0x00000ae5, cu-in-sect 0x0000000b, die-in-cu 0x00000ae5, cu-header-in-sect 0x00000000 'internal/cpu.X86'
global die-in-sect 0x00000b06, cu-in-sect 0x0000000b, die-in-cu 0x00000b06, cu-header-in-sect 0x00000000 'internal/cpu.CacheLineSize'
global die-in-sect 0x00000b31, cu-in-sect 0x0000000b, die-in-cu 0x00000b31, cu-header-in-sect 0x00000000 'internal/cpu.DebugOptions'
global die-in-sect 0x00000b5b, cu-in-sect 0x0000000b, die-in-cu 0x00000b5b, cu-header-in-sect 0x00000000 'internal/cpu.ARM64'
global die-in-sect 0x00000b7e, cu-in-sect 0x0000000b, die-in-cu 0x00000b7e, cu-header-in-sect 0x00000000 'internal/cpu.options'

dwarfdump NO ENTRY:  global dwarf_offdie : die offset does not reference valid DIE.  0x1a12.: 

More details on underlying cause

It was the libdwarf maintainer, David Anderson, who determined the underlying issue. In summary, the entries emitted in the .debug_pubnames and the .debug_pubtypes sections contain the global die offset for the entry, and not the offset from the start of the CU it is found in, as specified in section 6.1.1 of the DWARF4 spec.

Also as an interesting note, this issue happens to not be noticeable on Go 1.11, as all of the pubtypes and pubnames entries were found in the first CU, so the global offsets and the offset from their CU were the same.

The following snippet from running llvm-dwarfdump shows the first and second CU entries from the .debug_pubnames output:

llvm-dwarfdump output:
.debug_pubnames contents: a
length = 0x00000090 version = 0x0002 unit_offset = 0x00000000 unit_size = 0x00000ba4
Offset     Name
0x00000ae5 "internal/cpu.X86"
0x00000b06 "internal/cpu.CacheLineSize"
0x00000b31 "internal/cpu.DebugOptions"
0x00000b5b "internal/cpu.ARM64"
0x00000b7e "internal/cpu.options"
length = 0x0000004a version = 0x0002 unit_offset = 0x00000ba4 unit_size = 0x0000031f
Offset     Name
0x00000e6e "internal/bytealg.MaxLen"
0x00000e96 "internal/bytealg.initdone·"

The error message in the earlier dwarfdump output above shows it attempting to access the DIE at global offset 0x1a12, and finding that location does not contain a valid DIE entry. This is because dwarfdump is taking the offset of 0x00000e6e and adding it to the CU offset of 0x00000ba4 (listed in the llvm-dwardump) to obtain what it believes is the global offset of 0x1a12. However the offset of 0x00000e6e is already the global offset.

This is seen here by running dwarfdump -G -M -i and finding the .debug_info entry for internal/bytealg.MaxLen.

< 1><0x000002ca GOFF=0x00000e6e>   DW_TAG_variable
                                      DW_AT_name        internal/bytealg.MaxLen <form DW_FORM_string> 
                                      DW_AT_location    DW_OP_addr 0x00574f80 <form DW_FORM_block1>
                                      DW_AT_type        <GOFF=0x000353f7> <form DW_FORM_ref_addr>
                                      DW_AT_external    yes(1) <form DW_FORM_flag>

The global offset is 0x0e6e, and its offset within the CU is 0x02ca. Subtracting 0x02ca from 0xe6e gives 0x0ba4, which matches the offset of the CU itself shown in the llvm-dwardump output. So the offset that should be appearing in the pubname section for internal/bytealg.MaxLen should be 0x02ca, which is its offset from the start of the CU.

Also, as shown in this hexdump snippit, the global offset of 0x0e6e is coming from the binary itself, the 4 bytes starting at byte position 00193fc2, which precedes the string "internal/bytealg.MaxLen".

00193f30  00 00 69 6e 74 65 72 6e  61 6c 2f 63 70 75 2e 58  |..internal/cpu.X|
00193f40  38 36 00 06 0b 00 00 69  6e 74 65 72 6e 61 6c 2f  |86.....internal/|
00193f50  63 70 75 2e 43 61 63 68  65 4c 69 6e 65 53 69 7a  |cpu.CacheLineSiz|
00193f60  65 00 31 0b 00 00 69 6e  74 65 72 6e 61 6c 2f 63  |e.1...internal/c|
00193f70  70 75 2e 44 65 62 75 67  4f 70 74 69 6f 6e 73 00  |pu.DebugOptions.|
00193f80  5b 0b 00 00 69 6e 74 65  72 6e 61 6c 2f 63 70 75  |[...internal/cpu|
00193f90  2e 41 52 4d 36 34 00 7e  0b 00 00 69 6e 74 65 72  |.ARM64.~...inter|
00193fa0  6e 61 6c 2f 63 70 75 2e  6f 70 74 69 6f 6e 73 00  |nal/cpu.options.|
00193fb0  00 00 00 00 4a 00 00 00  02 00 a4 0b 00 00 1f 03  |....J...........|
00193fc0  00 00 6e 0e 00 00 69 6e  74 65 72 6e 61 6c 2f 62  |..n...internal/b|
00193fd0  79 74 65 61 6c 67 2e 4d  61 78 4c 65 6e 00 96 0e  |ytealg.MaxLen...|
@josharian

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

@josharian josharian changed the title debug/dwarf: .debug_pubnames and .debug_pubtypes not following DWARF4 spec cmd/compile: .debug_pubnames and .debug_pubtypes not following DWARF4 spec Mar 4, 2019

@josharian josharian added this to the Go1.13 milestone Mar 4, 2019

@aarzilli

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

This is my faul, I wrote the CL that distrubutes variables among CUs and exposes the bug. I'll write a fix for it.

@aarzilli

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

PS. the bug is in cmd/link

@josharian josharian changed the title cmd/compile: .debug_pubnames and .debug_pubtypes not following DWARF4 spec cmd/link: .debug_pubnames and .debug_pubtypes not following DWARF4 spec Mar 4, 2019

@heschik

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

I worked with Jason on this issue and I agree that the DWARF is incorrect. One interesting thing to note is that both readelf and llvm-dwarfdump are fine with it. Given that I don't think this needs to be backported to 1.12.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 5, 2019

Change https://golang.org/cl/165337 mentions this issue: cmd/link: fixes contents of debug_pubnames/debug_pubtypes

@aarzilli

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

I also have a different proposal: let's remove debug_pubnames/debug_pubtypes. They are entirely optional (DWARFv4, section 6.1, page 105), anything that can be done with them can be done without them and pubnames has been wrong for a long time. It's supposed to have entries for public functions but it hasn't since 1.8 (I think) when DWARF generation for functions was moved to the compiler.

It'll save some linker time and disk space at the expense of slower global variable lookup in some debuggers (assuming gdb and lldb don't compensate for its absence).

@jasonborg

This comment has been minimized.

Copy link
Author

commented Mar 5, 2019

I currently have some code that makes use of the debug_pubtypes section, so speaking personally it would be nice if it stuck around :)

@heschik

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

For the moment, if we've got it, it should at least follow the spec IMO. I'll take a look at the CL.

I think @aclements would need to make a call on whether to remove them. Austin?

@gopherbot gopherbot closed this in 4d8a37a Mar 6, 2019

@aclements

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

Given that these sections are 0.6% of the cmd/go binary, I'm not too worried about the space (though perhaps there is a broader "death by a thousand cuts" issue). I'm more worried about them being incomplete.

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