Page MenuHomePhabricator

[llvm-dwarfdump] - Stop printing the bogus empty section name on invalid dwarf.
ClosedPublic

Authored by grimar on Nov 30 2018, 3:11 AM.

Details

Summary

When there is no .debug_addr section for some reason,
llvm-dwarfdump would print the bogus empty section name when dumping ranges
in .debug_info:

# CHECK:       DW_AT_ranges [DW_FORM_rnglistx]   (indexed (0x0) rangelist = 0x00000004
# CHECK-NEXT:    [0x0000000000000000, 0x0000000000000001) ""
# CHECK-NEXT:    [0x0000000000000000, 0x0000000000000002) "")

That happens because of the code which uses 0 (zero) as a section index as a default value.
The code should use -1ULL instead because technically 0 is a valid zero section index
in ELF and -1ULL is a special constant used that means "no section available".

This is mostly a fix for the overall correctness/safety of the code, but a test case is provided too.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Nov 30 2018, 3:11 AM

Are those hard tabs in the test .s file? many editors do that automatically in assembler source, but the LLVM project doesn't want hard tabs.

LGTM otherwise.

grimar added a comment.Dec 3 2018, 2:30 AM

Are those hard tabs in the test .s file? many editors do that automatically in assembler source, but the LLVM project doesn't want hard tabs.

LGTM otherwise.

There were few trailing whitespaces I didn't notice. Will remove them before the commit. Thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Dec 3 2018, 2:36 AM
This revision was automatically updated to reflect the committed changes.
dblaikie added inline comments.Dec 3 2018, 10:41 AM
llvm/trunk/test/tools/llvm-dwarfdump/X86/no_debug_addr.s
4–11

Could you provide the source (& compilation commands) for this assembly in a comment here so it's easier to eyeball what the input is to this test rather than reading all the assembly?

Also, does this need 3 ranges, or would two suffice (thus creating shorter assembly that might be a little easier to understand)? Anything else you can do to simplify the test case would be great.

Actually, why are there 3 ranges here, when it looks like there are only 2 functions? (ah, I see, invalid input/DWARF as you mentioned in the description - but still, could be simpler to have 2 functions rather than 3)

grimar marked an inline comment as done.Dec 4 2018, 3:18 AM
grimar added inline comments.
llvm/trunk/test/tools/llvm-dwarfdump/X86/no_debug_addr.s
4–11

Could you provide the source (& compilation commands) for this assembly in a comment here so it's easier to eyeball what the input is to this > test rather than reading all the assembly?

I could, but found that would not be useful probably.
The idea of this test is that .debug_addr is absent. So I had to drop it manually. At the same time, test should
use both the DW_RLE_startx_length and DW_RLE_base_addressx. I had to reduce and manually edit the assembly and
particularly edited the .debug_rnglists to add the DW_RLE_base_addressx + 'DW_RLE_offset_pair' entries too.
(It was much easier than trying to find the matching c++ code for the DWARF I want to emit)

So I think the user needs to read it to understand the test anyways and having c++ code is more confusing than useful here,
because c++ code would have no so much common with the case I am testing here.

Actually, why are there 3 ranges here, when it looks like there are only 2 functions? (ah, I see, invalid input/DWARF as you mentioned
in the description - but still, could be simpler to have 2 functions rather than 3)

That is because DW_RLE_startx_length were generated from c++ file, but I edited the assembly and added the DW_RLE_base_addressx + DW_RLE_offset_pair entries manually because wanted to test the change I did for DW_RLE_base_addressx

Technically this test case needs 2 ranges in total, not 3.
I added/updated the comment and removed few excessive things from the test and posted D55261.