This is an archive of the discontinued LLVM Phabricator instance.

Differential D55113

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

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

Details

Reviewers
dblaikie
Commits
rG6d85c5832813: [llvm-dwarfdump] - Stop printing the bogus empty section name on invalid dwarf.
rL348115: [llvm-dwarfdump] - Stop printing the bogus empty section name on invalid dwarf.
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
Herald added subscribers: JDevlieghere, aprantl. · View Herald TranscriptNov 30 2018, 3:11 AM
probinson added a subscriber: probinson.Nov 30 2018, 7:50 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
In D55113#1314660, @probinson wrote:

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
Closed by commit rL348115: [llvm-dwarfdump] - Stop printing the bogus empty section name on invalid dwarf. (authored by grimar). · Explain Why
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 mentioned this in D55261: [llvm-dwarfdump] - Simplify the test case..Dec 4 2018, 3:16 AM
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.

Revision Contents

PathSize
llvm/
trunk/
lib/
DebugInfo/
DWARF/
4 lines
test/
tools/
llvm-dwarfdump/
X86/
197 lines

Diff 176333

llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugRnglists.cpp

Show First 20 Lines • Show All 117 Lines • ▼ Show 20 LinesDWARFDebugRnglist::getAbsoluteRanges(llvm::Optional<SectionedAddress> BaseAddr,
DWARFUnit &U) const { DWARFUnit &U) const {
DWARFAddressRangesVector Res; DWARFAddressRangesVector Res;
for (const RangeListEntry &RLE : Entries) { for (const RangeListEntry &RLE : Entries) {
if (RLE.EntryKind == dwarf::DW_RLE_end_of_list) if (RLE.EntryKind == dwarf::DW_RLE_end_of_list)
break; break;
if (RLE.EntryKind == dwarf::DW_RLE_base_addressx) { if (RLE.EntryKind == dwarf::DW_RLE_base_addressx) {
BaseAddr = U.getAddrOffsetSectionItem(RLE.Value0); BaseAddr = U.getAddrOffsetSectionItem(RLE.Value0);
if (!BaseAddr) if (!BaseAddr)
BaseAddr = {RLE.Value0, 0}; BaseAddr = {RLE.Value0, -1ULL};
continue; continue;
} }
if (RLE.EntryKind == dwarf::DW_RLE_base_address) { if (RLE.EntryKind == dwarf::DW_RLE_base_address) {
BaseAddr = {RLE.Value0, RLE.SectionIndex}; BaseAddr = {RLE.Value0, RLE.SectionIndex};
continue; continue;
} }
DWARFAddressRange E; DWARFAddressRange E;
Show All 16 Lines case dwarf::DW_RLE_start_end:
break; break;
case dwarf::DW_RLE_start_length: case dwarf::DW_RLE_start_length:
E.LowPC = RLE.Value0; E.LowPC = RLE.Value0;
E.HighPC = E.LowPC + RLE.Value1; E.HighPC = E.LowPC + RLE.Value1;
break; break;
case dwarf::DW_RLE_startx_length: { case dwarf::DW_RLE_startx_length: {
auto Start = U.getAddrOffsetSectionItem(RLE.Value0); auto Start = U.getAddrOffsetSectionItem(RLE.Value0);
if (!Start) if (!Start)
Start = {0, 0}; Start = {0, -1ULL};
E.SectionIndex = Start->SectionIndex; E.SectionIndex = Start->SectionIndex;
E.LowPC = Start->Address; E.LowPC = Start->Address;
E.HighPC = E.LowPC + RLE.Value1; E.HighPC = E.LowPC + RLE.Value1;
break; break;
} }
default: default:
// Unsupported encodings should have been reported during extraction, // Unsupported encodings should have been reported during extraction,
// so we should not run into any here. // so we should not run into any here.
▲ Show 20 Lines • Show All 81 Lines • Show Last 20 Lines

llvm/trunk/test/tools/llvm-dwarfdump/X86/no_debug_addr.s

# RUN: llvm-mc %s -filetype obj -triple=x86_64-pc-linux -o %t.o
# RUN: llvm-dwarfdump -v %t.o | FileCheck %s
## When .debug_addr is absent for some reason,
## check we do not print a bogus section name.
# CHECK: DW_AT_ranges [DW_FORM_rnglistx] (indexed (0x0) rangelist = 0x00000004
# CHECK-NEXT: [0x0000000000000000, 0x0000000000000001)
# CHECK-NEXT: [0x0000000000000000, 0x0000000000000002)
# CHECK-NEXT: [0x0000000000000003, 0x0000000000000004))
dblaikieUnsubmitted
Not Done
ReplyInline Actions

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)

dblaikie: Could you provide the source (& compilation commands) for this assembly in a comment here so…
grimarAuthorUnsubmitted
Done
ReplyInline Actions

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.

grimar: > Could you provide the source (& compilation commands) for this assembly in a comment here so…
.text
.section .text._Z3zedv,"ax",@progbits
.Lfunc_begin0:
retq
.Lfunc_end0:
.section .text.main,"ax",@progbits
.Lfunc_begin1:
retq
retq
.Lfunc_end1:
.section .debug_str_offsets,"",@progbits
.long 32
.short 5
.short 0
.Lstr_offsets_base0:
.long .Linfo_string0
.long .Linfo_string0
.long .Linfo_string0
.long .Linfo_string0
.long .Linfo_string0
.long .Linfo_string0
.long .Linfo_string0
.section .debug_str,"MS",@progbits,1
.Linfo_string0:
.asciz "stub"
.section .debug_abbrev,"",@progbits
.byte 1 # Abbreviation Code
.byte 17 # DW_TAG_compile_unit
.byte 1 # DW_CHILDREN_yes
.byte 37 # DW_AT_producer
.byte 37 # DW_FORM_strx1
.byte 19 # DW_AT_language
.byte 5 # DW_FORM_data2
.byte 3 # DW_AT_name
.byte 37 # DW_FORM_strx1
.byte 114 # DW_AT_str_offsets_base
.byte 23 # DW_FORM_sec_offset
.byte 16 # DW_AT_stmt_list
.byte 23 # DW_FORM_sec_offset
.byte 27 # DW_AT_comp_dir
.byte 37 # DW_FORM_strx1
.byte 115 # DW_AT_addr_base
.byte 23 # DW_FORM_sec_offset
.byte 17 # DW_AT_low_pc
.byte 1 # DW_FORM_addr
.byte 85 # DW_AT_ranges
.byte 35 # DW_FORM_rnglistx
.byte 116 # DW_AT_rnglists_base
.byte 23 # DW_FORM_sec_offset
.byte 0 # EOM(1)
.byte 0 # EOM(2)
.byte 2 # Abbreviation Code
.byte 46 # DW_TAG_subprogram
.byte 0 # DW_CHILDREN_no
.byte 17 # DW_AT_low_pc
.byte 27 # DW_FORM_addrx
.byte 18 # DW_AT_high_pc
.byte 6 # DW_FORM_data4
.byte 64 # DW_AT_frame_base
.byte 24 # DW_FORM_exprloc
.byte 110 # DW_AT_linkage_name
.byte 37 # DW_FORM_strx1
.byte 3 # DW_AT_name
.byte 37 # DW_FORM_strx1
.byte 58 # DW_AT_decl_file
.byte 11 # DW_FORM_data1
.byte 59 # DW_AT_decl_line
.byte 11 # DW_FORM_data1
.byte 73 # DW_AT_type
.byte 19 # DW_FORM_ref4
.byte 63 # DW_AT_external
.byte 25 # DW_FORM_flag_present
.byte 0 # EOM(1)
.byte 0 # EOM(2)
.byte 3 # Abbreviation Code
.byte 46 # DW_TAG_subprogram
.byte 0 # DW_CHILDREN_no
.byte 17 # DW_AT_low_pc
.byte 27 # DW_FORM_addrx
.byte 18 # DW_AT_high_pc
.byte 6 # DW_FORM_data4
.byte 64 # DW_AT_frame_base
.byte 24 # DW_FORM_exprloc
.byte 3 # DW_AT_name
.byte 37 # DW_FORM_strx1
.byte 58 # DW_AT_decl_file
.byte 11 # DW_FORM_data1
.byte 59 # DW_AT_decl_line
.byte 11 # DW_FORM_data1
.byte 73 # DW_AT_type
.byte 19 # DW_FORM_ref4
.byte 63 # DW_AT_external
.byte 25 # DW_FORM_flag_present
.byte 0 # EOM(1)
.byte 0 # EOM(2)
.byte 4 # Abbreviation Code
.byte 36 # DW_TAG_base_type
.byte 0 # DW_CHILDREN_no
.byte 3 # DW_AT_name
.byte 37 # DW_FORM_strx1
.byte 62 # DW_AT_encoding
.byte 11 # DW_FORM_data1
.byte 11 # DW_AT_byte_size
.byte 11 # DW_FORM_data1
.byte 0 # EOM(1)
.byte 0 # EOM(2)
.byte 0 # EOM(3)
.section .debug_info,"",@progbits
.Lcu_begin0:
.long 75 # Length of Unit
.short 5 # DWARF version number
.byte 1 # DWARF Unit Type
.byte 8 # Address Size (in bytes)
.long .debug_abbrev # Offset Into Abbrev. Section
.byte 1 # Abbrev [1] 0xc:0x43 DW_TAG_compile_unit
.byte 0 # DW_AT_producer
.short 4 # DW_AT_language
.byte 1 # DW_AT_name
.long .Lstr_offsets_base0 # DW_AT_str_offsets_base
.long 0 # DW_AT_stmt_list
.byte 2 # DW_AT_comp_dir
.long 0 # DW_AT_addr_base
.quad 0 # DW_AT_low_pc
.byte 0 # DW_AT_ranges
.long .Lrnglists_table_base0 # DW_AT_rnglists_base
.byte 2 # Abbrev [2] 0x2b:0x10 DW_TAG_subprogram
.byte 0 # DW_AT_low_pc
.long .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc
.byte 1 # DW_AT_frame_base
.byte 86
.byte 3 # DW_AT_linkage_name
.byte 4 # DW_AT_name
.byte 1 # DW_AT_decl_file
.byte 1 # DW_AT_decl_line
.long 74 # DW_AT_type
# DW_AT_external
.byte 3 # Abbrev [3] 0x3b:0xf DW_TAG_subprogram
.byte 1 # DW_AT_low_pc
.long .Lfunc_end1-.Lfunc_begin1 # DW_AT_high_pc
.byte 1 # DW_AT_frame_base
.byte 86
.byte 6 # DW_AT_name
.byte 1 # DW_AT_decl_file
.byte 5 # DW_AT_decl_line
.long 74 # DW_AT_type
# DW_AT_external
.byte 4 # Abbrev [4] 0x4a:0x4 DW_TAG_base_type
.byte 5 # DW_AT_name
.byte 5 # DW_AT_encoding
.byte 4 # DW_AT_byte_size
.byte 0 # End Of Children Mark
.section .debug_rnglists,"",@progbits
.long .Ldebug_rnglist_table_end0-.Ldebug_rnglist_table_start0 # Length
.Ldebug_rnglist_table_start0:
.short 5 # Version
.byte 8 # Address size
.byte 0 # Segment selector size
.long 1 # Offset entry count
.Lrnglists_table_base0:
.long .Ldebug_ranges0-.Lrnglists_table_base0
.Ldebug_ranges0:
.byte 3 # DW_RLE_startx_length
.byte 0 # start index
.uleb128 .Lfunc_end0-.Lfunc_begin0 # length
.byte 3 # DW_RLE_startx_length
.byte 1 # start index
.uleb128 .Lfunc_end1-.Lfunc_begin1 # length
.byte 1 # DW_RLE_base_addressx
.byte 0 # Base address (index 0 in .debug_addr)
.byte 4 # DW_RLE_offset_pair
.byte 0x3 # Start offset
.byte 0x4 # End offset
.byte 0 # DW_RLE_end_of_list
.Ldebug_rnglist_table_end0: