[lldb] Don't rely on wrapping in PutRawBytes/PutBytesAsRawHex8
authorJonas Devlieghere <jonas@devlieghere.com>
Wed, 16 Feb 2022 04:35:52 +0000 (20:35 -0800)
committerJonas Devlieghere <jonas@devlieghere.com>
Wed, 16 Feb 2022 04:38:25 +0000 (20:38 -0800)
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

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

index a1e2de9..b0f0854 100644 (file)
@@ -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);
index 940d49f..959097d 100644 (file)
@@ -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) {