[Lldb-commits] [lldb] 9201463 - [lldb] Don't rely on wrapping in PutRawBytes/PutBytesAsRawHex8

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-02-15T20:38:25-08:00
New Revision: 920146316da1e55017d46cfd62be783419da03d5

URL: https://github.com/llvm/llvm-project/commit/920146316da1e55017d46cfd62be783419da03d5
DIFF: https://github.com/llvm/llvm-project/commit/920146316da1e55017d46cfd62be783419da03d5.diff

LOG: [lldb] Don't rely on wrapping in PutRawBytes/PutBytesAsRawHex8

I was looking at Stream::PutRawBytes and thought I spotted a bug because
both loops are using `i < src_len` as the loop condition despite them
iterating in opposite directions.

On closer inspection, the existing code is correct, because it relies on
well-defined unsigned integer wrapping. Correct doesn't mean readable,
so this patch changes the loop condition to compare against 0 when
decrementing i while still covering the edge case of src_len potentially
being 0 itself.

Differential revision: https://reviews.llvm.org/D119857

Added: 
    

Modified: 
    lldb/source/Utility/Stream.cpp
    lldb/unittests/Utility/StreamTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Utility/Stream.cpp b/lldb/source/Utility/Stream.cpp
index a1e2de9da4d98..b0f085418023f 100644
--- a/lldb/source/Utility/Stream.cpp
+++ b/lldb/source/Utility/Stream.cpp
@@ -344,8 +344,8 @@ size_t Stream::PutRawBytes(const void *s, size_t src_len,
     for (size_t i = 0; i < src_len; ++i)
       _PutHex8(src[i], false);
   } else {
-    for (size_t i = src_len - 1; i < src_len; --i)
-      _PutHex8(src[i], false);
+    for (size_t i = src_len; i > 0; --i)
+      _PutHex8(src[i - 1], false);
   }
   if (!binary_was_set)
     m_flags.Clear(eBinary);
@@ -357,6 +357,7 @@ size_t Stream::PutBytesAsRawHex8(const void *s, size_t src_len,
                                  ByteOrder src_byte_order,
                                  ByteOrder dst_byte_order) {
   ByteDelta delta(*this);
+
   if (src_byte_order == eByteOrderInvalid)
     src_byte_order = m_byte_order;
 
@@ -370,8 +371,8 @@ size_t Stream::PutBytesAsRawHex8(const void *s, size_t src_len,
     for (size_t i = 0; i < src_len; ++i)
       _PutHex8(src[i], false);
   } else {
-    for (size_t i = src_len - 1; i < src_len; --i)
-      _PutHex8(src[i], false);
+    for (size_t i = src_len; i > 0; --i)
+      _PutHex8(src[i - 1], false);
   }
   if (binary_is_set)
     m_flags.Set(eBinary);

diff  --git a/lldb/unittests/Utility/StreamTest.cpp b/lldb/unittests/Utility/StreamTest.cpp
index 940d49fdfdb7d..959097dc52f98 100644
--- a/lldb/unittests/Utility/StreamTest.cpp
+++ b/lldb/unittests/Utility/StreamTest.cpp
@@ -504,6 +504,30 @@ TEST_F(StreamTest, PutRawBytesToMixedEndian) {
 #endif
 }
 
+TEST_F(StreamTest, PutRawBytesZeroLenght) {
+  uint32_t value = 0x12345678;
+
+  s.PutRawBytes(static_cast<void *>(&value), 0, hostByteOrder,
+                lldb::eByteOrderLittle);
+  EXPECT_EQ(0U, s.GetWrittenBytes());
+
+  s.PutRawBytes(static_cast<void *>(&value), 0, hostByteOrder,
+                lldb::eByteOrderBig);
+  EXPECT_EQ(0U, s.GetWrittenBytes());
+}
+
+TEST_F(StreamTest, PutBytesAsRawHex8ZeroLenght) {
+  uint32_t value = 0x12345678;
+
+  s.PutBytesAsRawHex8(static_cast<void *>(&value), 0, hostByteOrder,
+                      lldb::eByteOrderLittle);
+  EXPECT_EQ(0U, s.GetWrittenBytes());
+
+  s.PutBytesAsRawHex8(static_cast<void *>(&value), 0, hostByteOrder,
+                      lldb::eByteOrderBig);
+  EXPECT_EQ(0U, s.GetWrittenBytes());
+}
+
 // ULEB128 support for binary streams.
 
 TEST_F(BinaryStreamTest, PutULEB128OneByte) {