[Lldb-commits] [lldb] r315444 - Fix dumping of characters with non-standard sizes

Petr Pavlu via lldb-commits lldb-commits at lists.llvm.org
Wed Oct 11 01:48:18 PDT 2017


Author: petr.pavlu
Date: Wed Oct 11 01:48:18 2017
New Revision: 315444

URL: http://llvm.org/viewvc/llvm-project?rev=315444&view=rev
Log:
Fix dumping of characters with non-standard sizes

* Prevent dumping of characters in DumpDataExtractor() with
  item_byte_size bigger than 8 bytes. This case is not supported by the
  code and results in a crash because the code calls
  DataExtractor::GetMaxU64Bitfield() -> GetMaxU64() that asserts for
  byte size > 8 bytes.
* Teach DataExtractor::GetMaxU64(), GetMaxU32(), GetMaxS64() and
  GetMaxU64_unchecked() how to handle byte sizes that are not a multiple
  of 2. This allows DumpDataExtractor() to dump characters and booleans
  with item_byte_size in the interval of [1, 8] bytes. Values that are
  not a multiple of 2 would previously result in a crash because they
  were not handled by GetMaxU64().

Modified:
    lldb/trunk/include/lldb/Utility/DataExtractor.h
    lldb/trunk/source/Core/DumpDataExtractor.cpp
    lldb/trunk/source/Utility/DataExtractor.cpp
    lldb/trunk/unittests/Core/DataExtractorTest.cpp

Modified: lldb/trunk/include/lldb/Utility/DataExtractor.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/DataExtractor.h?rev=315444&r1=315443&r2=315444&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Utility/DataExtractor.h (original)
+++ lldb/trunk/include/lldb/Utility/DataExtractor.h Wed Oct 11 01:48:18 2017
@@ -513,10 +513,8 @@ public:
   ///
   /// Extract a single integer value and update the offset pointed to
   /// by \a offset_ptr. The size of the extracted integer is specified
-  /// by the \a byte_size argument. \a byte_size should have a value
-  /// >= 1 and <= 4 since the return value is only 32 bits wide. Any
-  /// \a byte_size values less than 1 or greater than 4 will result in
-  /// nothing being extracted, and zero being returned.
+  /// by the \a byte_size argument. \a byte_size must have a value
+  /// >= 1 and <= 4 since the return value is only 32 bits wide.
   ///
   /// @param[in,out] offset_ptr
   ///     A pointer to an offset within the data that will be advanced
@@ -539,11 +537,9 @@ public:
   ///
   /// Extract a single unsigned integer value and update the offset
   /// pointed to by \a offset_ptr. The size of the extracted integer
-  /// is specified by the \a byte_size argument. \a byte_size should
+  /// is specified by the \a byte_size argument. \a byte_size must
   /// have a value greater than or equal to one and less than or equal
-  /// to eight since the return value is 64 bits wide. Any
-  /// \a byte_size values less than 1 or greater than 8 will result in
-  /// nothing being extracted, and zero being returned.
+  /// to eight since the return value is 64 bits wide.
   ///
   /// @param[in,out] offset_ptr
   ///     A pointer to an offset within the data that will be advanced
@@ -570,10 +566,9 @@ public:
   /// Extract a single signed integer value (sign extending if required)
   /// and update the offset pointed to by \a offset_ptr. The size of
   /// the extracted integer is specified by the \a byte_size argument.
-  /// \a byte_size should have a value greater than or equal to one
-  /// and less than or equal to eight since the return value is 64
-  /// bits wide. Any \a byte_size values less than 1 or greater than
-  /// 8 will result in nothing being extracted, and zero being returned.
+  /// \a byte_size must have a value greater than or equal to one and
+  /// less than or equal to eight since the return value is 64 bits
+  /// wide.
   ///
   /// @param[in,out] offset_ptr
   ///     A pointer to an offset within the data that will be advanced
@@ -589,7 +584,7 @@ public:
   ///     The sign extended signed integer value that was extracted,
   ///     or zero on failure.
   //------------------------------------------------------------------
-  int64_t GetMaxS64(lldb::offset_t *offset_ptr, size_t size) const;
+  int64_t GetMaxS64(lldb::offset_t *offset_ptr, size_t byte_size) const;
 
   //------------------------------------------------------------------
   /// Extract an unsigned integer of size \a byte_size from \a
@@ -598,11 +593,9 @@ public:
   ///
   /// Extract a single unsigned integer value and update the offset
   /// pointed to by \a offset_ptr. The size of the extracted integer
-  /// is specified by the \a byte_size argument. \a byte_size should
+  /// is specified by the \a byte_size argument. \a byte_size must
   /// have a value greater than or equal to one and less than or equal
-  /// to 8 since the return value is 64 bits wide. Any
-  /// \a byte_size values less than 1 or greater than 8 will result in
-  /// nothing being extracted, and zero being returned.
+  /// to 8 since the return value is 64 bits wide.
   ///
   /// @param[in,out] offset_ptr
   ///     A pointer to an offset within the data that will be advanced
@@ -641,10 +634,9 @@ public:
   /// Extract a single signed integer value (sign extending if required)
   /// and update the offset pointed to by \a offset_ptr. The size of
   /// the extracted integer is specified by the \a byte_size argument.
-  /// \a byte_size should have a value greater than or equal to one
-  /// and less than or equal to eight since the return value is 64
-  /// bits wide. Any \a byte_size values less than 1 or greater than
-  /// 8 will result in nothing being extracted, and zero being returned.
+  /// \a byte_size must have a value greater than or equal to one and
+  /// less than or equal to eight since the return value is 64 bits
+  /// wide.
   ///
   /// @param[in,out] offset_ptr
   ///     A pointer to an offset within the data that will be advanced

Modified: lldb/trunk/source/Core/DumpDataExtractor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/DumpDataExtractor.cpp?rev=315444&r1=315443&r2=315444&view=diff
==============================================================================
--- lldb/trunk/source/Core/DumpDataExtractor.cpp (original)
+++ lldb/trunk/source/Core/DumpDataExtractor.cpp Wed Oct 11 01:48:18 2017
@@ -272,6 +272,13 @@ lldb::offset_t lldb_private::DumpDataExt
     case eFormatChar:
     case eFormatCharPrintable:
     case eFormatCharArray: {
+      // Reject invalid item_byte_size.
+      if (item_byte_size > 8) {
+        s->Printf("error: unsupported byte size (%" PRIu64 ") for char format",
+                  (uint64_t)item_byte_size);
+        return offset;
+      }
+
       // If we are only printing one character surround it with single
       // quotes
       if (item_count == 1 && item_format == eFormatChar)

Modified: lldb/trunk/source/Utility/DataExtractor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/DataExtractor.cpp?rev=315444&r1=315443&r2=315444&view=diff
==============================================================================
--- lldb/trunk/source/Utility/DataExtractor.cpp (original)
+++ lldb/trunk/source/Utility/DataExtractor.cpp Wed Oct 11 01:48:18 2017
@@ -17,6 +17,7 @@
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
+#include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
@@ -105,6 +106,20 @@ static inline uint64_t ReadSwapInt64(con
   return llvm::ByteSwap_64(value);
 }
 
+static inline uint64_t ReadMaxInt64(const uint8_t *data, size_t byte_size,
+                                    ByteOrder byte_order) {
+  uint64_t res = 0;
+  if (byte_order == eByteOrderBig)
+    for (size_t i = 0; i < byte_size; ++i)
+      res = (res << 8) | data[i];
+  else {
+    assert(byte_order == eByteOrderLittle);
+    for (size_t i = 0; i < byte_size; ++i)
+      res = (res << 8) | data[byte_size - 1 - i];
+  }
+  return res;
+}
+
 DataExtractor::DataExtractor()
     : m_start(nullptr), m_end(nullptr),
       m_byte_order(endian::InlHostByteOrder()), m_addr_size(sizeof(void *)),
@@ -551,107 +566,59 @@ void *DataExtractor::GetU64(offset_t *of
   return nullptr;
 }
 
-//----------------------------------------------------------------------
-// Extract a single integer value from the data and update the offset
-// pointed to by "offset_ptr". The size of the extracted integer
-// is specified by the "byte_size" argument. "byte_size" should have
-// a value between 1 and 4 since the return value is only 32 bits
-// wide. Any "byte_size" values less than 1 or greater than 4 will
-// result in nothing being extracted, and zero being returned.
-//
-// RETURNS the integer value that was extracted, or zero on failure.
-//----------------------------------------------------------------------
 uint32_t DataExtractor::GetMaxU32(offset_t *offset_ptr,
                                   size_t byte_size) const {
-  switch (byte_size) {
-  case 1:
-    return GetU8(offset_ptr);
-    break;
-  case 2:
-    return GetU16(offset_ptr);
-    break;
-  case 4:
-    return GetU32(offset_ptr);
-    break;
-  default:
-    assert(false && "GetMaxU32 unhandled case!");
-    break;
-  }
-  return 0;
+  lldbassert(byte_size > 0 && byte_size <= 4 && "GetMaxU32 invalid byte_size!");
+  return GetMaxU64(offset_ptr, byte_size);
 }
 
-//----------------------------------------------------------------------
-// Extract a single integer value from the data and update the offset
-// pointed to by "offset_ptr". The size of the extracted integer
-// is specified by the "byte_size" argument. "byte_size" should have
-// a value >= 1 and <= 8 since the return value is only 64 bits
-// wide. Any "byte_size" values less than 1 or greater than 8 will
-// result in nothing being extracted, and zero being returned.
-//
-// RETURNS the integer value that was extracted, or zero on failure.
-//----------------------------------------------------------------------
-uint64_t DataExtractor::GetMaxU64(offset_t *offset_ptr, size_t size) const {
-  switch (size) {
+uint64_t DataExtractor::GetMaxU64(offset_t *offset_ptr,
+                                  size_t byte_size) const {
+  lldbassert(byte_size > 0 && byte_size <= 8 && "GetMaxU64 invalid byte_size!");
+  switch (byte_size) {
   case 1:
     return GetU8(offset_ptr);
-    break;
   case 2:
     return GetU16(offset_ptr);
-    break;
   case 4:
     return GetU32(offset_ptr);
-    break;
   case 8:
     return GetU64(offset_ptr);
-    break;
-  default:
-    assert(false && "GetMax64 unhandled case!");
-    break;
+  default: {
+    // General case.
+    const uint8_t *data =
+        static_cast<const uint8_t *>(GetData(offset_ptr, byte_size));
+    if (data == nullptr)
+      return 0;
+    return ReadMaxInt64(data, byte_size, m_byte_order);
+  }
   }
   return 0;
 }
 
 uint64_t DataExtractor::GetMaxU64_unchecked(offset_t *offset_ptr,
-                                            size_t size) const {
-  switch (size) {
+                                            size_t byte_size) const {
+  switch (byte_size) {
   case 1:
     return GetU8_unchecked(offset_ptr);
-    break;
   case 2:
     return GetU16_unchecked(offset_ptr);
-    break;
   case 4:
     return GetU32_unchecked(offset_ptr);
-    break;
   case 8:
     return GetU64_unchecked(offset_ptr);
-    break;
-  default:
-    assert(false && "GetMax64 unhandled case!");
-    break;
+  default: {
+    uint64_t res = ReadMaxInt64(&m_start[*offset_ptr], byte_size, m_byte_order);
+    *offset_ptr += byte_size;
+    return res;
+  }
   }
   return 0;
 }
 
-int64_t DataExtractor::GetMaxS64(offset_t *offset_ptr, size_t size) const {
-  switch (size) {
-  case 1:
-    return (int8_t)GetU8(offset_ptr);
-    break;
-  case 2:
-    return (int16_t)GetU16(offset_ptr);
-    break;
-  case 4:
-    return (int32_t)GetU32(offset_ptr);
-    break;
-  case 8:
-    return (int64_t)GetU64(offset_ptr);
-    break;
-  default:
-    assert(false && "GetMax64 unhandled case!");
-    break;
-  }
-  return 0;
+int64_t DataExtractor::GetMaxS64(offset_t *offset_ptr, size_t byte_size) const {
+  uint64_t u64 = GetMaxU64(offset_ptr, byte_size);
+  return llvm::SignExtend64(u64, 8 * byte_size);
 }
 
 uint64_t DataExtractor::GetMaxU64Bitfield(offset_t *offset_ptr, size_t size,

Modified: lldb/trunk/unittests/Core/DataExtractorTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Core/DataExtractorTest.cpp?rev=315444&r1=315443&r2=315444&view=diff
==============================================================================
--- lldb/trunk/unittests/Core/DataExtractorTest.cpp (original)
+++ lldb/trunk/unittests/Core/DataExtractorTest.cpp Wed Oct 11 01:48:18 2017
@@ -49,3 +49,120 @@ TEST(DataExtractorTest, PeekData) {
   EXPECT_EQ(buffer + 4, E.PeekData(4, 0));
   EXPECT_EQ(nullptr, E.PeekData(4, 1));
 }
+
+TEST(DataExtractorTest, GetMaxU64) {
+  uint8_t buffer[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08};
+  DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+                   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
+
+  lldb::offset_t offset;
+
+  // Check with the minimum allowed byte size.
+  offset = 0;
+  EXPECT_EQ(0x01U, LE.GetMaxU64(&offset, 1));
+  EXPECT_EQ(1U, offset);
+  offset = 0;
+  EXPECT_EQ(0x01U, BE.GetMaxU64(&offset, 1));
+  EXPECT_EQ(1U, offset);
+
+  // Check with a non-zero offset.
+  offset = 1;
+  EXPECT_EQ(0x0302U, LE.GetMaxU64(&offset, 2));
+  EXPECT_EQ(3U, offset);
+  offset = 1;
+  EXPECT_EQ(0x0203U, BE.GetMaxU64(&offset, 2));
+  EXPECT_EQ(3U, offset);
+
+  // Check with the byte size not being a multiple of 2.
+  offset = 0;
+  EXPECT_EQ(0x07060504030201U, LE.GetMaxU64(&offset, 7));
+  EXPECT_EQ(7U, offset);
+  offset = 0;
+  EXPECT_EQ(0x01020304050607U, BE.GetMaxU64(&offset, 7));
+  EXPECT_EQ(7U, offset);
+
+  // Check with the maximum allowed byte size.
+  offset = 0;
+  EXPECT_EQ(0x0807060504030201U, LE.GetMaxU64(&offset, 8));
+  EXPECT_EQ(8U, offset);
+  offset = 0;
+  EXPECT_EQ(0x0102030405060708U, BE.GetMaxU64(&offset, 8));
+  EXPECT_EQ(8U, offset);
+}
+
+TEST(DataExtractorTest, GetMaxS64) {
+  uint8_t buffer[] = {0x01, 0x02, 0x83, 0x04, 0x05, 0x06, 0x07, 0x08};
+  DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+                   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
+
+  lldb::offset_t offset;
+
+  // Check with the minimum allowed byte size.
+  offset = 0;
+  EXPECT_EQ(0x01, LE.GetMaxS64(&offset, 1));
+  EXPECT_EQ(1U, offset);
+  offset = 0;
+  EXPECT_EQ(0x01, BE.GetMaxS64(&offset, 1));
+  EXPECT_EQ(1U, offset);
+
+  // Check that sign extension works correctly.
+  offset = 0;
+  int64_t value = LE.GetMaxS64(&offset, 3);
+  EXPECT_EQ(0xffffffffff830201U, *reinterpret_cast<uint64_t *>(&value));
+  EXPECT_EQ(3U, offset);
+  offset = 2;
+  value = BE.GetMaxS64(&offset, 3);
+  EXPECT_EQ(0xffffffffff830405U, *reinterpret_cast<uint64_t *>(&value));
+  EXPECT_EQ(5U, offset);
+
+  // Check with the maximum allowed byte size.
+  offset = 0;
+  EXPECT_EQ(0x0807060504830201, LE.GetMaxS64(&offset, 8));
+  EXPECT_EQ(8U, offset);
+  offset = 0;
+  EXPECT_EQ(0x0102830405060708, BE.GetMaxS64(&offset, 8));
+  EXPECT_EQ(8U, offset);
+}
+
+TEST(DataExtractorTest, GetMaxU64_unchecked) {
+  uint8_t buffer[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08};
+  DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+                   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
+
+  lldb::offset_t offset;
+
+  // Check with the minimum allowed byte size.
+  offset = 0;
+  EXPECT_EQ(0x01U, LE.GetMaxU64_unchecked(&offset, 1));
+  EXPECT_EQ(1U, offset);
+  offset = 0;
+  EXPECT_EQ(0x01U, BE.GetMaxU64_unchecked(&offset, 1));
+  EXPECT_EQ(1U, offset);
+
+  // Check with a non-zero offset.
+  offset = 1;
+  EXPECT_EQ(0x0302U, LE.GetMaxU64_unchecked(&offset, 2));
+  EXPECT_EQ(3U, offset);
+  offset = 1;
+  EXPECT_EQ(0x0203U, BE.GetMaxU64_unchecked(&offset, 2));
+  EXPECT_EQ(3U, offset);
+
+  // Check with the byte size not being a multiple of 2.
+  offset = 0;
+  EXPECT_EQ(0x07060504030201U, LE.GetMaxU64_unchecked(&offset, 7));
+  EXPECT_EQ(7U, offset);
+  offset = 0;
+  EXPECT_EQ(0x01020304050607U, BE.GetMaxU64_unchecked(&offset, 7));
+  EXPECT_EQ(7U, offset);
+
+  // Check with the maximum allowed byte size.
+  offset = 0;
+  EXPECT_EQ(0x0807060504030201U, LE.GetMaxU64_unchecked(&offset, 8));
+  EXPECT_EQ(8U, offset);
+  offset = 0;
+  EXPECT_EQ(0x0102030405060708U, BE.GetMaxU64_unchecked(&offset, 8));
+  EXPECT_EQ(8U, offset);
+}




More information about the lldb-commits mailing list