[PATCH] D63645: [Support] Fix error handling in DataExtractor::get[US]LEB128

[Pavel Labath via Phabricator via llvm-commits]

labath created this revision.
labath added reviewers: dblaikie, probinson, bkramer.
Herald added a subscriber: kristina.
Herald added a project: LLVM.

These functions are documented as not modifying the offset argument if
the extraction fails (just like other DataExtractor functions). However,
while reviewing D63591 <https://reviews.llvm.org/D63591> we discovered that this is not the case -- if the
function reaches the end of the data buffer, it will just return the
value parsed until that point and set offset to point to the end of the
buffer.

This fixes the functions to act as advertised, and adds a regression
test.


Repository:
  rL LLVM

https://reviews.llvm.org/D63645

Files:
  lib/Support/DataExtractor.cpp
  unittests/Support/DataExtractorTest.cpp


Index: unittests/Support/DataExtractorTest.cpp
===================================================================
--- unittests/Support/DataExtractorTest.cpp
+++ unittests/Support/DataExtractorTest.cpp
@@ -116,4 +116,15 @@
   EXPECT_EQ(8U, offset);
 }
 
+TEST(DataExtractorTest, LEB128_error) {
+  DataExtractor DE(StringRef("\x81"), false, 8);
+  uint32_t Offset = 0;
+  EXPECT_EQ(0U, DE.getULEB128(&Offset));
+  EXPECT_EQ(0U, Offset);
+
+  Offset = 0;
+  EXPECT_EQ(0U, DE.getSLEB128(&Offset));
+  EXPECT_EQ(0U, Offset);
+}
+
 }
Index: lib/Support/DataExtractor.cpp
===================================================================
--- lib/Support/DataExtractor.cpp
+++ lib/Support/DataExtractor.cpp
@@ -157,12 +157,12 @@
     byte = Data[offset++];
     result |= uint64_t(byte & 0x7f) << shift;
     shift += 7;
-    if ((byte & 0x80) == 0)
-      break;
+    if ((byte & 0x80) == 0) {
+      *offset_ptr = offset;
+      return result;
+    }
   }
-
-  *offset_ptr = offset;
-  return result;
+  return 0;
 }
 
 int64_t DataExtractor::getSLEB128(uint32_t *offset_ptr) const {
@@ -178,14 +178,14 @@
     byte = Data[offset++];
     result |= uint64_t(byte & 0x7f) << shift;
     shift += 7;
-    if ((byte & 0x80) == 0)
-      break;
+    if ((byte & 0x80) == 0) {
+      // Sign bit of byte is 2nd high order bit (0x40)
+      if (shift < 64 && (byte & 0x40))
+        result |= -(1ULL << shift);
+
+      *offset_ptr = offset;
+      return result;
+    }
   }
-
-  // Sign bit of byte is 2nd high order bit (0x40)
-  if (shift < 64 && (byte & 0x40))
-    result |= -(1ULL << shift);
-
-  *offset_ptr = offset;
-  return result;
+  return 0;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63645.205975.patch
Type: text/x-patch
Size: 1653 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190621/2dc369bb/attachment.bin>