[llvm] b761a64 - [DWARF] Detect extraction errors in DWARFFormValue::extractValue
Pavel Labath via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 9 04:44:01 PDT 2020
Author: Pavel Labath
Date: 2020-04-09T13:41:02+02:00
New Revision: b761a6484d4035785565d41db540bfa7b5fd677d
URL: https://github.com/llvm/llvm-project/commit/b761a6484d4035785565d41db540bfa7b5fd677d
DIFF: https://github.com/llvm/llvm-project/commit/b761a6484d4035785565d41db540bfa7b5fd677d.diff
LOG: [DWARF] Detect extraction errors in DWARFFormValue::extractValue
Summary:
Although the function had a bool return value, it was always returning
true. Presumably this is because the main type of errors one can
encounter here is running off the end of the stream, and until very
recently, the DataExtractor class made it very difficult to detect that.
The situation has changed now, and we can easily detect errors here,
which this patch does.
Reviewers: dblaikie, aprantl
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D77308
Added:
Modified:
llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
llvm/unittests/DebugInfo/DWARF/DWARFFormValueTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
index e97ae81345b8..d93253f47f37 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
@@ -246,6 +246,7 @@ bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,
Value.data = nullptr;
// Read the value for the form into value and follow and DW_FORM_indirect
// instances we run into
+ Error Err = Error::success();
do {
Indirect = false;
switch (Form) {
@@ -253,24 +254,25 @@ bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,
case DW_FORM_ref_addr: {
uint16_t Size =
(Form == DW_FORM_addr) ? FP.AddrSize : FP.getRefAddrByteSize();
- Value.uval = Data.getRelocatedValue(Size, OffsetPtr, &Value.SectionIndex);
+ Value.uval =
+ Data.getRelocatedValue(Size, OffsetPtr, &Value.SectionIndex, &Err);
break;
}
case DW_FORM_exprloc:
case DW_FORM_block:
- Value.uval = Data.getULEB128(OffsetPtr);
+ Value.uval = Data.getULEB128(OffsetPtr, &Err);
IsBlock = true;
break;
case DW_FORM_block1:
- Value.uval = Data.getU8(OffsetPtr);
+ Value.uval = Data.getU8(OffsetPtr, &Err);
IsBlock = true;
break;
case DW_FORM_block2:
- Value.uval = Data.getU16(OffsetPtr);
+ Value.uval = Data.getU16(OffsetPtr, &Err);
IsBlock = true;
break;
case DW_FORM_block4:
- Value.uval = Data.getU32(OffsetPtr);
+ Value.uval = Data.getU32(OffsetPtr, &Err);
IsBlock = true;
break;
case DW_FORM_data1:
@@ -278,28 +280,28 @@ bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,
case DW_FORM_flag:
case DW_FORM_strx1:
case DW_FORM_addrx1:
- Value.uval = Data.getU8(OffsetPtr);
+ Value.uval = Data.getU8(OffsetPtr, &Err);
break;
case DW_FORM_data2:
case DW_FORM_ref2:
case DW_FORM_strx2:
case DW_FORM_addrx2:
- Value.uval = Data.getU16(OffsetPtr);
+ Value.uval = Data.getU16(OffsetPtr, &Err);
break;
case DW_FORM_strx3:
- Value.uval = Data.getU24(OffsetPtr);
+ Value.uval = Data.getU24(OffsetPtr, &Err);
break;
case DW_FORM_data4:
case DW_FORM_ref4:
case DW_FORM_ref_sup4:
case DW_FORM_strx4:
case DW_FORM_addrx4:
- Value.uval = Data.getRelocatedValue(4, OffsetPtr);
+ Value.uval = Data.getRelocatedValue(4, OffsetPtr, nullptr, &Err);
break;
case DW_FORM_data8:
case DW_FORM_ref8:
case DW_FORM_ref_sup8:
- Value.uval = Data.getRelocatedValue(8, OffsetPtr);
+ Value.uval = Data.getRelocatedValue(8, OffsetPtr, nullptr, &Err);
break;
case DW_FORM_data16:
// Treat this like a 16-byte block.
@@ -307,19 +309,23 @@ bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,
IsBlock = true;
break;
case DW_FORM_sdata:
- Value.sval = Data.getSLEB128(OffsetPtr);
+ Value.sval = Data.getSLEB128(OffsetPtr, &Err);
break;
case DW_FORM_udata:
case DW_FORM_ref_udata:
case DW_FORM_rnglistx:
case DW_FORM_loclistx:
- Value.uval = Data.getULEB128(OffsetPtr);
+ case DW_FORM_GNU_addr_index:
+ case DW_FORM_GNU_str_index:
+ case DW_FORM_addrx:
+ case DW_FORM_strx:
+ Value.uval = Data.getULEB128(OffsetPtr, &Err);
break;
case DW_FORM_string:
- Value.cstr = Data.getCStr(OffsetPtr);
+ Value.cstr = Data.getCStr(OffsetPtr, &Err);
break;
case DW_FORM_indirect:
- Form = static_cast<dwarf::Form>(Data.getULEB128(OffsetPtr));
+ Form = static_cast<dwarf::Form>(Data.getULEB128(OffsetPtr, &Err));
Indirect = true;
break;
case DW_FORM_strp:
@@ -328,39 +334,27 @@ bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,
case DW_FORM_GNU_strp_alt:
case DW_FORM_line_strp:
case DW_FORM_strp_sup: {
- Value.uval =
- Data.getRelocatedValue(FP.getDwarfOffsetByteSize(), OffsetPtr);
+ Value.uval = Data.getRelocatedValue(FP.getDwarfOffsetByteSize(),
+ OffsetPtr, nullptr, &Err);
break;
}
case DW_FORM_flag_present:
Value.uval = 1;
break;
case DW_FORM_ref_sig8:
- Value.uval = Data.getU64(OffsetPtr);
- break;
- case DW_FORM_GNU_addr_index:
- case DW_FORM_GNU_str_index:
- case DW_FORM_addrx:
- case DW_FORM_strx:
- Value.uval = Data.getULEB128(OffsetPtr);
+ Value.uval = Data.getU64(OffsetPtr, &Err);
break;
default:
// DWARFFormValue::skipValue() will have caught this and caused all
// DWARF DIEs to fail to be parsed, so this code is not be reachable.
llvm_unreachable("unsupported form");
}
- } while (Indirect);
+ } while (Indirect && !Err);
- if (IsBlock) {
- StringRef Str = Data.getData().substr(*OffsetPtr, Value.uval);
- Value.data = nullptr;
- if (!Str.empty()) {
- Value.data = Str.bytes_begin();
- *OffsetPtr += Value.uval;
- }
- }
+ if (IsBlock)
+ Value.data = Data.getBytes(OffsetPtr, Value.uval, &Err).bytes_begin();
- return true;
+ return !errorToBool(std::move(Err));
}
void DWARFFormValue::dumpSectionedAddress(raw_ostream &OS,
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFFormValueTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFFormValueTest.cpp
index 68b85f7c68cb..378440abccc0 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFFormValueTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFFormValueTest.cpp
@@ -10,6 +10,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/BinaryFormat/Dwarf.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/LEB128.h"
#include "gtest/gtest.h"
@@ -303,4 +304,39 @@ INSTANTIATE_TEST_CASE_P(
ParamType(/*Unknown=*/Form(0xff), 4, 4, DWARF32, SampleU32, 0,
false)), );
+using ErrorParams = std::tuple<Form, std::vector<uint8_t>>;
+struct ExtractValueErrorFixture : public testing::TestWithParam<ErrorParams> {
+ void SetUp() override { std::tie(Fm, InitialData) = GetParam(); }
+
+ Form Fm;
+ ArrayRef<uint8_t> InitialData;
+};
+
+TEST_P(ExtractValueErrorFixture, Test) {
+ SCOPED_TRACE(formatv("Fm = {0}, InitialData = {1}", Fm,
+ make_range(InitialData.begin(), InitialData.end()))
+ .str());
+
+ DWARFDataExtractor Data(InitialData, sys::IsLittleEndianHost, 4);
+ DWARFFormValue Form(Fm);
+ uint64_t Offset = 0;
+ EXPECT_FALSE(Form.extractValue(Data, &Offset, {0, 0, DWARF32}));
+}
+
+INSTANTIATE_TEST_CASE_P(
+ ExtractValueErrorParams, ExtractValueErrorFixture,
+ testing::Values(
+ ErrorParams{DW_FORM_ref_addr, {}}, ErrorParams{DW_FORM_block, {}},
+ ErrorParams{DW_FORM_block, {1}}, ErrorParams{DW_FORM_block, {2, 0}},
+ ErrorParams{DW_FORM_block1, {}}, ErrorParams{DW_FORM_block2, {}},
+ ErrorParams{DW_FORM_block4, {}}, ErrorParams{DW_FORM_data1, {}},
+ ErrorParams{DW_FORM_data2, {}}, ErrorParams{DW_FORM_strx3, {}},
+ ErrorParams{DW_FORM_data4, {}}, ErrorParams{DW_FORM_data8, {}},
+ ErrorParams{DW_FORM_data16, {}}, ErrorParams{DW_FORM_sdata, {}},
+ ErrorParams{DW_FORM_udata, {}}, ErrorParams{DW_FORM_string, {}},
+ ErrorParams{DW_FORM_indirect, {}},
+ ErrorParams{DW_FORM_indirect, {DW_FORM_data1}},
+ ErrorParams{DW_FORM_strp_sup, {}},
+ ErrorParams{DW_FORM_ref_sig8, {}}), );
+
} // end anonymous namespace
More information about the llvm-commits
mailing list