labath created this revision.
labath added reviewers: dblaikie, JDevlieghere, probinson.
Herald added a project: LLVM.
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 Expected<T>s. 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 making the parser
explictly check for success. This makes the code a bit clunky, as the
DataExtractor API makes it really hard to do that, but this is the best
implementation I came up with given the current API.
I also add tests for handling the error cases, as they currently have no
coverage.
Repository:
rL LLVM
https://reviews.llvm.org/D63591
Files:
include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
lib/DebugInfo/DWARF/DWARFDie.cpp
test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s
Index: test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s
===================================================================
--- /dev/null
+++ test/DebugInfo/X86/dwarfdump-debug-loclists-error-cases.s
@@ -0,0 +1,62 @@
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE1=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loclists %t.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE2=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loclists %t.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE3=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loclists %t.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE4=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loclists %t.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE5=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loclists %t.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE6=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loclists %t.o 2>&1 | FileCheck %s --check-prefix=UNIMPL
+
+# CHECK: error: location list overflows the debug_loclists section
+
+# UNIMPL: error: LLE of kind 47 not implemented
+
+.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 0x47
+.endif
+
+.Ldebug_loclist_table_end0:
+
Index: test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
===================================================================
--- /dev/null
+++ test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s
@@ -0,0 +1,52 @@
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE1=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loc %t.o 2>&1 | FileCheck %s --check-prefix=CONSUME
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE2=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loc %t.o 2>&1 | FileCheck %s --check-prefix=CONSUME
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE3=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loc %t.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE4=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loc %t.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE5=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loc %t.o 2>&1 | FileCheck %s
+
+# CONSUME: error: failed to consume entire .debug_loc section
+
+# CHECK: error: location list overflows the debug_loc section
+
+.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
+
+# 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:
Index: lib/DebugInfo/DWARF/DWARFDie.cpp
===================================================================
--- lib/DebugInfo/DWARF/DWARFDie.cpp
+++ lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -97,15 +97,15 @@
DWARFDebugLoc DebugLoc;
DWARFDataExtractor Data(Obj, *U->getLocSection(), Ctx.isLittleEndian(),
Obj.getAddressSize());
- auto LL = DebugLoc.parseOneLocationList(Data, &Offset);
- if (LL) {
+ if (auto LL = DebugLoc.parseOneLocationList(Data, &Offset)) {
uint64_t BaseAddr = 0;
if (Optional<object::SectionedAddress> BA = U->getBaseAddress())
BaseAddr = BA->Address;
LL->dump(OS, Ctx.isLittleEndian(), Obj.getAddressSize(), MRI, U,
BaseAddr, Indent);
} else
- OS << "error extracting location list.";
+ logAllUnhandledErrors(LL.takeError(), OS,
+ "error extracting location list:");
return;
}
@@ -131,8 +131,10 @@
if (LL)
LL->dump(OS, BaseAddr, Ctx.isLittleEndian(), Obj.getAddressSize(), MRI,
U, Indent);
- else
- OS << "error extracting location list.";
+ else {
+ logAllUnhandledErrors(LL.takeError(), OS,
+ "error extracting location list:");
+ }
}
}
}
Index: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
===================================================================
--- lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
+++ lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
@@ -23,6 +23,11 @@
using namespace llvm;
+template <typename... Ts>
+static llvm::Error createError(const char *Fmt, Ts &&... Vals) {
+ return createStringError(inconvertibleErrorCode(), Fmt, Vals...);
+}
+
// When directly dumping the .debug_loc without a compile unit, we have to guess
// at the DWARF version. This only affects DW_OP_call_ref, which is a rare
// expression that LLVM doesn't produce. Guessing the wrong version means we
@@ -83,7 +88,7 @@
}
}
-Optional<DWARFDebugLoc::LocationList>
+Expected<DWARFDebugLoc::LocationList>
DWARFDebugLoc::parseOneLocationList(DWARFDataExtractor Data, unsigned *Offset) {
LocationList LL;
LL.Offset = *Offset;
@@ -92,10 +97,8 @@
// 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;
- }
+ if (!Data.isValidOffsetForDataOfSize(*Offset, 2 * Data.getAddressSize()))
+ return createError("location list overflows the debug_loc section");
// 1. A beginning address offset. ...
E.Begin = Data.getRelocatedAddress(Offset);
@@ -109,16 +112,13 @@
if (E.Begin == 0 && E.End == 0)
return LL;
- if (!Data.isValidOffsetForDataOfSize(*Offset, 2)) {
- WithColor::error() << "location list overflows the debug_loc section.\n";
- return None;
- }
+ if (!Data.isValidOffsetForDataOfSize(*Offset, 2))
+ return createError("location list overflows the debug_loc section");
unsigned Bytes = Data.getU16(Offset);
- if (!Data.isValidOffsetForDataOfSize(*Offset, Bytes)) {
- WithColor::error() << "location list overflows the debug_loc section.\n";
- return None;
- }
+ if (!Data.isValidOffsetForDataOfSize(*Offset, Bytes))
+ return createError("location list overflows the debug_loc section");
+
// A single location description describing the location of the object...
StringRef str = Data.getData().substr(*Offset, Bytes);
*Offset += Bytes;
@@ -136,28 +136,49 @@
while (data.isValidOffset(Offset + data.getAddressSize() - 1)) {
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::LocationList>
+Expected<DWARFDebugLoclists::LocationList>
DWARFDebugLoclists::parseOneLocationList(DataExtractor Data, unsigned *Offset,
unsigned Version) {
LocationList LL;
LL.Offset = *Offset;
- // dwarf::DW_LLE_end_of_list_entry is 0 and indicates the end of the list.
- while (auto Kind =
- static_cast<dwarf::LocationListEntry>(Data.getU8(Offset))) {
+ while (true) {
+ if (!Data.isValidOffset(*Offset))
+ return createError("location list overflows the debug_loclists section");
Entry E;
- E.Kind = Kind;
- switch (Kind) {
+ E.Kind = static_cast<dwarf::LocationListEntry>(Data.getU8(Offset));
+ if (E.Kind == dwarf::DW_LLE_end_of_list)
+ return LL;
+
+ unsigned SavedOffset = *Offset;
+ switch (E.Kind) {
case dwarf::DW_LLE_startx_length:
+ case dwarf::DW_LLE_offset_pair:
E.Value0 = Data.getULEB128(Offset);
+ break;
+ case dwarf::DW_LLE_start_length:
+ case dwarf::DW_LLE_base_address:
+ E.Value0 = Data.getAddress(Offset);
+ break;
+ default:
+ return createError("LLE of kind %x not implemented", (int)E.Kind);
+ }
+ if (SavedOffset == *Offset)
+ return createError("location list overflows the debug_loclists section");
+
+ SavedOffset = *Offset;
+ switch (E.Kind) {
+ case dwarf::DW_LLE_startx_length:
// 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)
@@ -166,25 +187,25 @@
E.Value1 = Data.getULEB128(Offset);
break;
case dwarf::DW_LLE_start_length:
- E.Value0 = Data.getAddress(Offset);
- E.Value1 = Data.getULEB128(Offset);
- break;
case dwarf::DW_LLE_offset_pair:
- E.Value0 = Data.getULEB128(Offset);
E.Value1 = Data.getULEB128(Offset);
break;
case dwarf::DW_LLE_base_address:
- E.Value0 = Data.getAddress(Offset);
break;
- default:
- WithColor::error() << "dumping support for LLE of kind " << (int)Kind
- << " not implemented\n";
- return None;
}
+ if (E.Kind != dwarf::DW_LLE_base_address) {
+ if (SavedOffset == *Offset)
+ return createError(
+ "location list overflows the debug_loclists section");
- if (Kind != dwarf::DW_LLE_base_address) {
+ SavedOffset = *Offset;
unsigned Bytes =
Version >= 5 ? Data.getULEB128(Offset) : Data.getU16(Offset);
+ if (SavedOffset == *Offset ||
+ !Data.isValidOffsetForDataOfSize(*Offset, Bytes))
+ return createError(
+ "location list overflows the debug_loclists section");
+
// A single location description describing the location of the object...
StringRef str = Data.getData().substr(*Offset, Bytes);
*Offset += Bytes;
@@ -194,7 +215,6 @@
LL.Entries.push_back(std::move(E));
}
- return LL;
}
void DWARFDebugLoclists::parse(DataExtractor data, unsigned Version) {
@@ -205,8 +225,10 @@
while (data.isValidOffset(Offset)) {
if (auto LL = parseOneLocationList(data, &Offset, Version))
Locations.push_back(std::move(*LL));
- else
+ else {
+ logAllUnhandledErrors(LL.takeError(), WithColor::error());
return;
+ }
}
}
Index: include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
===================================================================
--- include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
+++ include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
@@ -68,8 +68,8 @@
/// Return the location list at the given offset or nullptr.
LocationList const *getLocationListAtOffset(uint64_t Offset) const;
- Optional<LocationList> parseOneLocationList(DWARFDataExtractor Data,
- uint32_t *Offset);
+ static Expected<LocationList> parseOneLocationList(DWARFDataExtractor Data,
+ uint32_t *Offset);
};
class DWARFDebugLoclists {
@@ -106,7 +106,7 @@
/// Return the location list at the given offset or nullptr.
LocationList const *getLocationListAtOffset(uint64_t Offset) const;
- static Optional<LocationList>
+ static Expected<LocationList>
parseOneLocationList(DataExtractor Data, unsigned *Offset, unsigned Version);
};
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
