Page MenuHomePhabricator

Initial DWARF64 support for .debug_line
ClosedPublic

Authored by emaste on Oct 21 2013, 9:22 AM.

Details

Summary

Other DWARF sections still need to be done, as well as 64-bit offsets in DataExtractor also needed (as described DebugFrame).

http://llvm.org/bugs/show_bug.cgi?id=17411

Diff Detail

Repository
rL LLVM

Event Timeline

Could you include some test cases?

Since LLVM cannot produce DWARF64 yet (so far as I know) you may need to
generate them with GCC (you'll find other binary tests in
test/DebugInfo/Inputs/dwarfdump-* (with corresponding tests like
test/DebugInfo/dwarfdump-test.test)). Please include comments about which
version of GCC and what flags were used to generate the binaries.

I compiled the existing dwarfdump-inl-test.cc for FreeBSD mips64:

% g++ --version
g++ (GCC) 4.2.1 20070831 patched [FreeBSD]
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
% g++ -g dwarfdump-inl-test.cc -o dwarfdump-inl-test.freebsd-elf-mips64

current state:

% bin/llvm-dwarfdump --debug-dump=line dwarfdump-inl-test.freebsd-elf-mips64
dwarfdump-inl-test.freebsd-elf-mips64:  file format ELF64-unknown


.debug_line contents:
%

with this patch:

% bin/llvm-dwarfdump --debug-dump=line dwarfdump-inl-test.freebsd-elf-mips64
dwarfdump-inl-test.freebsd-elf-mips64:  file format ELF64-unknown


.debug_line contents:
Line table prologue:
   total_length: 0x00000212
        version: 2
prologue_length: 0x000001ab
min_inst_length: 1
default_is_stmt: 1
      line_base: -5
     line_range: 14
    opcode_base: 13
...

this only outputs the first CompileUnit though, perhaps because the parsing fails in another section in DWARFContext::parseCompileUnits(). Is it reasonable to add this dwarfdump-inl-test.freebsd-elf-mips64 binary and a basic test, fleshing it out when DWARF64 parsing for other sections is added?

(the test file is 15638 bytes -- larger than the clang-compiled version, due to the lack of -gline-tables-only)

Is this actually using 64-bit dwarf or is it dwarf with 64-bit
pointers? If it's the former ... then how big is the testcase? It
really shouldn't be creating dwarf64 unless the references in the
object are large (not the size of a pointer) unless this is dwarf2.

-eric

This patch addresses the output of GCC and gas on FreeBSD mips64

# gcc --version
gcc (GCC) 4.2.1 20070831 patched [FreeBSD]
# as --version
GNU assembler 2.17.50 [FreeBSD] 2007-07-03

This toolchain generally outputs dwarf2 but uses the 64-bit format from dwarf3. This is the format that uses 0xffffffff in the initial length field, followed by a 64-bit length, and 64-bit offsets throughout. Equivalent support already exists in DWARFDebugFrame.cpp.

My testcase binary is tiny; 64-bit is not necessary. The (older) version of GCC and gas in FreeBSD output this format for some reason; I do not know if more recent versions still do.

for whatever reason the toolchain insists on

That's really a shame. For dwarf3 it shouldn't generate 64-bit dwarf
unless the offsets in the file require 64-bits to represent. For
dwarf2 it's based on pointer size. Seems like a bug. Which version of
gcc is this?

-eric

The version was in the Phabricator review or email :-)
gcc 4.2.1, gas "2.17.50"

I agree that this is a shame, but it's what we've got. I haven't investigated in detail, but quick Googling suggests that the GNU toolchain switched n64 Linux to DWARF32 in 2006; I did not find evidence that the same change was made for FreeBSD.

It appears the GNU toolchain was changed to default to dwarf32 on all MIPS platforms sometime later, and I will make a similar change to the FreeBSD in-tree toolchain. That said, we still have MIPS binaries and libraries built with dwarf64 data today.

Excellent. Thanks.

FreeBSD in-tree toolchain updated in svn r256859:
http://svnweb.freebsd.org/base?view=revision&revision=256859

Shall I still proceed with this patch? We (a) have objects with 64-bit dwarf .debug_line already due to the silly default in older toolchains, and (b) may eventually encounter an object that actually requires this support.

echristo accepted this revision.Oct 21 2013, 2:00 PM

The patch itself is ok, though it needs a few things:

a) comments on what is and is not handled for dwarf64. The compile unit header is probably the best place to document that there's at least some partial support for reading line tables.

b) testcase. I'm ok with adding the testcase and a comment on it that dwarf64 parsing isn't very good at this point and that the only thing that's expected to work are the line tables. You can, for now, just dump the line table in the testcase.

emaste updated this revision to Unknown Object (????).Oct 23 2013, 8:05 AM

Added comments and a basic test

(Comments are in DWARFContext.h as that seemed to be a more clear reference to the individual classes.)

One comment, otherwise it looks ok.

lib/DebugInfo/DWARFDebugLine.cpp
220 ↗(On Diff #5096)

Not sure I understand the error here.

(Sorry for the delay)

emaste added inline comments.Feb 27 2014, 10:12 AM
lib/DebugInfo/DWARFDebugLine.cpp
220 ↗(On Diff #5096)

gas has a bug that produces an incorrect prologue_length for 64-bit DWARF. As it turns out we can handle that condition, so we can still return true.

I fixed the underlying bug in FreeBSD here:
http://svnweb.freebsd.org/base?view=revision&revision=256692
and your note reminded me that I never forwarded this to the binutils upstream, so I've now posted to their mailing list as well.

Maybe just drop the message?

echristo accepted this revision.Feb 28 2014, 10:20 AM

Sounds good to me.

Thanks!

Was this ever committed? Any reason why not?

emaste updated this revision to Diff 10244.Jun 9 2014, 11:33 AM
emaste edited edge metadata.

Rebase

It just fell off my radar during the previous iterations. I subsequently committed an improved version of the change to LLDB - that's in http://llvm-reviews.chandlerc.com/D2007 .

We'll want the equivalent change here too, but I think it makes sense to bring in this by itself since it's already done. It wasn't a trivial rebase against head so I'll commit if I can get a review of the new diff.

samsonov accepted this revision.Jun 9 2014, 1:07 PM
samsonov added a reviewer: samsonov.
samsonov added a subscriber: samsonov.

I'm ok with this patch.

test/DebugInfo/dwarfdump-64-bit-dwarf.test
1 ↗(On Diff #10244)

Is "inl-test" necessary in binary name? If not, please rename it.

3 ↗(On Diff #10244)

Please add a comment about limited DWARF-64 parsing capabilities.

emaste added inline comments.Jun 9 2014, 1:32 PM
test/DebugInfo/dwarfdump-64-bit-dwarf.test
1 ↗(On Diff #10244)

I named it this only because it was built from dwarfdump-inl-test.cc

The actual source isn't really relevant though, so I can just call it dwarfdump.elf-mips64-64-bit-dwarf if that's preferable.

emaste updated this revision to Diff 10248.Jun 9 2014, 1:37 PM
emaste edited edge metadata.

Add comment and rename input binary file

emaste updated this revision to Diff 10276.Jun 10 2014, 6:48 AM

correct arg types for printf

Sorry this slipped through the cracks for so long. I'll commit shortly, but have uploaded a rebased version of the change for now.

This revision was automatically updated to reflect the committed changes.