[llvm] 32e98f0 - Diagnose if a SLEB128 is too large to fit in an int64_t.

Richard Smith via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 2 14:33:47 PST 2021


Author: Richard Smith
Date: 2021-02-02T14:33:34-08:00
New Revision: 32e98f05fe108e7b25dcf2031c499b96a1436e1d

URL: https://github.com/llvm/llvm-project/commit/32e98f05fe108e7b25dcf2031c499b96a1436e1d
DIFF: https://github.com/llvm/llvm-project/commit/32e98f05fe108e7b25dcf2031c499b96a1436e1d.diff

LOG: Diagnose if a SLEB128 is too large to fit in an int64_t.

Previously we'd hit UB due to an invalid left shift operand.

Also fix the WASM emitter to properly use SLEB128 encoding instead of
ULEB128 encoding for signed fields so that negative numbers don't
result in overly-large values that we can't read back any more.

In passing, don't diagnose a non-canonical ULEB128 that fits in a uint64_t but
has redundant trailing zero bytes.

Reviewed By: dblaikie, aardappel

Differential Revision: https://reviews.llvm.org/D95510

Added: 
    

Modified: 
    llvm/include/llvm/Support/LEB128.h
    llvm/lib/ObjectYAML/WasmEmitter.cpp
    llvm/unittests/Support/LEB128Test.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/LEB128.h b/llvm/include/llvm/Support/LEB128.h
index 8ab35431354d..9ba5f29b7b44 100644
--- a/llvm/include/llvm/Support/LEB128.h
+++ b/llvm/include/llvm/Support/LEB128.h
@@ -142,14 +142,14 @@ inline uint64_t decodeULEB128(const uint8_t *p, unsigned *n = nullptr,
       return 0;
     }
     uint64_t Slice = *p & 0x7f;
-    if (Shift >= 64 || Slice << Shift >> Shift != Slice) {
+    if ((Shift >= 64 && Slice != 0) || Slice << Shift >> Shift != Slice) {
       if (error)
         *error = "uleb128 too big for uint64";
       if (n)
         *n = (unsigned)(p - orig_p);
       return 0;
     }
-    Value += uint64_t(*p & 0x7f) << Shift;
+    Value += Slice << Shift;
     Shift += 7;
   } while (*p++ >= 128);
   if (n)
@@ -175,9 +175,19 @@ inline int64_t decodeSLEB128(const uint8_t *p, unsigned *n = nullptr,
         *n = (unsigned)(p - orig_p);
       return 0;
     }
-    Byte = *p++;
-    Value |= (uint64_t(Byte & 0x7f) << Shift);
+    Byte = *p;
+    int64_t Slice = Byte & 0x7f;
+    if ((Shift >= 64 && Slice != (Value < 0 ? 0x7f : 0x00)) ||
+        (Shift == 63 && Slice != 0 && Slice != 0x7f)) {
+      if (error)
+        *error = "sleb128 too big for int64";
+      if (n)
+        *n = (unsigned)(p - orig_p);
+      return 0;
+    }
+    Value |= Slice << Shift;
     Shift += 7;
+    ++p;
   } while (Byte >= 128);
   // Sign extend negative numbers if needed.
   if (Shift < 64 && (Byte & 0x40))

diff  --git a/llvm/lib/ObjectYAML/WasmEmitter.cpp b/llvm/lib/ObjectYAML/WasmEmitter.cpp
index c09d7f1bc95a..83d4701ed3e6 100644
--- a/llvm/lib/ObjectYAML/WasmEmitter.cpp
+++ b/llvm/lib/ObjectYAML/WasmEmitter.cpp
@@ -575,7 +575,8 @@ void WasmWriter::writeRelocSection(raw_ostream &OS, WasmYAML::Section &Sec,
     case wasm::R_WASM_FUNCTION_OFFSET_I32:
     case wasm::R_WASM_FUNCTION_OFFSET_I64:
     case wasm::R_WASM_SECTION_OFFSET_I32:
-      encodeULEB128(Reloc.Addend, OS);
+      encodeSLEB128(Reloc.Addend, OS);
+      break;
     }
   }
 }

diff  --git a/llvm/unittests/Support/LEB128Test.cpp b/llvm/unittests/Support/LEB128Test.cpp
index 9a5d4cbaba3b..986d29358458 100644
--- a/llvm/unittests/Support/LEB128Test.cpp
+++ b/llvm/unittests/Support/LEB128Test.cpp
@@ -137,10 +137,37 @@ TEST(LEB128Test, DecodeULEB128) {
   EXPECT_DECODE_ULEB128_EQ(0x7fu, "\xff\x80\x00");
   EXPECT_DECODE_ULEB128_EQ(0x80u, "\x80\x81\x00");
   EXPECT_DECODE_ULEB128_EQ(0x80u, "\x80\x81\x80\x00");
+  EXPECT_DECODE_ULEB128_EQ(0x80u, "\x80\x81\x80\x80\x80\x80\x80\x80\x80\x00");
+  EXPECT_DECODE_ULEB128_EQ(0x80000000'00000000ul,
+                           "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x01");
 
 #undef EXPECT_DECODE_ULEB128_EQ
 }
 
+TEST(LEB128Test, DecodeInvalidULEB128) {
+#define EXPECT_INVALID_ULEB128(VALUE, ERROR_OFFSET)                            \
+  do {                                                                         \
+    const uint8_t *Value = reinterpret_cast<const uint8_t *>(VALUE);           \
+    const char *Error = nullptr;                                               \
+    unsigned ErrorOffset = 0;                                                  \
+    uint64_t Actual =                                                          \
+        decodeULEB128(Value, &ErrorOffset, Value + strlen(VALUE), &Error);     \
+    EXPECT_NE(Error, nullptr);                                                 \
+    EXPECT_EQ(0ul, Actual);                                                    \
+    EXPECT_EQ(ERROR_OFFSET, ErrorOffset);                                      \
+  } while (0)
+
+  // Buffer overflow.
+  EXPECT_INVALID_ULEB128("", 0u);
+  EXPECT_INVALID_ULEB128("\x80", 1u);
+
+  // Does not fit in 64 bits.
+  EXPECT_INVALID_ULEB128("\x80\x80\x80\x80\x80\x80\x80\x80\x80\x02", 9u);
+  EXPECT_INVALID_ULEB128("\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x02", 10u);
+
+#undef EXPECT_INVALID_ULEB128
+}
+
 TEST(LEB128Test, DecodeSLEB128) {
 #define EXPECT_DECODE_SLEB128_EQ(EXPECTED, VALUE) \
   do { \
@@ -176,10 +203,45 @@ TEST(LEB128Test, DecodeSLEB128) {
   EXPECT_DECODE_SLEB128_EQ(0x7fL, "\xff\x80\x00");
   EXPECT_DECODE_SLEB128_EQ(0x80L, "\x80\x81\x00");
   EXPECT_DECODE_SLEB128_EQ(0x80L, "\x80\x81\x80\x00");
+  EXPECT_DECODE_SLEB128_EQ(0x80L, "\x80\x81\x80\x80\x80\x80\x80\x80\x80\x00");
+  EXPECT_DECODE_SLEB128_EQ(-2L, "\xFE\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x7F");
+  EXPECT_DECODE_SLEB128_EQ(INT64_MIN,
+                           "\x80\x80\x80\x80\x80\x80\x80\x80\x80\x7F");
+  EXPECT_DECODE_SLEB128_EQ(INT64_MAX,
+                           "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x00");
 
 #undef EXPECT_DECODE_SLEB128_EQ
 }
 
+TEST(LEB128Test, DecodeInvalidSLEB128) {
+#define EXPECT_INVALID_SLEB128(VALUE, ERROR_OFFSET)                            \
+  do {                                                                         \
+    const uint8_t *Value = reinterpret_cast<const uint8_t *>(VALUE);           \
+    const char *Error = nullptr;                                               \
+    unsigned ErrorOffset = 0;                                                  \
+    uint64_t Actual =                                                          \
+        decodeSLEB128(Value, &ErrorOffset, Value + strlen(VALUE), &Error);     \
+    EXPECT_NE(Error, nullptr);                                                 \
+    EXPECT_EQ(0ul, Actual);                                                    \
+    EXPECT_EQ(ERROR_OFFSET, ErrorOffset);                                      \
+  } while (0)
+
+  // Buffer overflow.
+  EXPECT_INVALID_SLEB128("", 0u);
+  EXPECT_INVALID_SLEB128("\x80", 1u);
+
+  // Does not fit in 64 bits.
+  EXPECT_INVALID_SLEB128("\x80\x80\x80\x80\x80\x80\x80\x80\x80\x01", 9u);
+  EXPECT_INVALID_SLEB128("\x80\x80\x80\x80\x80\x80\x80\x80\x80\x7E", 9u);
+  EXPECT_INVALID_SLEB128("\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x02", 10u);
+  EXPECT_INVALID_SLEB128("\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x7E", 9u);
+  EXPECT_INVALID_SLEB128("\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x01", 9u);
+  EXPECT_INVALID_SLEB128("\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x7E", 10u);
+  EXPECT_INVALID_SLEB128("\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x00", 10u);
+
+#undef EXPECT_INVALID_SLEB128
+}
+
 TEST(LEB128Test, SLEB128Size) {
   // Positive Value Testing Plan:
   // (1) 128 ^ n - 1 ........ need (n+1) bytes


        


More information about the llvm-commits mailing list