[Support] Make DataExtractor error messages more clear

Summary:
This is a result of the discussion at D78113. Previously we would be
only giving the current offset at which the error was detected. However,
this was phrased somewhat ambiguously (as it could also mean that end of
data was at that offset). The new error message includes the current
offset as well as the extent of the data being read.

I've changed a couple of file-level static functions into private member
functions in order to avoid passing a bunch of new arguments everywhere.

Reviewers: dblaikie, jhenderson

Subscribers: hiraditya, MaskRay, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D78558
This commit is contained in:
Pavel Labath 2020-04-20 17:28:15 +02:00
parent ffff13ee8c
commit bbfbcfbfb2
13 changed files with 102 additions and 71 deletions

View File

@ -689,6 +689,16 @@ protected:
// public.
static uint64_t &getOffset(Cursor &C) { return C.Offset; }
static Error &getError(Cursor &C) { return C.Err; }
private:
/// If it is possible to read \a Size bytes at offset \a Offset, returns \b
/// true. Otherwise, returns \b false. If \a E is not nullptr, also sets the
/// error object to indicate an error.
bool prepareRead(uint64_t Offset, uint64_t Size, Error *E) const;
template <typename T> T getU(uint64_t *OffsetPtr, Error *Err) const;
template <typename T>
T *getUs(uint64_t *OffsetPtr, T *Dst, uint32_t Count, Error *Err) const;
};
} // namespace llvm

View File

@ -15,30 +15,40 @@
using namespace llvm;
static void unexpectedEndReached(Error *E, uint64_t Offset) {
if (E)
*E = createStringError(errc::illegal_byte_sequence,
"unexpected end of data at offset 0x%" PRIx64,
Offset);
bool DataExtractor::prepareRead(uint64_t Offset, uint64_t Size,
Error *E) const {
if (isValidOffsetForDataOfSize(Offset, Size))
return true;
if (E) {
if (Offset <= Data.size())
*E = createStringError(
errc::illegal_byte_sequence,
"unexpected end of data at offset 0x%zx while reading [0x%" PRIx64
", 0x%" PRIx64 ")",
Data.size(), Offset, Offset + Size);
else
*E = createStringError(errc::invalid_argument,
"offset 0x%" PRIx64
" is beyond the end of data at 0x%zx",
Offset, Data.size());
}
return false;
}
static bool isError(Error *E) { return E && *E; }
template <typename T>
static T getU(uint64_t *offset_ptr, const DataExtractor *de,
bool isLittleEndian, const char *Data, llvm::Error *Err) {
T DataExtractor::getU(uint64_t *offset_ptr, Error *Err) const {
ErrorAsOutParameter ErrAsOut(Err);
T val = 0;
if (isError(Err))
return val;
uint64_t offset = *offset_ptr;
if (!de->isValidOffsetForDataOfSize(offset, sizeof(T))) {
unexpectedEndReached(Err, offset);
if (!prepareRead(offset, sizeof(T), Err))
return val;
}
std::memcpy(&val, &Data[offset], sizeof(val));
if (sys::IsLittleEndianHost != isLittleEndian)
std::memcpy(&val, &Data.data()[offset], sizeof(val));
if (sys::IsLittleEndianHost != IsLittleEndian)
sys::swapByteOrder(val);
// Advance the offset
@ -47,22 +57,19 @@ static T getU(uint64_t *offset_ptr, const DataExtractor *de,
}
template <typename T>
static T *getUs(uint64_t *offset_ptr, T *dst, uint32_t count,
const DataExtractor *de, bool isLittleEndian, const char *Data,
llvm::Error *Err) {
T *DataExtractor::getUs(uint64_t *offset_ptr, T *dst, uint32_t count,
Error *Err) const {
ErrorAsOutParameter ErrAsOut(Err);
if (isError(Err))
return nullptr;
uint64_t offset = *offset_ptr;
if (!de->isValidOffsetForDataOfSize(offset, sizeof(*dst) * count)) {
unexpectedEndReached(Err, offset);
if (!prepareRead(offset, sizeof(*dst) * count, Err))
return nullptr;
}
for (T *value_ptr = dst, *end = dst + count; value_ptr != end;
++value_ptr, offset += sizeof(*dst))
*value_ptr = getU<T>(offset_ptr, de, isLittleEndian, Data, Err);
*value_ptr = getU<T>(offset_ptr, Err);
// Advance the offset
*offset_ptr = offset;
// Return a non-NULL pointer to the converted data as an indicator of
@ -71,55 +78,49 @@ static T *getUs(uint64_t *offset_ptr, T *dst, uint32_t count,
}
uint8_t DataExtractor::getU8(uint64_t *offset_ptr, llvm::Error *Err) const {
return getU<uint8_t>(offset_ptr, this, IsLittleEndian, Data.data(), Err);
return getU<uint8_t>(offset_ptr, Err);
}
uint8_t *
DataExtractor::getU8(uint64_t *offset_ptr, uint8_t *dst, uint32_t count) const {
return getUs<uint8_t>(offset_ptr, dst, count, this, IsLittleEndian,
Data.data(), nullptr);
uint8_t *DataExtractor::getU8(uint64_t *offset_ptr, uint8_t *dst,
uint32_t count) const {
return getUs<uint8_t>(offset_ptr, dst, count, nullptr);
}
uint8_t *DataExtractor::getU8(Cursor &C, uint8_t *Dst, uint32_t Count) const {
return getUs<uint8_t>(&C.Offset, Dst, Count, this, IsLittleEndian,
Data.data(), &C.Err);
return getUs<uint8_t>(&C.Offset, Dst, Count, &C.Err);
}
uint16_t DataExtractor::getU16(uint64_t *offset_ptr, llvm::Error *Err) const {
return getU<uint16_t>(offset_ptr, this, IsLittleEndian, Data.data(), Err);
return getU<uint16_t>(offset_ptr, Err);
}
uint16_t *DataExtractor::getU16(uint64_t *offset_ptr, uint16_t *dst,
uint32_t count) const {
return getUs<uint16_t>(offset_ptr, dst, count, this, IsLittleEndian,
Data.data(), nullptr);
return getUs<uint16_t>(offset_ptr, dst, count, nullptr);
}
uint32_t DataExtractor::getU24(uint64_t *OffsetPtr, Error *Err) const {
uint24_t ExtractedVal =
getU<uint24_t>(OffsetPtr, this, IsLittleEndian, Data.data(), Err);
uint24_t ExtractedVal = getU<uint24_t>(OffsetPtr, Err);
// 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, llvm::Error *Err) const {
return getU<uint32_t>(offset_ptr, this, IsLittleEndian, Data.data(), Err);
return getU<uint32_t>(offset_ptr, Err);
}
uint32_t *DataExtractor::getU32(uint64_t *offset_ptr, uint32_t *dst,
uint32_t count) const {
return getUs<uint32_t>(offset_ptr, dst, count, this, IsLittleEndian,
Data.data(), nullptr);
return getUs<uint32_t>(offset_ptr, dst, count, nullptr);
}
uint64_t DataExtractor::getU64(uint64_t *offset_ptr, llvm::Error *Err) const {
return getU<uint64_t>(offset_ptr, this, IsLittleEndian, Data.data(), Err);
return getU<uint64_t>(offset_ptr, Err);
}
uint64_t *DataExtractor::getU64(uint64_t *offset_ptr, uint64_t *dst,
uint32_t count) const {
return getUs<uint64_t>(offset_ptr, dst, count, this, IsLittleEndian,
Data.data(), nullptr);
return getUs<uint64_t>(offset_ptr, dst, count, nullptr);
}
uint64_t DataExtractor::getUnsigned(uint64_t *offset_ptr, uint32_t byte_size,
@ -163,7 +164,10 @@ StringRef DataExtractor::getCStrRef(uint64_t *OffsetPtr, Error *Err) const {
*OffsetPtr = Pos + 1;
return StringRef(Data.data() + Start, Pos - Start);
}
unexpectedEndReached(Err, Start);
if (Err)
*Err = createStringError(errc::illegal_byte_sequence,
"no null terminated string at offset 0x%" PRIx64,
Start);
return StringRef();
}
@ -180,10 +184,8 @@ StringRef DataExtractor::getBytes(uint64_t *OffsetPtr, uint64_t Length,
if (isError(Err))
return StringRef();
if (!isValidOffsetForDataOfSize(*OffsetPtr, Length)) {
unexpectedEndReached(Err, *OffsetPtr);
if (!prepareRead(*OffsetPtr, Length, Err))
return StringRef();
}
StringRef Result = Data.substr(*OffsetPtr, Length);
*OffsetPtr += Length;
@ -229,8 +231,6 @@ void DataExtractor::skip(Cursor &C, uint64_t Length) const {
if (isError(&C.Err))
return;
if (isValidOffsetForDataOfSize(C.Offset, Length))
if (prepareRead(C.Offset, Length, &C.Err))
C.Offset += Length;
else
unexpectedEndReached(&C.Err, C.Offset);
}

View File

@ -9,11 +9,11 @@
# CHECK: DW_AT_name ("x1")
# CHECK-NEXT: DW_AT_location (0xdeadbeef: )
# ERR: error: unexpected end of data at offset 0xdeadbeef
# ERR: error: offset 0xdeadbeef is beyond the end of data at 0x48
# CHECK: DW_AT_name ("x2")
# CHECK-NEXT: DW_AT_location (0x00000036: )
# ERR: error: unexpected end of data at offset 0x48
# ERR: error: unexpected end of data at offset 0x48 while reading [0x48, 0xdef5)
.type f,@function

View File

@ -9,11 +9,11 @@
# CHECK: DW_AT_name ("x1")
# CHECK-NEXT: DW_AT_location (0xdeadbeef: )
# ERR: error: unexpected end of data at offset 0xdeadbeef
# ERR: error: offset 0xdeadbeef is beyond the end of data at 0x34
# CHECK: DW_AT_name ("x2")
# CHECK-NEXT: DW_AT_location (0x00000025
# ERR: error: unexpected end of data at offset 0x34
# ERR: error: unexpected end of data at offset 0x34 while reading [0x34, 0xdeadbf23)
.type f,@function

View File

@ -4,7 +4,7 @@
# CHECK: .debug_addr contents:
# CHECK-NOT: {{.}}
# ERR: parsing address table at offset 0x0: unexpected end of data at offset 0x4
# ERR: parsing address table at offset 0x0: unexpected end of data at offset 0xb while reading [0x4, 0xc)
# ERR-NOT: {{.}}
# too small section to contain an extended length field of a DWARF64 address table.

View File

@ -4,7 +4,7 @@
# CHECK: .debug_addr contents:
# CHECK-NOT: {{.}}
# ERR: parsing address table at offset 0x0: unexpected end of data at offset 0x0
# ERR: parsing address table at offset 0x0: unexpected end of data at offset 0x2 while reading [0x0, 0x4)
# ERR-NOT: {{.}}
# too small section to contain length field

View File

@ -2,7 +2,7 @@
# RUN: not llvm-dwarfdump --debug-rnglists - 2>&1 | FileCheck %s --check-prefix=SHORT
# SHORT-NOT: error:
# SHORT-NOT: range list header
# SHORT: error: parsing .debug_rnglists table at offset 0x0: unexpected end of data at offset 0x0
# SHORT: error: parsing .debug_rnglists table at offset 0x0: unexpected end of data at offset 0x3
# SHORT-NOT: range list header
# SHORT-NOT: error:

View File

@ -43,7 +43,7 @@ TEST(DWARFDebugNames, TooSmallForDWARF64) {
ExtractDebugNames(StringRef(NamesSecData, sizeof(NamesSecData)),
StringRef()),
FailedWithMessage("parsing .debug_names header at 0x0: unexpected end of "
"data at offset 0x28"));
"data at offset 0x2b while reading [0x28, 0x2c)"));
}
} // end anonymous namespace

View File

@ -61,8 +61,10 @@ Symbols:
DataExtractor::Cursor C(0);
EXPECT_EQ(0x42u, Data.getRelocatedAddress(C));
EXPECT_EQ(0u, Data.getRelocatedAddress(C));
EXPECT_THAT_ERROR(C.takeError(),
FailedWithMessage("unexpected end of data at offset 0x4"));
EXPECT_THAT_ERROR(
C.takeError(),
FailedWithMessage(
"unexpected end of data at offset 0x6 while reading [0x4, 0x8)"));
}
TEST(DWARFDataExtractorTest, getInitialLength) {
@ -94,13 +96,15 @@ TEST(DWARFDataExtractorTest, getInitialLength) {
// Empty data.
EXPECT_THAT_EXPECTED(
GetWithError({}),
FailedWithMessage("unexpected end of data at offset 0x0"));
FailedWithMessage(
"unexpected end of data at offset 0x0 while reading [0x0, 0x4)"));
EXPECT_EQ(GetWithoutError({}), ErrorResult);
// Not long enough for the U32 field.
EXPECT_THAT_EXPECTED(
GetWithError({0x00, 0x01, 0x02}),
FailedWithMessage("unexpected end of data at offset 0x0"));
FailedWithMessage(
"unexpected end of data at offset 0x3 while reading [0x0, 0x4)"));
EXPECT_EQ(GetWithoutError({0x00, 0x01, 0x02}), ErrorResult);
EXPECT_THAT_EXPECTED(
@ -127,13 +131,15 @@ TEST(DWARFDataExtractorTest, getInitialLength) {
// DWARF64 marker without the subsequent length field.
EXPECT_THAT_EXPECTED(
GetWithError({0xff, 0xff, 0xff, 0xff}),
FailedWithMessage("unexpected end of data at offset 0x4"));
FailedWithMessage(
"unexpected end of data at offset 0x4 while reading [0x4, 0xc)"));
EXPECT_EQ(GetWithoutError({0xff, 0xff, 0xff, 0xff}), ErrorResult);
// Not enough data for the U64 length.
EXPECT_THAT_EXPECTED(
GetWithError({0xff, 0xff, 0xff, 0xff, 0x00, 0x01, 0x02, 0x03}),
FailedWithMessage("unexpected end of data at offset 0x4"));
FailedWithMessage(
"unexpected end of data at offset 0x8 while reading [0x4, 0xc)"));
EXPECT_EQ(GetWithoutError({0xff, 0xff, 0xff, 0xff, 0x00, 0x01, 0x02, 0x03}),
ErrorResult);
@ -195,21 +201,27 @@ Symbols:
EXPECT_EQ(0x64636261u, Truncated8.getRelocatedAddress(C));
EXPECT_EQ(0x42u, Truncated8.getRelocatedAddress(C));
EXPECT_EQ(0x0u, Truncated8.getRelocatedAddress(C));
EXPECT_THAT_ERROR(C.takeError(),
FailedWithMessage("unexpected end of data at offset 0x8"));
EXPECT_THAT_ERROR(
C.takeError(),
FailedWithMessage(
"unexpected end of data at offset 0x8 while reading [0x8, 0xc)"));
C = DataExtractor::Cursor{0};
DWARFDataExtractor Truncated6(Data, 6);
EXPECT_EQ(0x64636261u, Truncated6.getRelocatedAddress(C));
EXPECT_EQ(0x0u, Truncated6.getRelocatedAddress(C));
EXPECT_THAT_ERROR(C.takeError(),
FailedWithMessage("unexpected end of data at offset 0x4"));
EXPECT_THAT_ERROR(
C.takeError(),
FailedWithMessage(
"unexpected end of data at offset 0x6 while reading [0x4, 0x8)"));
C = DataExtractor::Cursor{0};
DWARFDataExtractor Truncated2(Data, 2);
EXPECT_EQ(0x0u, Truncated2.getRelocatedAddress(C));
EXPECT_THAT_ERROR(C.takeError(),
FailedWithMessage("unexpected end of data at offset 0x0"));
EXPECT_THAT_ERROR(
C.takeError(),
FailedWithMessage(
"unexpected end of data at offset 0x2 while reading [0x0, 0x4)"));
}
} // namespace

View File

@ -121,7 +121,7 @@ TEST(DWARFDebugArangeSet, SectionTooShort) {
static const char DebugArangesSecRaw[11 + 1] = {0};
ExpectExtractError(DebugArangesSecRaw,
"parsing address ranges table at offset 0x0: unexpected "
"end of data at offset 0xb");
"end of data at offset 0xb while reading [0xb, 0xc)");
}
TEST(DWARFDebugArangeSet, SectionTooShortDWARF64) {
@ -130,7 +130,7 @@ TEST(DWARFDebugArangeSet, SectionTooShortDWARF64) {
"\xff\xff\xff\xff"; // DWARF64 mark
ExpectExtractError(DebugArangesSecRaw,
"parsing address ranges table at offset 0x0: unexpected "
"end of data at offset 0x17");
"end of data at offset 0x17 while reading [0x17, 0x18)");
}
TEST(DWARFDebugArangeSet, NoSpaceForEntries) {

View File

@ -211,8 +211,9 @@ TEST_F(DebugLineBasicFixture, GetOrParseLineTableAtInvalidOffsetAfterData) {
EXPECT_THAT_EXPECTED(
getOrParseLineTableFatalErrors(0),
FailedWithMessage("parsing line table prologue at offset 0x00000000: "
"unexpected end of data at offset 0x0"));
FailedWithMessage(
"parsing line table prologue at offset 0x00000000: "
"unexpected end of data at offset 0x1 while reading [0x0, 0x4)"));
EXPECT_THAT_EXPECTED(
getOrParseLineTableFatalErrors(1),

View File

@ -107,7 +107,8 @@ TEST(DWARFDie, getLocations) {
EXPECT_THAT_EXPECTED(
Die.getLocations(DW_AT_call_data_location),
FailedWithMessage("unexpected end of data at offset 0x20"));
FailedWithMessage(
"unexpected end of data at offset 0x20 while reading [0x20, 0x21)"));
EXPECT_THAT_EXPECTED(
Die.getLocations(DW_AT_call_data_value),

View File

@ -102,8 +102,9 @@ TEST(DataExtractorTest, Strings) {
EXPECT_EQ(11U, C.tell());
EXPECT_EQ(nullptr, DE.getCStr(C));
EXPECT_EQ(11U, C.tell());
EXPECT_THAT_ERROR(C.takeError(),
FailedWithMessage("unexpected end of data at offset 0xb"));
EXPECT_THAT_ERROR(
C.takeError(),
FailedWithMessage("no null terminated string at offset 0xb"));
}
TEST(DataExtractorTest, LEB128) {
@ -270,6 +271,12 @@ TEST(DataExtractorTest, getU8_vector) {
DE.getU8(C, S, 2);
EXPECT_THAT_ERROR(C.takeError(), Succeeded());
EXPECT_EQ("AB", toStringRef(S));
C = DataExtractor::Cursor(0x47);
DE.getU8(C, S, 2);
EXPECT_THAT_ERROR(
C.takeError(),
FailedWithMessage("offset 0x47 is beyond the end of data at 0x2"));
}
TEST(DataExtractorTest, getU24) {