labath created this revision.
labath added a reviewer: clayborg.
labath added a subscriber: lldb-commits.
The function was returning the null pointer for peeks of size zero, which seems
like a sensible
thing to do, but is actually pretty easy to get bitten by that if you are
extracting a variable
length field which happens to be of zero length and then doing pointer
arithmetic on that (which
SymbolFileDWARF does, and ended up crashing in case of empty DW_AT_location).
This changes the function to return a null pointer only when it gets queried
for data which is
outside of the range of the extractor, which is more c++-y, as one can still do
reasonable things
with pointers to data of size zero (think, end() iterators).
I also add a test and fix some signedness warnings in the existing data
extractor tests.
https://reviews.llvm.org/D22755
Files:
include/lldb/Core/DataExtractor.h
unittests/Core/DataExtractorTest.cpp
Index: unittests/Core/DataExtractorTest.cpp
===================================================================
--- unittests/Core/DataExtractorTest.cpp
+++ unittests/Core/DataExtractorTest.cpp
@@ -21,7 +21,7 @@
TEST(DataExtractorTest, GetBitfield)
{
- char buffer[] = { 0x01, 0x23, 0x45, 0x67 };
+ uint8_t buffer[] = { 0x01, 0x23, 0x45, 0x67 };
DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
sizeof(void *));
DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void
*));
@@ -33,7 +33,24 @@
ASSERT_EQ(buffer[1], BE.GetMaxU64Bitfield(&offset, sizeof(buffer), 8, 8));
offset = 0;
- ASSERT_EQ(buffer[1], LE.GetMaxS64Bitfield(&offset, sizeof(buffer), 8, 8));
+ ASSERT_EQ(int8_t(buffer[1]), LE.GetMaxS64Bitfield(&offset, sizeof(buffer),
8, 8));
offset = 0;
- ASSERT_EQ(buffer[1], BE.GetMaxS64Bitfield(&offset, sizeof(buffer), 8, 8));
+ ASSERT_EQ(int8_t(buffer[1]), BE.GetMaxS64Bitfield(&offset, sizeof(buffer),
8, 8));
+}
+
+TEST(DataExtractorTest, PeekData)
+{
+ uint8_t buffer[] = { 0x01, 0x02, 0x03, 0x04 };
+ DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+ EXPECT_EQ(buffer + 0, E.PeekData(0, 0));
+ EXPECT_EQ(buffer + 0, E.PeekData(0, 4));
+ EXPECT_EQ(nullptr, E.PeekData(0, 5));
+
+ EXPECT_EQ(buffer + 2, E.PeekData(2, 0));
+ EXPECT_EQ(buffer + 2, E.PeekData(2, 2));
+ EXPECT_EQ(nullptr, E.PeekData(2, 3));
+
+ EXPECT_EQ(buffer + 4, E.PeekData(4, 0));
+ EXPECT_EQ(nullptr, E.PeekData(4, 1));
}
Index: include/lldb/Core/DataExtractor.h
===================================================================
--- include/lldb/Core/DataExtractor.h
+++ include/lldb/Core/DataExtractor.h
@@ -1139,7 +1139,7 @@
const uint8_t*
PeekData (lldb::offset_t offset, lldb::offset_t length) const
{
- if (length > 0 && ValidOffsetForDataOfSize(offset, length))
+ if (ValidOffsetForDataOfSize(offset, length))
return m_start + offset;
return nullptr;
}
Index: unittests/Core/DataExtractorTest.cpp
===================================================================
--- unittests/Core/DataExtractorTest.cpp
+++ unittests/Core/DataExtractorTest.cpp
@@ -21,7 +21,7 @@
TEST(DataExtractorTest, GetBitfield)
{
- char buffer[] = { 0x01, 0x23, 0x45, 0x67 };
+ uint8_t buffer[] = { 0x01, 0x23, 0x45, 0x67 };
DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle, sizeof(void *));
DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
@@ -33,7 +33,24 @@
ASSERT_EQ(buffer[1], BE.GetMaxU64Bitfield(&offset, sizeof(buffer), 8, 8));
offset = 0;
- ASSERT_EQ(buffer[1], LE.GetMaxS64Bitfield(&offset, sizeof(buffer), 8, 8));
+ ASSERT_EQ(int8_t(buffer[1]), LE.GetMaxS64Bitfield(&offset, sizeof(buffer), 8, 8));
offset = 0;
- ASSERT_EQ(buffer[1], BE.GetMaxS64Bitfield(&offset, sizeof(buffer), 8, 8));
+ ASSERT_EQ(int8_t(buffer[1]), BE.GetMaxS64Bitfield(&offset, sizeof(buffer), 8, 8));
+}
+
+TEST(DataExtractorTest, PeekData)
+{
+ uint8_t buffer[] = { 0x01, 0x02, 0x03, 0x04 };
+ DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+ EXPECT_EQ(buffer + 0, E.PeekData(0, 0));
+ EXPECT_EQ(buffer + 0, E.PeekData(0, 4));
+ EXPECT_EQ(nullptr, E.PeekData(0, 5));
+
+ EXPECT_EQ(buffer + 2, E.PeekData(2, 0));
+ EXPECT_EQ(buffer + 2, E.PeekData(2, 2));
+ EXPECT_EQ(nullptr, E.PeekData(2, 3));
+
+ EXPECT_EQ(buffer + 4, E.PeekData(4, 0));
+ EXPECT_EQ(nullptr, E.PeekData(4, 1));
}
Index: include/lldb/Core/DataExtractor.h
===================================================================
--- include/lldb/Core/DataExtractor.h
+++ include/lldb/Core/DataExtractor.h
@@ -1139,7 +1139,7 @@
const uint8_t*
PeekData (lldb::offset_t offset, lldb::offset_t length) const
{
- if (length > 0 && ValidOffsetForDataOfSize(offset, length))
+ if (ValidOffsetForDataOfSize(offset, length))
return m_start + offset;
return nullptr;
}
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
