From cc5e2974692aced5d98d09af40dcdbd6725f5a55 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 29 Aug 2019 14:26:05 +0000 Subject: [PATCH 1/4] DWARFDebugLoc: Make parsing and error reporting more robust Summary: While examining this class for possible use in lldb, I noticed two things: - it spits out parsing errors directly to stderr - the loclists parser can incorrectly return valid location lists when parsing malformed (truncated) data I improve the stderr situation by making the parseOneLocationList functions return Expecteds. The errors are still dumped to stderr by their callers, so this is only a partial fix, but it is enough for my use case, as I intend to parse the locations lists one by one. I fix the behavior in the truncated scenario by using the newly introduced DataExtractor Cursor API. I also add tests for handling the error cases, as they currently have no coverage. Reviewers: dblaikie, JDevlieghere, probinson Subscribers: lldb-commits, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D63591 llvm-svn: 370363 (cherry picked from commit bd546e59026d90b3a9f48f90586779de1f8c7202) --- .../llvm/DebugInfo/DWARF/DWARFDebugLoc.h | 13 +-- llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp | 97 ++++++++----------- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 12 +-- .../X86/dwarfdump-debug-loc-error-cases.s | 58 +++++++++++ .../dwarfdump-debug-loclists-error-cases.s | 71 ++++++++++++++ 5 files changed, 185 insertions(+), 66 deletions(-) create mode 100644 llvm/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s create mode 100644 llvm/test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h index adda8eef5aabc..3852312745b02 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h @@ -29,7 +29,7 @@ class DWARFDebugLoc { /// The ending address of the instruction range. uint64_t End; /// The location of the variable within the specified range. - SmallVector Loc; + SmallVector Loc; }; /// A list of locations that contain one variable. @@ -68,8 +68,8 @@ class DWARFDebugLoc { /// Return the location list at the given offset or nullptr. LocationList const *getLocationListAtOffset(uint64_t Offset) const; - Optional parseOneLocationList(DWARFDataExtractor Data, - uint64_t *Offset); + static Expected + parseOneLocationList(const DWARFDataExtractor &Data, uint64_t *Offset); }; class DWARFDebugLoclists { @@ -78,7 +78,7 @@ class DWARFDebugLoclists { uint8_t Kind; uint64_t Value0; uint64_t Value1; - SmallVector Loc; + SmallVector Loc; }; struct LocationList { @@ -106,8 +106,9 @@ class DWARFDebugLoclists { /// Return the location list at the given offset or nullptr. LocationList const *getLocationListAtOffset(uint64_t Offset) const; - static Optional - parseOneLocationList(DataExtractor Data, uint64_t *Offset, unsigned Version); + static Expected parseOneLocationList(const DataExtractor &Data, + uint64_t *Offset, + unsigned Version); }; } // end namespace llvm diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp index 1469a210785be..e932f98204e2e 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp @@ -28,11 +28,10 @@ using namespace llvm; // expression that LLVM doesn't produce. Guessing the wrong version means we // won't be able to pretty print expressions in DWARF2 binaries produced by // non-LLVM tools. -static void dumpExpression(raw_ostream &OS, ArrayRef Data, +static void dumpExpression(raw_ostream &OS, ArrayRef Data, bool IsLittleEndian, unsigned AddressSize, const MCRegisterInfo *MRI, DWARFUnit *U) { - DWARFDataExtractor Extractor(StringRef(Data.data(), Data.size()), - IsLittleEndian, AddressSize); + DWARFDataExtractor Extractor(toStringRef(Data), IsLittleEndian, AddressSize); DWARFExpression(Extractor, dwarf::DWARF_VERSION, AddressSize).print(OS, MRI, U); } @@ -83,47 +82,37 @@ void DWARFDebugLoc::dump(raw_ostream &OS, const MCRegisterInfo *MRI, } } -Optional -DWARFDebugLoc::parseOneLocationList(DWARFDataExtractor Data, uint64_t *Offset) { +Expected +DWARFDebugLoc::parseOneLocationList(const DWARFDataExtractor &Data, + uint64_t *Offset) { LocationList LL; LL.Offset = *Offset; + DataExtractor::Cursor C(*Offset); // 2.6.2 Location Lists // A location list entry consists of: while (true) { Entry E; - if (!Data.isValidOffsetForDataOfSize(*Offset, 2 * Data.getAddressSize())) { - WithColor::error() << "location list overflows the debug_loc section.\n"; - return None; - } // 1. A beginning address offset. ... - E.Begin = Data.getRelocatedAddress(Offset); + E.Begin = Data.getRelocatedAddress(C); // 2. An ending address offset. ... - E.End = Data.getRelocatedAddress(Offset); + E.End = Data.getRelocatedAddress(C); + if (Error Err = C.takeError()) + return std::move(Err); // The end of any given location list is marked by an end of list entry, // which consists of a 0 for the beginning address offset and a 0 for the // ending address offset. - if (E.Begin == 0 && E.End == 0) + if (E.Begin == 0 && E.End == 0) { + *Offset = C.tell(); return LL; - - if (!Data.isValidOffsetForDataOfSize(*Offset, 2)) { - WithColor::error() << "location list overflows the debug_loc section.\n"; - return None; } - unsigned Bytes = Data.getU16(Offset); - if (!Data.isValidOffsetForDataOfSize(*Offset, Bytes)) { - WithColor::error() << "location list overflows the debug_loc section.\n"; - return None; - } + unsigned Bytes = Data.getU16(C); // A single location description describing the location of the object... - StringRef str = Data.getData().substr(*Offset, Bytes); - *Offset += Bytes; - E.Loc.reserve(str.size()); - llvm::copy(str, std::back_inserter(E.Loc)); + Data.getU8(C, E.Loc, Bytes); LL.Entries.push_back(std::move(E)); } } @@ -133,67 +122,65 @@ void DWARFDebugLoc::parse(const DWARFDataExtractor &data) { AddressSize = data.getAddressSize(); uint64_t Offset = 0; - while (data.isValidOffset(Offset + data.getAddressSize() - 1)) { + while (Offset < data.getData().size()) { if (auto LL = parseOneLocationList(data, &Offset)) Locations.push_back(std::move(*LL)); - else + else { + logAllUnhandledErrors(LL.takeError(), WithColor::error()); break; + } } - if (data.isValidOffset(Offset)) - WithColor::error() << "failed to consume entire .debug_loc section\n"; } -Optional -DWARFDebugLoclists::parseOneLocationList(DataExtractor Data, uint64_t *Offset, - unsigned Version) { +Expected +DWARFDebugLoclists::parseOneLocationList(const DataExtractor &Data, + uint64_t *Offset, unsigned Version) { LocationList LL; LL.Offset = *Offset; + DataExtractor::Cursor C(*Offset); // dwarf::DW_LLE_end_of_list_entry is 0 and indicates the end of the list. - while (auto Kind = - static_cast(Data.getU8(Offset))) { - + while (auto Kind = static_cast(Data.getU8(C))) { Entry E; E.Kind = Kind; switch (Kind) { case dwarf::DW_LLE_startx_length: - E.Value0 = Data.getULEB128(Offset); + E.Value0 = Data.getULEB128(C); // Pre-DWARF 5 has different interpretation of the length field. We have // to support both pre- and standartized styles for the compatibility. if (Version < 5) - E.Value1 = Data.getU32(Offset); + E.Value1 = Data.getU32(C); else - E.Value1 = Data.getULEB128(Offset); + E.Value1 = Data.getULEB128(C); break; case dwarf::DW_LLE_start_length: - E.Value0 = Data.getAddress(Offset); - E.Value1 = Data.getULEB128(Offset); + E.Value0 = Data.getAddress(C); + E.Value1 = Data.getULEB128(C); break; case dwarf::DW_LLE_offset_pair: - E.Value0 = Data.getULEB128(Offset); - E.Value1 = Data.getULEB128(Offset); + E.Value0 = Data.getULEB128(C); + E.Value1 = Data.getULEB128(C); break; case dwarf::DW_LLE_base_address: - E.Value0 = Data.getAddress(Offset); + E.Value0 = Data.getAddress(C); break; default: - WithColor::error() << "dumping support for LLE of kind " << (int)Kind - << " not implemented\n"; - return None; + cantFail(C.takeError()); + return createStringError(errc::illegal_byte_sequence, + "LLE of kind %x not supported", (int)Kind); } if (Kind != dwarf::DW_LLE_base_address) { - unsigned Bytes = - Version >= 5 ? Data.getULEB128(Offset) : Data.getU16(Offset); + unsigned Bytes = Version >= 5 ? Data.getULEB128(C) : Data.getU16(C); // A single location description describing the location of the object... - StringRef str = Data.getData().substr(*Offset, Bytes); - *Offset += Bytes; - E.Loc.resize(str.size()); - llvm::copy(str, E.Loc.begin()); + Data.getU8(C, E.Loc, Bytes); } LL.Entries.push_back(std::move(E)); } + if (Error Err = C.takeError()) + return std::move(Err); + *Offset = C.tell(); return LL; } @@ -202,11 +189,13 @@ void DWARFDebugLoclists::parse(DataExtractor data, unsigned Version) { AddressSize = data.getAddressSize(); uint64_t Offset = 0; - while (data.isValidOffset(Offset)) { + while (Offset < data.getData().size()) { if (auto LL = parseOneLocationList(data, &Offset, Version)) Locations.push_back(std::move(*LL)); - else + else { + logAllUnhandledErrors(LL.takeError(), WithColor::error()); return; + } } } diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index f2fb7ef210822..896b96a3966b3 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -466,9 +466,9 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, ReportError("DIE has invalid DW_AT_stmt_list encoding:"); break; case DW_AT_location: { - auto VerifyLocationExpr = [&](StringRef D) { + auto VerifyLocationExpr = [&](ArrayRef D) { DWARFUnit *U = Die.getDwarfUnit(); - DataExtractor Data(D, DCtx.isLittleEndian(), 0); + DataExtractor Data(toStringRef(D), DCtx.isLittleEndian(), 0); DWARFExpression Expression(Data, U->getVersion(), U->getAddressByteSize()); bool Error = llvm::any_of(Expression, [](DWARFExpression::Operation &Op) { @@ -479,7 +479,7 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, }; if (Optional> Expr = AttrValue.Value.getAsBlock()) { // Verify inlined location. - VerifyLocationExpr(llvm::toStringRef(*Expr)); + VerifyLocationExpr(*Expr); } else if (auto LocOffset = AttrValue.Value.getAsSectionOffset()) { // Verify location list. if (auto DebugLoc = DCtx.getDebugLoc()) @@ -1277,9 +1277,9 @@ static bool isVariableIndexable(const DWARFDie &Die, DWARFContext &DCtx) { if (!Location) return false; - auto ContainsInterestingOperators = [&](StringRef D) { + auto ContainsInterestingOperators = [&](ArrayRef D) { DWARFUnit *U = Die.getDwarfUnit(); - DataExtractor Data(D, DCtx.isLittleEndian(), U->getAddressByteSize()); + DataExtractor Data(toStringRef(D), DCtx.isLittleEndian(), U->getAddressByteSize()); DWARFExpression Expression(Data, U->getVersion(), U->getAddressByteSize()); return any_of(Expression, [](DWARFExpression::Operation &Op) { return !Op.isError() && (Op.getCode() == DW_OP_addr || @@ -1290,7 +1290,7 @@ static bool isVariableIndexable(const DWARFDie &Die, DWARFContext &DCtx) { if (Optional> Expr = Location->getAsBlock()) { // Inlined location. - if (ContainsInterestingOperators(toStringRef(*Expr))) + if (ContainsInterestingOperators(*Expr)) return true; } else if (Optional Offset = Location->getAsSectionOffset()) { // Location list. diff --git a/llvm/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s b/llvm/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s new file mode 100644 index 0000000000000..092765d4fa3cb --- /dev/null +++ b/llvm/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s @@ -0,0 +1,58 @@ +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE1=0 -o %t1.o +# RUN: llvm-dwarfdump -debug-loc %t1.o 2>&1 | FileCheck %s + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE2=0 -o %t2.o +# RUN: llvm-dwarfdump -debug-loc %t2.o 2>&1 | FileCheck %s + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE3=0 -o %t3.o +# RUN: llvm-dwarfdump -debug-loc %t3.o 2>&1 | FileCheck %s + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE4=0 -o %t4.o +# RUN: llvm-dwarfdump -debug-loc %t4.o 2>&1 | FileCheck %s + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE5=0 -o %t5.o +# RUN: llvm-dwarfdump -debug-loc %t5.o 2>&1 | FileCheck %s + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE6=0 -o %t6.o +# RUN: llvm-dwarfdump -debug-loc %t6.o 2>&1 | FileCheck %s + +# CHECK: error: unexpected end of data + +.section .debug_loc,"",@progbits +.ifdef CASE1 + .byte 1 # bogus +.endif +.ifdef CASE2 + .long 0 # starting offset +.endif +.ifdef CASE3 + .long 0 # starting offset + .long 1 # ending offset +.endif +.ifdef CASE4 + .long 0 # starting offset + .long 1 # ending offset + .word 0 # Loc expr size +.endif +.ifdef CASE5 + .long 0 # starting offset + .long 1 # ending offset + .word 0 # Loc expr size + .long 0 # starting offset +.endif +.ifdef CASE6 + .long 0 # starting offset + .long 1 # ending offset + .word 0xffff # Loc expr size +.endif + +# A minimal compile unit is needed to deduce the address size of the location +# lists +.section .debug_info,"",@progbits + .long .Lcu_end0-.Lcu_begin0 # Length of Unit +.Lcu_begin0: + .short 4 # DWARF version number + .long 0 # Offset Into Abbrev. Section + .byte 8 # Address Size (in bytes) + .byte 0 # End Of Children Mark +.Lcu_end0: diff --git a/llvm/test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s b/llvm/test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s new file mode 100644 index 0000000000000..bb83684d60289 --- /dev/null +++ b/llvm/test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s @@ -0,0 +1,71 @@ +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE1=0 -o %t1.o +# RUN: llvm-dwarfdump -debug-loclists %t1.o 2>&1 | FileCheck %s --check-prefix=ULEB + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE2=0 -o %t2.o +# RUN: llvm-dwarfdump -debug-loclists %t2.o 2>&1 | FileCheck %s --check-prefix=ULEB + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE3=0 -o %t3.o +# RUN: llvm-dwarfdump -debug-loclists %t3.o 2>&1 | FileCheck %s --check-prefix=ULEB + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE4=0 -o %t4.o +# RUN: llvm-dwarfdump -debug-loclists %t4.o 2>&1 | FileCheck %s + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE5=0 -o %t5.o +# RUN: llvm-dwarfdump -debug-loclists %t5.o 2>&1 | FileCheck %s + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE6=0 -o %t6.o +# RUN: llvm-dwarfdump -debug-loclists %t6.o 2>&1 | FileCheck %s + +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE7=0 -o %t7.o +# RUN: llvm-dwarfdump -debug-loclists %t7.o 2>&1 | FileCheck %s --check-prefix=UNIMPL + +# CHECK: error: unexpected end of data +# ULEB: error: malformed uleb128, extends past end +# UNIMPL: error: LLE of kind 47 not supported + +.section .debug_loclists,"",@progbits + .long .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0 +.Ldebug_loclist_table_start0: + .short 5 # Version. + .byte 8 # Address size. + .byte 0 # Segment selector size. + .long 0 # Offset entry count. +.Lloclists_table_base0: +.Ldebug_loc0: +.ifdef CASE1 + .byte 4 # DW_LLE_offset_pair +.endif +.ifdef CASE2 + .byte 4 # DW_LLE_offset_pair + .uleb128 0x0 # starting offset +.endif +.ifdef CASE3 + .byte 4 # DW_LLE_offset_pair + .uleb128 0x0 # starting offset + .uleb128 0x10 # ending offset +.endif +.ifdef CASE4 + .byte 4 # DW_LLE_offset_pair + .uleb128 0x0 # starting offset + .uleb128 0x10 # ending offset + .byte 1 # Loc expr size +.endif +.ifdef CASE5 + .byte 4 # DW_LLE_offset_pair + .uleb128 0x0 # starting offset + .uleb128 0x10 # ending offset + .byte 1 # Loc expr size + .byte 117 # DW_OP_breg5 +.endif +.ifdef CASE6 + .byte 4 # DW_LLE_offset_pair + .uleb128 0x0 # starting offset + .uleb128 0x10 # ending offset + .uleb128 0xdeadbeef # Loc expr size +.endif +.ifdef CASE7 + .byte 0x47 +.endif + +.Ldebug_loclist_table_end0: + From b9ce527f13f23f127f59af41ab9df872b5c78212 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 4 Sep 2019 10:09:12 +0000 Subject: [PATCH 2/4] DWARF: Fix a regression in location list dumping Summary: While fixing the handling of some error cases, r370363 introduced new problems -- assertion failures due to unchecked errors (my excuse is that a very early version of that patch used Optional instead of Expected). This patch adds proper handling of parsing errors encountered when dumping location lists from inside DWARF DIEs, and adds a bunch of additional tests. I reorder the arguments of the location list dumping functions to make them consistent, and also be able to dump the two kinds of location lists generically. Reviewers: JDevlieghere, dblaikie, probinson Subscribers: aprantl, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D67102 llvm-svn: 370868 (cherry picked from commit 88b4e28a679a5aaa14ef41a1901d3d24ddd8946b) --- .../llvm/DebugInfo/DWARF/DWARFDebugLoc.h | 4 +- llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp | 9 +- llvm/lib/DebugInfo/DWARF/DWARFDie.cpp | 39 +++--- .../X86/dwarfdump-debug-loc-error-cases2.s | 121 ++++++++++++++++ .../dwarfdump-debug-loclists-error-cases2.s | 132 ++++++++++++++++++ 5 files changed, 277 insertions(+), 28 deletions(-) create mode 100644 llvm/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases2.s create mode 100644 llvm/test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases2.s diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h index 3852312745b02..dcc47346fa4d5 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h @@ -40,8 +40,8 @@ class DWARFDebugLoc { /// All the locations in which the variable is stored. SmallVector Entries; /// Dump this list on OS. - void dump(raw_ostream &OS, bool IsLittleEndian, unsigned AddressSize, - const MCRegisterInfo *MRI, DWARFUnit *U, uint64_t BaseAddress, + void dump(raw_ostream &OS, uint64_t BaseAddress, bool IsLittleEndian, + unsigned AddressSize, const MCRegisterInfo *MRI, DWARFUnit *U, unsigned Indent) const; }; diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp index e932f98204e2e..5507448ff5986 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp @@ -35,11 +35,10 @@ static void dumpExpression(raw_ostream &OS, ArrayRef Data, DWARFExpression(Extractor, dwarf::DWARF_VERSION, AddressSize).print(OS, MRI, U); } -void DWARFDebugLoc::LocationList::dump(raw_ostream &OS, bool IsLittleEndian, +void DWARFDebugLoc::LocationList::dump(raw_ostream &OS, uint64_t BaseAddress, + bool IsLittleEndian, unsigned AddressSize, - const MCRegisterInfo *MRI, - DWARFUnit *U, - uint64_t BaseAddress, + const MCRegisterInfo *MRI, DWARFUnit *U, unsigned Indent) const { for (const Entry &E : Entries) { OS << '\n'; @@ -67,7 +66,7 @@ void DWARFDebugLoc::dump(raw_ostream &OS, const MCRegisterInfo *MRI, Optional Offset) const { auto DumpLocationList = [&](const LocationList &L) { OS << format("0x%8.8" PRIx64 ": ", L.Offset); - L.dump(OS, IsLittleEndian, AddressSize, MRI, nullptr, 0, 12); + L.dump(OS, 0, IsLittleEndian, AddressSize, MRI, nullptr, 12); OS << "\n\n"; }; diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp index 3547eb1825d32..0f90f221877ee 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp @@ -21,6 +21,7 @@ #include "llvm/Object/ObjectFile.h" #include "llvm/Support/DataExtractor.h" #include "llvm/Support/Format.h" +#include "llvm/Support/FormatAdapters.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/MathExtras.h" #include "llvm/Support/WithColor.h" @@ -91,21 +92,27 @@ static void dumpLocation(raw_ostream &OS, DWARFFormValue &FormValue, } FormValue.dump(OS, DumpOpts); + const auto &DumpLL = [&](auto ExpectedLL) { + if (ExpectedLL) { + uint64_t BaseAddr = 0; + if (Optional BA = U->getBaseAddress()) + BaseAddr = BA->Address; + ExpectedLL->dump(OS, BaseAddr, Ctx.isLittleEndian(), Obj.getAddressSize(), + MRI, U, Indent); + } else { + OS << '\n'; + OS.indent(Indent); + OS << formatv("error extracting location list: {0}", + fmt_consume(ExpectedLL.takeError())); + } + }; if (FormValue.isFormClass(DWARFFormValue::FC_SectionOffset)) { uint64_t Offset = *FormValue.getAsSectionOffset(); if (!U->isDWOUnit() && !U->getLocSection()->Data.empty()) { DWARFDebugLoc DebugLoc; DWARFDataExtractor Data(Obj, *U->getLocSection(), Ctx.isLittleEndian(), Obj.getAddressSize()); - auto LL = DebugLoc.parseOneLocationList(Data, &Offset); - if (LL) { - uint64_t BaseAddr = 0; - if (Optional BA = U->getBaseAddress()) - BaseAddr = BA->Address; - LL->dump(OS, Ctx.isLittleEndian(), Obj.getAddressSize(), MRI, U, - BaseAddr, Indent); - } else - OS << "error extracting location list."; + DumpLL(DebugLoc.parseOneLocationList(Data, &Offset)); return; } @@ -121,18 +128,8 @@ static void dumpLocation(raw_ostream &OS, DWARFFormValue &FormValue, // Modern locations list (.debug_loclists) are used starting from v5. // Ideally we should take the version from the .debug_loclists section // header, but using CU's version for simplicity. - auto LL = DWARFDebugLoclists::parseOneLocationList( - Data, &Offset, UseLocLists ? U->getVersion() : 4); - - uint64_t BaseAddr = 0; - if (Optional BA = U->getBaseAddress()) - BaseAddr = BA->Address; - - if (LL) - LL->dump(OS, BaseAddr, Ctx.isLittleEndian(), Obj.getAddressSize(), MRI, - U, Indent); - else - OS << "error extracting location list."; + DumpLL(DWARFDebugLoclists::parseOneLocationList( + Data, &Offset, UseLocLists ? U->getVersion() : 4)); } } } diff --git a/llvm/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases2.s b/llvm/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases2.s new file mode 100644 index 0000000000000..6f9794379c4f9 --- /dev/null +++ b/llvm/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases2.s @@ -0,0 +1,121 @@ +# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t +# RUN: llvm-dwarfdump %t | FileCheck %s + +# CHECK: DW_AT_name ("x0") +# CHECK-NEXT: DW_AT_location (0x00000000 +# CHECK-NEXT: [0x0000000000000000, 0x0000000000000002): DW_OP_reg5 RDI +# CHECK-NEXT: [0x0000000000000002, 0x0000000000000003): DW_OP_reg0 RAX) + +# CHECK: DW_AT_name ("x1") +# CHECK-NEXT: DW_AT_location (0xdeadbeef +# CHECK-NEXT: error extracting location list: unexpected end of data) + +# CHECK: DW_AT_name ("x2") +# CHECK-NEXT: DW_AT_location (0x00000036 +# CHECK-NEXT: error extracting location list: unexpected end of data) + + + .type f,@function +f: # @f +.Lfunc_begin0: + movl %edi, %eax +.Ltmp0: + retq +.Ltmp1: +.Lfunc_end0: + .size f, .Lfunc_end0-f + + .section .debug_str,"MS",@progbits,1 +.Linfo_string0: + .asciz "Hand-written DWARF" +.Linfo_string3: + .asciz "f" +.Linfo_string4: + .asciz "int" +.Lx0: + .asciz "x0" +.Lx1: + .asciz "x1" +.Lx2: + .asciz "x2" + + .section .debug_loc,"",@progbits +.Ldebug_loc0: + .quad .Lfunc_begin0-.Lfunc_begin0 + .quad .Ltmp0-.Lfunc_begin0 + .short 1 # Loc expr size + .byte 85 # super-register DW_OP_reg5 + .quad .Ltmp0-.Lfunc_begin0 + .quad .Lfunc_end0-.Lfunc_begin0 + .short 1 # Loc expr size + .byte 80 # super-register DW_OP_reg0 + .quad 0 + .quad 0 +.Ldebug_loc2: + .quad .Lfunc_begin0-.Lfunc_begin0 + .quad .Lfunc_end0-.Lfunc_begin0 + .short 0xdead # Loc expr size + + .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 14 # DW_FORM_strp + .byte 19 # DW_AT_language + .byte 5 # DW_FORM_data2 + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 2 # Abbreviation Code + .byte 46 # DW_TAG_subprogram + .byte 1 # DW_CHILDREN_yes + .byte 3 # DW_AT_name + .byte 14 # DW_FORM_strp + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 3 # Abbreviation Code + .byte 5 # DW_TAG_formal_parameter + .byte 0 # DW_CHILDREN_no + .byte 3 # DW_AT_name + .byte 14 # DW_FORM_strp + .byte 2 # DW_AT_location + .byte 23 # DW_FORM_sec_offset + .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 14 # DW_FORM_strp + .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 .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit +.Ldebug_info_start0: + .short 4 # DWARF version number + .long .debug_abbrev # Offset Into Abbrev. Section + .byte 8 # Address Size (in bytes) + .byte 1 # Abbrev [1] 0xb:0x50 DW_TAG_compile_unit + .long .Linfo_string0 # DW_AT_producer + .short 12 # DW_AT_language + .byte 2 # Abbrev [2] 0x2a:0x29 DW_TAG_subprogram + .long .Linfo_string3 # DW_AT_name + .byte 3 # Abbrev [3] DW_TAG_formal_parameter + .long .Lx0 # DW_AT_name + .long .Ldebug_loc0 # DW_AT_location + .byte 3 # Abbrev [3] DW_TAG_formal_parameter + .long .Lx1 # DW_AT_name + .long 0xdeadbeef # DW_AT_location + .byte 3 # Abbrev [3] DW_TAG_formal_parameter + .long .Lx2 # DW_AT_name + .long .Ldebug_loc2 # DW_AT_location + .byte 0 # End Of Children Mark + .byte 0 # End Of Children Mark +.Ldebug_info_end0: diff --git a/llvm/test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases2.s b/llvm/test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases2.s new file mode 100644 index 0000000000000..47307e2d11383 --- /dev/null +++ b/llvm/test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases2.s @@ -0,0 +1,132 @@ +# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t +# RUN: llvm-dwarfdump %t | FileCheck %s + +# CHECK: DW_AT_name ("x0") +# CHECK-NEXT: DW_AT_location (0x0000000c +# CHECK-NEXT: [0x0000000000000000, 0x0000000000000002): DW_OP_reg5 RDI +# CHECK-NEXT: [0x0000000000000002, 0x0000000000000003): DW_OP_reg0 RAX) + +# CHECK: DW_AT_name ("x1") +# CHECK-NEXT: DW_AT_location (0xdeadbeef +# CHECK-NEXT: error extracting location list: unexpected end of data) + +# CHECK: DW_AT_name ("x2") +# CHECK-NEXT: DW_AT_location (0x00000025 +# CHECK-NEXT: error extracting location list: unexpected end of data) + + + .type f,@function +f: # @f +.Lfunc_begin0: + movl %edi, %eax +.Ltmp0: + retq +.Ltmp1: +.Lfunc_end0: + .size f, .Lfunc_end0-f + + .section .debug_str,"MS",@progbits,1 +.Linfo_string0: + .asciz "Hand-written DWARF" +.Linfo_string3: + .asciz "f" +.Linfo_string4: + .asciz "int" +.Lx0: + .asciz "x0" +.Lx1: + .asciz "x1" +.Lx2: + .asciz "x2" + + .section .debug_loclists,"",@progbits + .long .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0 # Length +.Ldebug_loclist_table_start0: + .short 5 # Version + .byte 8 # Address size + .byte 0 # Segment selector size + .long 0 # Offset entry count +.Lloclists_table_base0: +.Ldebug_loc0: + .byte 8 # DW_LLE_start_length + .quad .Lfunc_begin0-.Lfunc_begin0 # starting offset + .uleb128 .Ltmp0-.Lfunc_begin0 # size + .byte 1 # Loc expr size + .byte 85 # super-register DW_OP_reg5 + .byte 8 # DW_LLE_start_length + .quad .Ltmp0-.Lfunc_begin0 # starting offset + .uleb128 .Lfunc_end0-.Ltmp0 # size + .byte 1 # Loc expr size + .byte 80 # super-register DW_OP_reg0 + .byte 0 # DW_LLE_end_of_list +.Ldebug_loc2: + .byte 8 # DW_LLE_start_length + .quad .Lfunc_begin0-.Lfunc_begin0 # starting offset + .uleb128 .Ltmp0-.Lfunc_begin0 # size + .uleb128 0xdeadbeef # Loc expr size +.Ldebug_loclist_table_end0: + + .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 14 # DW_FORM_strp + .byte 19 # DW_AT_language + .byte 5 # DW_FORM_data2 + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 2 # Abbreviation Code + .byte 46 # DW_TAG_subprogram + .byte 1 # DW_CHILDREN_yes + .byte 3 # DW_AT_name + .byte 14 # DW_FORM_strp + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 3 # Abbreviation Code + .byte 5 # DW_TAG_formal_parameter + .byte 0 # DW_CHILDREN_no + .byte 3 # DW_AT_name + .byte 14 # DW_FORM_strp + .byte 2 # DW_AT_location + .byte 23 # DW_FORM_sec_offset + .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 14 # DW_FORM_strp + .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 .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit +.Ldebug_info_start0: + .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] 0xb:0x50 DW_TAG_compile_unit + .long .Linfo_string0 # DW_AT_producer + .short 12 # DW_AT_language + .byte 2 # Abbrev [2] 0x2a:0x29 DW_TAG_subprogram + .long .Linfo_string3 # DW_AT_name + .byte 3 # Abbrev [3] DW_TAG_formal_parameter + .long .Lx0 # DW_AT_name + .long .Ldebug_loc0 # DW_AT_location + .byte 3 # Abbrev [3] DW_TAG_formal_parameter + .long .Lx1 # DW_AT_name + .long 0xdeadbeef # DW_AT_location + .byte 3 # Abbrev [3] DW_TAG_formal_parameter + .long .Lx2 # DW_AT_name + .long .Ldebug_loc2 # DW_AT_location + .byte 0 # End Of Children Mark + .byte 0 # End Of Children Mark +.Ldebug_info_end0: From d488f7869c0512471b88dbf95c9252cae27db89b Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 27 Aug 2019 11:24:08 +0000 Subject: [PATCH 3/4] Add error handling to the DataExtractor class Summary: This is motivated by D63591, where we realized that there isn't a really good way of telling whether a DataExtractor is reading actual data, or is it just returning default values because it reached the end of the buffer. This patch resolves that by providing a new "Cursor" class. A Cursor object encapsulates two things: - the current position/offset in the DataExtractor - an error object Storing the error object inside the Cursor enables one to use the same pattern as the std::{io}stream API, where one can blindly perform a sequence of reads and only check for errors once at the end of the operation. Similarly to the stream API, as soon as we encounter one error, all of the subsequent operations are skipped (return default values) too, even if the would suceed with clear error state. Unlike the std::stream API (but in line with other llvm APIs), we force the error state to be checked through usage of llvm::Error. Reviewers: probinson, dblaikie, JDevlieghere, aprantl, echristo Subscribers: kristina, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D63713 llvm-svn: 370042 (cherry picked from commit b1f29cec251188a594148f7a53d063281d4ef155) --- .../llvm/DebugInfo/DWARF/DWARFDataExtractor.h | 7 +- llvm/include/llvm/Support/DataExtractor.h | 155 +++++++++++++++++- .../DebugInfo/DWARF/DWARFDataExtractor.cpp | 7 +- llvm/lib/Support/DataExtractor.cpp | 127 +++++++++----- llvm/unittests/Support/DataExtractorTest.cpp | 143 ++++++++++++++++ 5 files changed, 388 insertions(+), 51 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDataExtractor.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDataExtractor.h index 059aa82fccda4..90f2caf80c751 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDataExtractor.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDataExtractor.h @@ -36,13 +36,18 @@ class DWARFDataExtractor : public DataExtractor { /// Extracts a value and applies a relocation to the result if /// one exists for the given offset. uint64_t getRelocatedValue(uint32_t Size, uint64_t *Off, - uint64_t *SectionIndex = nullptr) const; + uint64_t *SectionIndex = nullptr, + Error *Err = nullptr) const; /// Extracts an address-sized value and applies a relocation to the result if /// one exists for the given offset. uint64_t getRelocatedAddress(uint64_t *Off, uint64_t *SecIx = nullptr) const { return getRelocatedValue(getAddressSize(), Off, SecIx); } + uint64_t getRelocatedAddress(Cursor &C, uint64_t *SecIx = nullptr) const { + return getRelocatedValue(getAddressSize(), &getOffset(C), SecIx, + &getError(C)); + } /// Extracts a DWARF-encoded pointer in \p Offset using \p Encoding. /// There is a DWARF encoding that uses a PC-relative adjustment. diff --git a/llvm/include/llvm/Support/DataExtractor.h b/llvm/include/llvm/Support/DataExtractor.h index 7c458aaf1cac2..bd337f23925c9 100644 --- a/llvm/include/llvm/Support/DataExtractor.h +++ b/llvm/include/llvm/Support/DataExtractor.h @@ -11,6 +11,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/DataTypes.h" +#include "llvm/Support/Error.h" namespace llvm { @@ -42,6 +43,38 @@ class DataExtractor { uint8_t IsLittleEndian; uint8_t AddressSize; public: + /// A class representing a position in a DataExtractor, as well as any error + /// encountered during extraction. It enables one to extract a sequence of + /// values without error-checking and then checking for errors in bulk at the + /// end. The class holds an Error object, so failing to check the result of + /// the parse will result in a runtime error. The error flag is sticky and + /// will cause all subsequent extraction functions to fail without even + /// attempting to parse and without updating the Cursor offset. After clearing + /// the error flag, one can again use the Cursor object for parsing. + class Cursor { + uint64_t Offset; + Error Err; + + friend class DataExtractor; + + public: + /// Construct a cursor for extraction from the given offset. + explicit Cursor(uint64_t Offset) : Offset(Offset), Err(Error::success()) {} + + /// Checks whether the cursor is valid (i.e. no errors were encountered). In + /// case of errors, this does not clear the error flag -- one must call + /// takeError() instead. + explicit operator bool() { return !Err; } + + /// Return the current position of this Cursor. In the error state this is + /// the position of the Cursor before the first error was encountered. + uint64_t tell() const { return Offset; } + + /// Return error contained inside this Cursor, if any. Clears the internal + /// Cursor state. + Error takeError() { return std::move(Err); } + }; + /// Construct with a buffer that is owned by the caller. /// /// This constructor allows us to use data that is owned by the @@ -124,10 +157,24 @@ class DataExtractor { /// @param[in] byte_size /// The size in byte of the integer to extract. /// + /// @param[in,out] Err + /// A pointer to an Error object. Upon return the Error object is set to + /// indicate the result (success/failure) of the function. If the Error + /// object is already set when calling this function, no extraction is + /// performed. + /// /// @return /// The unsigned integer value that was extracted, or zero on /// failure. - uint64_t getUnsigned(uint64_t *offset_ptr, uint32_t byte_size) const; + uint64_t getUnsigned(uint64_t *offset_ptr, uint32_t byte_size, + Error *Err = nullptr) const; + + /// Extract an unsigned integer of the given size from the location given by + /// the cursor. In case of an extraction error, or if the cursor is already in + /// an error state, zero is returned. + uint64_t getUnsigned(Cursor &C, uint32_t Size) const { + return getUnsigned(&C.Offset, Size, &C.Err); + } /// Extract an signed integer of size \a byte_size from \a *offset_ptr. /// @@ -175,6 +222,11 @@ class DataExtractor { return getUnsigned(offset_ptr, AddressSize); } + /// Extract a pointer-sized unsigned integer from the location given by the + /// cursor. In case of an extraction error, or if the cursor is already in + /// an error state, zero is returned. + uint64_t getAddress(Cursor &C) const { return getUnsigned(C, AddressSize); } + /// Extract a uint8_t value from \a *offset_ptr. /// /// Extract a single uint8_t from the binary data at the offset @@ -187,9 +239,20 @@ class DataExtractor { /// enough bytes to extract this value, the offset will be left /// unmodified. /// + /// @param[in,out] Err + /// A pointer to an Error object. Upon return the Error object is set to + /// indicate the result (success/failure) of the function. If the Error + /// object is already set when calling this function, no extraction is + /// performed. + /// /// @return /// The extracted uint8_t value. - uint8_t getU8(uint64_t *offset_ptr) const; + uint8_t getU8(uint64_t *offset_ptr, Error *Err = nullptr) const; + + /// Extract a single uint8_t value from the location given by the cursor. In + /// case of an extraction error, or if the cursor is already in an error + /// state, zero is returned. + uint8_t getU8(Cursor &C) const { return getU8(&C.Offset, &C.Err); } /// Extract \a count uint8_t values from \a *offset_ptr. /// @@ -216,6 +279,26 @@ class DataExtractor { /// NULL otherise. uint8_t *getU8(uint64_t *offset_ptr, uint8_t *dst, uint32_t count) const; + /// Extract \a Count uint8_t values from the location given by the cursor and + /// store them into the destination buffer. In case of an extraction error, or + /// if the cursor is already in an error state, a nullptr is returned and the + /// destination buffer is left unchanged. + uint8_t *getU8(Cursor &C, uint8_t *Dst, uint32_t Count) const; + + /// Extract \a Count uint8_t values from the location given by the cursor and + /// store them into the destination vector. The vector is resized to fit the + /// extracted data. In case of an extraction error, or if the cursor is + /// already in an error state, the destination vector is left unchanged and + /// cursor is placed into an error state. + void getU8(Cursor &C, SmallVectorImpl &Dst, uint32_t Count) const { + if (isValidOffsetForDataOfSize(C.Offset, Count)) + Dst.resize(Count); + + // This relies on the fact that getU8 will not attempt to write to the + // buffer if isValidOffsetForDataOfSize(C.Offset, Count) is false. + getU8(C, Dst.data(), Count); + } + //------------------------------------------------------------------ /// Extract a uint16_t value from \a *offset_ptr. /// @@ -229,10 +312,21 @@ class DataExtractor { /// enough bytes to extract this value, the offset will be left /// unmodified. /// + /// @param[in,out] Err + /// A pointer to an Error object. Upon return the Error object is set to + /// indicate the result (success/failure) of the function. If the Error + /// object is already set when calling this function, no extraction is + /// performed. + /// /// @return /// The extracted uint16_t value. //------------------------------------------------------------------ - uint16_t getU16(uint64_t *offset_ptr) const; + uint16_t getU16(uint64_t *offset_ptr, Error *Err = nullptr) const; + + /// Extract a single uint16_t value from the location given by the cursor. In + /// case of an extraction error, or if the cursor is already in an error + /// state, zero is returned. + uint16_t getU16(Cursor &C) const { return getU16(&C.Offset, &C.Err); } /// Extract \a count uint16_t values from \a *offset_ptr. /// @@ -288,9 +382,20 @@ class DataExtractor { /// enough bytes to extract this value, the offset will be left /// unmodified. /// + /// @param[in,out] Err + /// A pointer to an Error object. Upon return the Error object is set to + /// indicate the result (success/failure) of the function. If the Error + /// object is already set when calling this function, no extraction is + /// performed. + /// /// @return /// The extracted uint32_t value. - uint32_t getU32(uint64_t *offset_ptr) const; + uint32_t getU32(uint64_t *offset_ptr, Error *Err = nullptr) const; + + /// Extract a single uint32_t value from the location given by the cursor. In + /// case of an extraction error, or if the cursor is already in an error + /// state, zero is returned. + uint32_t getU32(Cursor &C) const { return getU32(&C.Offset, &C.Err); } /// Extract \a count uint32_t values from \a *offset_ptr. /// @@ -329,9 +434,20 @@ class DataExtractor { /// enough bytes to extract this value, the offset will be left /// unmodified. /// + /// @param[in,out] Err + /// A pointer to an Error object. Upon return the Error object is set to + /// indicate the result (success/failure) of the function. If the Error + /// object is already set when calling this function, no extraction is + /// performed. + /// /// @return /// The extracted uint64_t value. - uint64_t getU64(uint64_t *offset_ptr) const; + uint64_t getU64(uint64_t *offset_ptr, Error *Err = nullptr) const; + + /// Extract a single uint64_t value from the location given by the cursor. In + /// case of an extraction error, or if the cursor is already in an error + /// state, zero is returned. + uint64_t getU64(Cursor &C) const { return getU64(&C.Offset, &C.Err); } /// Extract \a count uint64_t values from \a *offset_ptr. /// @@ -390,9 +506,30 @@ class DataExtractor { /// enough bytes to extract this value, the offset will be left /// unmodified. /// + /// @param[in,out] Err + /// A pointer to an Error object. Upon return the Error object is set to + /// indicate the result (success/failure) of the function. If the Error + /// object is already set when calling this function, no extraction is + /// performed. + /// /// @return /// The extracted unsigned integer value. - uint64_t getULEB128(uint64_t *offset_ptr) const; + uint64_t getULEB128(uint64_t *offset_ptr, llvm::Error *Err = nullptr) const; + + /// Extract an unsigned ULEB128 value from the location given by the cursor. + /// In case of an extraction error, or if the cursor is already in an error + /// state, zero is returned. + uint64_t getULEB128(Cursor &C) const { return getULEB128(&C.Offset, &C.Err); } + + /// Advance the Cursor position by the given number of bytes. No-op if the + /// cursor is in an error state. + void skip(Cursor &C, uint64_t Length) const; + + /// Return true iff the cursor is at the end of the buffer, regardless of the + /// error state of the cursor. The only way both eof and error states can be + /// true is if one attempts a read while the cursor is at the very end of the + /// data buffer. + bool eof(const Cursor &C) const { return Data.size() == C.Offset; } /// Test the validity of \a offset. /// @@ -420,6 +557,12 @@ class DataExtractor { bool isValidOffsetForAddress(uint64_t offset) const { return isValidOffsetForDataOfSize(offset, AddressSize); } + +protected: + // Make it possible for subclasses to access these fields without making them + // public. + static uint64_t &getOffset(Cursor &C) { return C.Offset; } + static Error &getError(Cursor &C) { return C.Err; } }; } // namespace llvm diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDataExtractor.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDataExtractor.cpp index 491f6b08b0ad7..4c4f39bf79627 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFDataExtractor.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFDataExtractor.cpp @@ -13,13 +13,14 @@ using namespace llvm; uint64_t DWARFDataExtractor::getRelocatedValue(uint32_t Size, uint64_t *Off, - uint64_t *SecNdx) const { + uint64_t *SecNdx, + Error *Err) const { if (SecNdx) *SecNdx = object::SectionedAddress::UndefSection; if (!Section) - return getUnsigned(Off, Size); + return getUnsigned(Off, Size, Err); Optional E = Obj->find(*Section, *Off); - uint64_t A = getUnsigned(Off, Size); + uint64_t A = getUnsigned(Off, Size, Err); if (!E) return A; if (SecNdx) diff --git a/llvm/lib/Support/DataExtractor.cpp b/llvm/lib/Support/DataExtractor.cpp index 1d88ec061514c..a98297cdb35f2 100644 --- a/llvm/lib/Support/DataExtractor.cpp +++ b/llvm/lib/Support/DataExtractor.cpp @@ -7,104 +7,131 @@ //===----------------------------------------------------------------------===// #include "llvm/Support/DataExtractor.h" +#include "llvm/Support/Errc.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Host.h" -#include "llvm/Support/SwapByteOrder.h" #include "llvm/Support/LEB128.h" +#include "llvm/Support/SwapByteOrder.h" + using namespace llvm; +static void unexpectedEndReached(Error *E) { + if (E) + *E = createStringError(errc::illegal_byte_sequence, + "unexpected end of data"); +} + +static bool isError(Error *E) { return E && *E; } + template static T getU(uint64_t *offset_ptr, const DataExtractor *de, - bool isLittleEndian, const char *Data) { + bool isLittleEndian, const char *Data, llvm::Error *Err) { + ErrorAsOutParameter ErrAsOut(Err); T val = 0; - uint64_t offset = *offset_ptr; - if (de->isValidOffsetForDataOfSize(offset, sizeof(val))) { - std::memcpy(&val, &Data[offset], sizeof(val)); - if (sys::IsLittleEndianHost != isLittleEndian) - sys::swapByteOrder(val); + if (isError(Err)) + return val; - // Advance the offset - *offset_ptr += sizeof(val); + uint64_t offset = *offset_ptr; + if (!de->isValidOffsetForDataOfSize(offset, sizeof(T))) { + unexpectedEndReached(Err); + return val; } + std::memcpy(&val, &Data[offset], sizeof(val)); + if (sys::IsLittleEndianHost != isLittleEndian) + sys::swapByteOrder(val); + + // Advance the offset + *offset_ptr += sizeof(val); return val; } template static T *getUs(uint64_t *offset_ptr, T *dst, uint32_t count, - const DataExtractor *de, bool isLittleEndian, const char *Data){ + const DataExtractor *de, bool isLittleEndian, const char *Data, + llvm::Error *Err) { + ErrorAsOutParameter ErrAsOut(Err); + if (isError(Err)) + return nullptr; + uint64_t offset = *offset_ptr; - if (count > 0 && de->isValidOffsetForDataOfSize(offset, sizeof(*dst)*count)) { - for (T *value_ptr = dst, *end = dst + count; value_ptr != end; - ++value_ptr, offset += sizeof(*dst)) - *value_ptr = getU(offset_ptr, de, isLittleEndian, Data); - // Advance the offset - *offset_ptr = offset; - // Return a non-NULL pointer to the converted data as an indicator of - // success - return dst; + if (!de->isValidOffsetForDataOfSize(offset, sizeof(*dst) * count)) { + unexpectedEndReached(Err); + return nullptr; } - return nullptr; + for (T *value_ptr = dst, *end = dst + count; value_ptr != end; + ++value_ptr, offset += sizeof(*dst)) + *value_ptr = getU(offset_ptr, de, isLittleEndian, Data, Err); + // Advance the offset + *offset_ptr = offset; + // Return a non-NULL pointer to the converted data as an indicator of + // success + return dst; } -uint8_t DataExtractor::getU8(uint64_t *offset_ptr) const { - return getU(offset_ptr, this, IsLittleEndian, Data.data()); +uint8_t DataExtractor::getU8(uint64_t *offset_ptr, llvm::Error *Err) const { + return getU(offset_ptr, this, IsLittleEndian, Data.data(), Err); } uint8_t * DataExtractor::getU8(uint64_t *offset_ptr, uint8_t *dst, uint32_t count) const { return getUs(offset_ptr, dst, count, this, IsLittleEndian, - Data.data()); + Data.data(), nullptr); +} + +uint8_t *DataExtractor::getU8(Cursor &C, uint8_t *Dst, uint32_t Count) const { + return getUs(&C.Offset, Dst, Count, this, IsLittleEndian, + Data.data(), &C.Err); } -uint16_t DataExtractor::getU16(uint64_t *offset_ptr) const { - return getU(offset_ptr, this, IsLittleEndian, Data.data()); +uint16_t DataExtractor::getU16(uint64_t *offset_ptr, llvm::Error *Err) const { + return getU(offset_ptr, this, IsLittleEndian, Data.data(), Err); } uint16_t *DataExtractor::getU16(uint64_t *offset_ptr, uint16_t *dst, uint32_t count) const { return getUs(offset_ptr, dst, count, this, IsLittleEndian, - Data.data()); + Data.data(), nullptr); } uint32_t DataExtractor::getU24(uint64_t *offset_ptr) const { uint24_t ExtractedVal = - getU(offset_ptr, this, IsLittleEndian, Data.data()); + getU(offset_ptr, this, IsLittleEndian, Data.data(), nullptr); // The 3 bytes are in the correct byte order for the host. return ExtractedVal.getAsUint32(sys::IsLittleEndianHost); } -uint32_t DataExtractor::getU32(uint64_t *offset_ptr) const { - return getU(offset_ptr, this, IsLittleEndian, Data.data()); +uint32_t DataExtractor::getU32(uint64_t *offset_ptr, llvm::Error *Err) const { + return getU(offset_ptr, this, IsLittleEndian, Data.data(), Err); } uint32_t *DataExtractor::getU32(uint64_t *offset_ptr, uint32_t *dst, uint32_t count) const { return getUs(offset_ptr, dst, count, this, IsLittleEndian, - Data.data()); + Data.data(), nullptr); } -uint64_t DataExtractor::getU64(uint64_t *offset_ptr) const { - return getU(offset_ptr, this, IsLittleEndian, Data.data()); +uint64_t DataExtractor::getU64(uint64_t *offset_ptr, llvm::Error *Err) const { + return getU(offset_ptr, this, IsLittleEndian, Data.data(), Err); } uint64_t *DataExtractor::getU64(uint64_t *offset_ptr, uint64_t *dst, uint32_t count) const { return getUs(offset_ptr, dst, count, this, IsLittleEndian, - Data.data()); + Data.data(), nullptr); } -uint64_t -DataExtractor::getUnsigned(uint64_t *offset_ptr, uint32_t byte_size) const { +uint64_t DataExtractor::getUnsigned(uint64_t *offset_ptr, uint32_t byte_size, + llvm::Error *Err) const { switch (byte_size) { case 1: - return getU8(offset_ptr); + return getU8(offset_ptr, Err); case 2: - return getU16(offset_ptr); + return getU16(offset_ptr, Err); case 4: - return getU32(offset_ptr); + return getU32(offset_ptr, Err); case 8: - return getU64(offset_ptr); + return getU64(offset_ptr, Err); } llvm_unreachable("getUnsigned unhandled case!"); } @@ -144,16 +171,23 @@ StringRef DataExtractor::getCStrRef(uint64_t *offset_ptr) const { return StringRef(); } -uint64_t DataExtractor::getULEB128(uint64_t *offset_ptr) const { +uint64_t DataExtractor::getULEB128(uint64_t *offset_ptr, + llvm::Error *Err) const { assert(*offset_ptr <= Data.size()); + ErrorAsOutParameter ErrAsOut(Err); + if (isError(Err)) + return 0; const char *error; unsigned bytes_read; uint64_t result = decodeULEB128( reinterpret_cast(Data.data() + *offset_ptr), &bytes_read, reinterpret_cast(Data.data() + Data.size()), &error); - if (error) + if (error) { + if (Err) + *Err = createStringError(errc::illegal_byte_sequence, error); return 0; + } *offset_ptr += bytes_read; return result; } @@ -171,3 +205,14 @@ int64_t DataExtractor::getSLEB128(uint64_t *offset_ptr) const { *offset_ptr += bytes_read; return result; } + +void DataExtractor::skip(Cursor &C, uint64_t Length) const { + ErrorAsOutParameter ErrAsOut(&C.Err); + if (isError(&C.Err)) + return; + + if (isValidOffsetForDataOfSize(C.Offset, Length)) + C.Offset += Length; + else + unexpectedEndReached(&C.Err); +} diff --git a/llvm/unittests/Support/DataExtractorTest.cpp b/llvm/unittests/Support/DataExtractorTest.cpp index d1c23cd15e5b2..d182715d19954 100644 --- a/llvm/unittests/Support/DataExtractorTest.cpp +++ b/llvm/unittests/Support/DataExtractorTest.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "llvm/Support/DataExtractor.h" +#include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" using namespace llvm; @@ -126,4 +127,146 @@ TEST(DataExtractorTest, LEB128_error) { EXPECT_EQ(0U, DE.getSLEB128(&Offset)); EXPECT_EQ(0U, Offset); } + +TEST(DataExtractorTest, Cursor_tell) { + DataExtractor DE(StringRef("AB"), false, 8); + DataExtractor::Cursor C(0); + // A successful read operation advances the cursor + EXPECT_EQ('A', DE.getU8(C)); + EXPECT_EQ(1u, C.tell()); + + // An unsuccessful one doesn't. + EXPECT_EQ(0u, DE.getU16(C)); + EXPECT_EQ(1u, C.tell()); + + // And neither do any subsequent operations. + EXPECT_EQ(0, DE.getU8(C)); + EXPECT_EQ(1u, C.tell()); + + consumeError(C.takeError()); +} + +TEST(DataExtractorTest, Cursor_takeError) { + DataExtractor DE(StringRef("AB"), false, 8); + DataExtractor::Cursor C(0); + // Initially, the cursor is in the "success" state. + EXPECT_THAT_ERROR(C.takeError(), Succeeded()); + + // It remains "success" after a successful read. + EXPECT_EQ('A', DE.getU8(C)); + EXPECT_THAT_ERROR(C.takeError(), Succeeded()); + + // An unsuccessful read sets the error state. + EXPECT_EQ(0u, DE.getU32(C)); + EXPECT_THAT_ERROR(C.takeError(), Failed()); + + // Once set the error sticks until explicitly cleared. + EXPECT_EQ(0u, DE.getU32(C)); + EXPECT_EQ(0, DE.getU8(C)); + EXPECT_THAT_ERROR(C.takeError(), Failed()); + + // At which point reads can be succeed again. + EXPECT_EQ('B', DE.getU8(C)); + EXPECT_THAT_ERROR(C.takeError(), Succeeded()); +} + +TEST(DataExtractorTest, Cursor_chaining) { + DataExtractor DE(StringRef("ABCD"), false, 8); + DataExtractor::Cursor C(0); + + // Multiple reads can be chained without trigerring any assertions. + EXPECT_EQ('A', DE.getU8(C)); + EXPECT_EQ('B', DE.getU8(C)); + EXPECT_EQ('C', DE.getU8(C)); + EXPECT_EQ('D', DE.getU8(C)); + // And the error checked at the end. + EXPECT_THAT_ERROR(C.takeError(), Succeeded()); +} + +#if defined(GTEST_HAS_DEATH_TEST) && defined(_DEBUG) +TEST(DataExtractorDeathTest, Cursor) { + DataExtractor DE(StringRef("AB"), false, 8); + + // Even an unused cursor must be checked for errors: + EXPECT_DEATH(DataExtractor::Cursor(0), + "Success values must still be checked prior to being destroyed"); + + { + auto C = std::make_unique(0); + EXPECT_EQ(0u, DE.getU32(*C)); + // It must also be checked after an unsuccessful operation. + // destruction. + EXPECT_DEATH(C.reset(), "unexpected end of data"); + EXPECT_THAT_ERROR(C->takeError(), Failed()); + } + { + auto C = std::make_unique(0); + EXPECT_EQ('A', DE.getU8(*C)); + // Same goes for a successful one. + EXPECT_DEATH( + C.reset(), + "Success values must still be checked prior to being destroyed"); + EXPECT_THAT_ERROR(C->takeError(), Succeeded()); + } + { + auto C = std::make_unique(0); + EXPECT_EQ('A', DE.getU8(*C)); + EXPECT_EQ(0u, DE.getU32(*C)); + // Even if a successful operation is followed by an unsuccessful one. + EXPECT_DEATH(C.reset(), "unexpected end of data"); + EXPECT_THAT_ERROR(C->takeError(), Failed()); + } + { + auto C = std::make_unique(0); + EXPECT_EQ(0u, DE.getU32(*C)); + EXPECT_EQ(0, DE.getU8(*C)); + // Even if an unsuccessful operation is followed by one that would normally + // succeed. + EXPECT_DEATH(C.reset(), "unexpected end of data"); + EXPECT_THAT_ERROR(C->takeError(), Failed()); + } +} +#endif + +TEST(DataExtractorTest, getU8_vector) { + DataExtractor DE(StringRef("AB"), false, 8); + DataExtractor::Cursor C(0); + SmallVector S; + + DE.getU8(C, S, 4); + EXPECT_THAT_ERROR(C.takeError(), Failed()); + EXPECT_EQ("", toStringRef(S)); + + DE.getU8(C, S, 2); + EXPECT_THAT_ERROR(C.takeError(), Succeeded()); + EXPECT_EQ("AB", toStringRef(S)); +} + +TEST(DataExtractorTest, skip) { + DataExtractor DE(StringRef("AB"), false, 8); + DataExtractor::Cursor C(0); + + DE.skip(C, 4); + EXPECT_THAT_ERROR(C.takeError(), Failed()); + EXPECT_EQ(0u, C.tell()); + + DE.skip(C, 2); + EXPECT_THAT_ERROR(C.takeError(), Succeeded()); + EXPECT_EQ(2u, C.tell()); +} + +TEST(DataExtractorTest, eof) { + DataExtractor DE(StringRef("A"), false, 8); + DataExtractor::Cursor C(0); + + EXPECT_FALSE(DE.eof(C)); + + EXPECT_EQ(0, DE.getU16(C)); + EXPECT_FALSE(DE.eof(C)); + EXPECT_THAT_ERROR(C.takeError(), Failed()); + + EXPECT_EQ('A', DE.getU8(C)); + EXPECT_TRUE(DE.eof(C)); + EXPECT_THAT_ERROR(C.takeError(), Succeeded()); +} } From b13da177d54f94d47f882a931a76a95f2aa9609f Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 1 Oct 2019 00:29:13 +0000 Subject: [PATCH 4/4] DebugInfo: Add parsing support for debug_loc base address specifiers llvm-svn: 373278 (cherry picked from commit 5ca306666c485b56866c4ed95472e55ee59ab35d) --- .../llvm/DebugInfo/DWARF/DWARFDebugLoc.h | 2 +- llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp | 10 ++++-- .../llvm-dwarfdump/debug_loc_base_address.s | 34 +++++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 llvm/test/tools/llvm-dwarfdump/debug_loc_base_address.s diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h index dcc47346fa4d5..f65a20af0cb1c 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h @@ -68,7 +68,7 @@ class DWARFDebugLoc { /// Return the location list at the given offset or nullptr. LocationList const *getLocationListAtOffset(uint64_t Offset) const; - static Expected + Expected parseOneLocationList(const DWARFDataExtractor &Data, uint64_t *Offset); }; diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp index 5507448ff5986..f25431321bbc9 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp @@ -101,6 +101,7 @@ DWARFDebugLoc::parseOneLocationList(const DWARFDataExtractor &Data, if (Error Err = C.takeError()) return std::move(Err); + // The end of any given location list is marked by an end of list entry, // which consists of a 0 for the beginning address offset and a 0 for the // ending address offset. @@ -109,9 +110,12 @@ DWARFDebugLoc::parseOneLocationList(const DWARFDataExtractor &Data, return LL; } - unsigned Bytes = Data.getU16(C); - // A single location description describing the location of the object... - Data.getU8(C, E.Loc, Bytes); + if (E.Begin != (AddressSize == 4 ? -1U : -1ULL)) { + unsigned Bytes = Data.getU16(C); + // A single location description describing the location of the object... + Data.getU8(C, E.Loc, Bytes); + } + LL.Entries.push_back(std::move(E)); } } diff --git a/llvm/test/tools/llvm-dwarfdump/debug_loc_base_address.s b/llvm/test/tools/llvm-dwarfdump/debug_loc_base_address.s new file mode 100644 index 0000000000000..fde942b2ebf8d --- /dev/null +++ b/llvm/test/tools/llvm-dwarfdump/debug_loc_base_address.s @@ -0,0 +1,34 @@ +# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux -o %t.o +# RUN: llvm-dwarfdump --debug-loc %t.o | FileCheck %s + +# CHECK: .debug_loc contents: +# CHECK-NEXT: 0x00000000: +# CHECK-NEXT: [0xffffffffffffffff, 0x000000000000002a): +# CHECK-NEXT: [0x0000000000000003, 0x0000000000000007): DW_OP_consts +3, DW_OP_stack_value + + .section .debug_loc,"",@progbits + .quad 0xffffffffffffffff + .quad 42 + .quad 3 + .quad 7 + .short 3 # Loc expr size + .byte 17 # DW_OP_consts + .byte 3 # 3 + .byte 159 # DW_OP_stack_value + .quad 0 + .quad 0 + .section .debug_abbrev,"",@progbits + .byte 1 # Abbreviation Code + .byte 17 # DW_TAG_compile_unit + .byte 0 # DW_CHILDREN_no + .byte 0 # EOM(1) + .byte 0 # EOM(2) + .byte 0 # EOM(3) + .section .debug_info,"",@progbits + .long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit +.Ldebug_info_start0: + .short 4 # DWARF version number + .long .debug_abbrev # Offset Into Abbrev. Section + .byte 8 # Address Size (in bytes) + .byte 1 # Abbrev [1] DW_TAG_compile_unit +.Ldebug_info_end0: