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

diff --git a/llvm/include/llvm/Support/LEB128.h b/llvm/include/llvm/Support/LEB128.h
index 8ab3543..9ba5f29 100644 (file)
--- 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 c09d7f1..83d4701 100644 (file)
--- 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 9a5d4cb..986d293 100644 (file)
--- 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