[Lldb-commits] [PATCH] D73969: [LLDB] Let DataExtractor deal with two-byte addresses

[Ayke via Phabricator via lldb-commits]

aykevl created this revision.
aykevl added reviewers: labath, dylanmckay, Andrzej.
aykevl added projects: LLVM, LLDB.
Herald added subscribers: lldb-commits, aprantl.
aykevl updated this revision to Diff 242340.
aykevl added a comment.

Added a unit test.

I think it would also be a good idea to test this by loading an AVR binary and connecting to a debugger (thus triggering this code), but I don't know how to add such a test. Suggestions would be appreciated.


AVR usually uses two byte addresses. By making DataExtractor deal with this, it is possible to load AVR binaries that don't have debug info associated with them.

---

This is a more focused version of D73961 <https://reviews.llvm.org/D73961>. With this, I can load an avr-gcc compiled binary and connect to a gdb-remote as long as it doesn't include any instructions that cannot yet be disassembled (see D73911 <https://reviews.llvm.org/D73911> and D73958 <https://reviews.llvm.org/D73958>).


https://reviews.llvm.org/D73969

Files:
  lldb/source/Utility/DataExtractor.cpp
  lldb/unittests/Utility/DataExtractorTest.cpp


Index: lldb/unittests/Utility/DataExtractorTest.cpp
===================================================================
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -103,6 +103,20 @@
   EXPECT_EQ(4U, offset);
 }
 
+TEST(DataExtractorTest, UncommonAddressSize) {
+  uint8_t buffer[] = {0x01, 0x02, 0x03, 0x04};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 2);
+
+  lldb::offset_t offset;
+
+  offset = 0;
+  EXPECT_EQ(0x01U, E.GetMaxU64(&offset, 1));
+  EXPECT_EQ(1U, offset);
+  offset = 0;
+  EXPECT_EQ(0x0201U, E.GetMaxU64(&offset, 2));
+  EXPECT_EQ(2U, offset);
+}
+
 TEST(DataExtractorTest, GetMaxU64) {
   uint8_t buffer[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08};
   DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
Index: lldb/source/Utility/DataExtractor.cpp
===================================================================
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -133,7 +133,7 @@
       m_end(const_cast<uint8_t *>(static_cast<const uint8_t *>(data)) + length),
       m_byte_order(endian), m_addr_size(addr_size), m_data_sp(),
       m_target_byte_size(target_byte_size) {
-  assert(addr_size == 4 || addr_size == 8);
+  assert(addr_size >= 1 && addr_size <= 8);
 }
 
 // Make a shared pointer reference to the shared data in "data_sp" and set the
@@ -146,7 +146,7 @@
     : m_start(nullptr), m_end(nullptr), m_byte_order(endian),
       m_addr_size(addr_size), m_data_sp(),
       m_target_byte_size(target_byte_size) {
-  assert(addr_size == 4 || addr_size == 8);
+  assert(addr_size >= 1 && addr_size <= 8);
   SetData(data_sp);
 }
 
@@ -160,7 +160,7 @@
     : m_start(nullptr), m_end(nullptr), m_byte_order(data.m_byte_order),
       m_addr_size(data.m_addr_size), m_data_sp(),
       m_target_byte_size(target_byte_size) {
-  assert(m_addr_size == 4 || m_addr_size == 8);
+  assert(m_addr_size >= 1 && m_addr_size <= 8);
   if (data.ValidOffset(offset)) {
     offset_t bytes_available = data.GetByteSize() - offset;
     if (length > bytes_available)
@@ -173,7 +173,7 @@
     : m_start(rhs.m_start), m_end(rhs.m_end), m_byte_order(rhs.m_byte_order),
       m_addr_size(rhs.m_addr_size), m_data_sp(rhs.m_data_sp),
       m_target_byte_size(rhs.m_target_byte_size) {
-  assert(m_addr_size == 4 || m_addr_size == 8);
+  assert(m_addr_size >= 1 && m_addr_size <= 8);
 }
 
 // Assignment operator
@@ -251,7 +251,7 @@
                                       offset_t data_offset,
                                       offset_t data_length) {
   m_addr_size = data.m_addr_size;
-  assert(m_addr_size == 4 || m_addr_size == 8);
+  assert(m_addr_size >= 1 && m_addr_size <= 8);
   // If "data" contains shared pointer to data, then we can use that
   if (data.m_data_sp) {
     m_byte_order = data.m_byte_order;
@@ -679,12 +679,12 @@
 //
 // RETURNS the address that was extracted, or zero on failure.
 uint64_t DataExtractor::GetAddress(offset_t *offset_ptr) const {
-  assert(m_addr_size == 4 || m_addr_size == 8);
+  assert(m_addr_size >= 1 && m_addr_size <= 8);
   return GetMaxU64(offset_ptr, m_addr_size);
 }
 
 uint64_t DataExtractor::GetAddress_unchecked(offset_t *offset_ptr) const {
-  assert(m_addr_size == 4 || m_addr_size == 8);
+  assert(m_addr_size >= 1 && m_addr_size <= 8);
   return GetMaxU64_unchecked(offset_ptr, m_addr_size);
 }
 
@@ -695,7 +695,7 @@
 //
 // RETURNS the pointer that was extracted, or zero on failure.
 uint64_t DataExtractor::GetPointer(offset_t *offset_ptr) const {
-  assert(m_addr_size == 4 || m_addr_size == 8);
+  assert(m_addr_size >= 1 && m_addr_size <= 8);
   return GetMaxU64(offset_ptr, m_addr_size);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73969.242340.patch
Type: text/x-patch
Size: 3715 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200204/4fae096f/attachment.bin>