code review 96680045: debug/elf: support DWARF that needs relocs for 386

debug/elf: support DWARF that needs relocs for 386

It's not clear how widespread this issue is, but we do have a
test case generated by a development version of clang.

I don't know whether this should go into 1.3 or not; happy to
hear arguments either way.

[iant, 2014-06-03 00:57:24]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2014-06-03 01:09:37]

https://codereview.appspot.com/96680045/diff/1/src/pkg/debug/elf/file.go
File src/pkg/debug/elf/file.go (right):

https://codereview.appspot.com/96680045/diff/1/src/pkg/debug/elf/file.go#newc...
src/pkg/debug/elf/file.go:592: binary.Read(b, f.ByteOrder, &rel)
check error? if rels is short (say: 1 byte), this could fail I assume?

[iant, 2014-06-03 01:26:15]

https://codereview.appspot.com/96680045/diff/1/src/pkg/debug/elf/file.go
File src/pkg/debug/elf/file.go (right):

https://codereview.appspot.com/96680045/diff/1/src/pkg/debug/elf/file.go#newc...
src/pkg/debug/elf/file.go:592: binary.Read(b, f.ByteOrder, &rel)
On 2014/06/03 01:09:36, bradfitz wrote:
> check error? if rels is short (say: 1 byte), this could fail I assume?

At the start of the function we verify that we can't get a short read here.

[rsc, 2014-06-03 22:44:28]

LGTM

This is fine to go into 1.3.
What's broken? This is the elf reader not the elf writer.

[iant, 2014-06-03 23:36:55]

On 2014/06/03 22:44:28, rsc wrote:
>
> What's broken? This is the elf reader not the elf writer.

What is broken is cgo.

Clang generates objects like the test case.  When cgo goes to read them, it
compiles an object and reads the debug info using debug/elf.File.DWARF.  If we
don't relocate the debug info, the DW_FORM_strp entries all wind up with a value
of zero, so we don't get the correct variable names, so cgo fails to find the
variables it is looking for.

[iant, 2014-06-03 23:39:43]

*** Submitted as https://code.google.com/p/go/source/detail?r=5439c77d4acb ***

debug/elf: support DWARF that needs relocs for 386

It's not clear how widespread this issue is, but we do have a
test case generated by a development version of clang.

I don't know whether this should go into 1.3 or not; happy to
hear arguments either way.

LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://codereview.appspot.com/96680045

[rsc, 2014-06-03 23:40:13]

Thanks. I always forget that cgo uses this.