This is an archive of the discontinued LLVM Phabricator instance.

Differential D98589

[llvm] [dwarf] Fix DWARFListTableHeader::getOffsetEntry off-by-one
ClosedPublic

Authored by jankratochvil on Mar 13 2021, 1:56 PM.

Details

Reviewers
dblaikie
labath
Commits
rGa28facba1ccd: [llvm] [dwarf] Fix DWARFListTableHeader::getOffsetEntry off-by-one
Summary

D98289 was erroneously reporting invalid range list offset 0x20110 instead of invalid range list table index 0.

Diff Detail

Event Timeline

jankratochvil requested review of this revision.Mar 13 2021, 1:56 PM
jankratochvil created this revision.
jankratochvil mentioned this in D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC).Mar 13 2021, 1:59 PM
jankratochvil added a child revision: D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC).
Harbormaster completed remote builds in B93680: Diff 330472.Mar 13 2021, 2:33 PM
labath accepted this revision.Mar 14 2021, 1:08 PM
labath added a subscriber: labath.
labath added inline comments.
llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp
95–96

I think EXPECT_EQ(Offset0, uint64_t(4)); (note missing *) should be enough.

This revision is now accepted and ready to land.Mar 14 2021, 1:08 PM
This revision was landed with ongoing or failed builds.Mar 14 2021, 1:43 PM
Closed by commit rGa28facba1ccd: [llvm] [dwarf] Fix DWARFListTableHeader::getOffsetEntry off-by-one (authored by jankratochvil). · Explain Why
This revision was automatically updated to reflect the committed changes.
jankratochvil added a commit: rGa28facba1ccd: [llvm] [dwarf] Fix DWARFListTableHeader::getOffsetEntry off-by-one.
jankratochvil marked an inline comment as done.Mar 14 2021, 1:44 PM
jankratochvil added inline comments.
llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp
95–96

Nice, I did not know (even std::optional).

jankratochvil removed a child revision: D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC).Mar 31 2021, 5:03 AM

Revision Contents

PathSize
llvm/
include/
llvm/
DebugInfo/
DWARF/
2 lines
unittests/
DebugInfo/
DWARF/
26 lines

Diff 330529

llvm/include/llvm/DebugInfo/DWARF/DWARFListTable.h

Show First 20 Lines • Show All 107 Lines • ▼ Show 20 Lines case dwarf::DwarfFormat::DWARF64:
return 20; return 20;
} }
llvm_unreachable("Invalid DWARF format (expected DWARF32 or DWARF64"); llvm_unreachable("Invalid DWARF format (expected DWARF32 or DWARF64");
} }
void dump(DataExtractor Data, raw_ostream &OS, void dump(DataExtractor Data, raw_ostream &OS,
DIDumpOptions DumpOpts = {}) const; DIDumpOptions DumpOpts = {}) const;
Optional<uint64_t> getOffsetEntry(DataExtractor Data, uint32_t Index) const { Optional<uint64_t> getOffsetEntry(DataExtractor Data, uint32_t Index) const {
if (Index > HeaderData.OffsetEntryCount) if (Index >= HeaderData.OffsetEntryCount)
return None; return None;
return getOffsetEntry(Data, getHeaderOffset() + getHeaderSize(Format), Format, Index); return getOffsetEntry(Data, getHeaderOffset() + getHeaderSize(Format), Format, Index);
} }
static Optional<uint64_t> getOffsetEntry(DataExtractor Data, static Optional<uint64_t> getOffsetEntry(DataExtractor Data,
uint64_t OffsetTableOffset, uint64_t OffsetTableOffset,
dwarf::DwarfFormat Format, dwarf::DwarfFormat Format,
▲ Show 20 Lines • Show All 168 Lines • Show Last 20 Lines

llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp

Show First 20 Lines • Show All 67 Lines • ▼ Show 20 Lines EXPECT_THAT_ERROR(
FailedWithMessage(".debug_rnglists table at offset 0x0 has too small " FailedWithMessage(".debug_rnglists table at offset 0x0 has too small "
"length (0x6) to contain a complete header")); "length (0x6) to contain a complete header"));
// length() is expected to return the full length of the set if the unit // length() is expected to return the full length of the set if the unit
// length field is read, even if an error occurred during the parsing, // length field is read, even if an error occurred during the parsing,
// to allow skipping the current set and continue parsing from the next one. // to allow skipping the current set and continue parsing from the next one.
EXPECT_EQ(Header.length(), 6u); EXPECT_EQ(Header.length(), 6u);
} }
TEST(DWARFListTableHeader, OffsetEntryCount) {
static const char SecData[] = "\x10\x00\x00\x00" // Length
"\x05\x00" // Version
"\x08" // Address size
"\x00" // Segment selector size
"\x01\x00\x00\x00" // Offset entry count
"\x04\x00\x00\x00" // offset[0]
"\x04" // DW_RLE_offset_pair
"\x01" // ULEB128 starting offset
"\x02" // ULEB128 ending offset
"\x00"; // DW_RLE_end_of_list
DWARFDataExtractor Extractor(StringRef(SecData, sizeof(SecData) - 1),
/*isLittleEndian=*/true,
/*AddrSize=*/4);
DWARFListTableHeader Header(/*SectionName=*/".debug_rnglists",
/*ListTypeString=*/"range");
uint64_t Offset = 0;
EXPECT_FALSE(!!Header.extract(Extractor, &Offset));
Optional<uint64_t> Offset0 = Header.getOffsetEntry(Extractor, 0);
EXPECT_TRUE(!!Offset0);
EXPECT_EQ(Offset0, uint64_t(4));
labathUnsubmitted
Not Done
ReplyInline Actions

I think EXPECT_EQ(Offset0, uint64_t(4)); (note missing *) should be enough.

labath: I think `EXPECT_EQ(Offset0, uint64_t(4));` (note missing `*`) should be enough.
jankratochvilAuthorUnsubmitted
Done
ReplyInline Actions

Nice, I did not know (even std::optional).

jankratochvil: Nice, I did not know (even `std::optional`).
Optional<uint64_t> Offset1 = Header.getOffsetEntry(Extractor, 1);
EXPECT_FALSE(!!Offset1);
EXPECT_EQ(Header.length(), sizeof(SecData) - 1);
}
} // end anonymous namespace } // end anonymous namespace