[llvm/Support] Don't crash on empty nullptr ranges when decoding LEBs

[labath]

Summary

If the decoding functions are called with both start and end pointers
being nullptr, the function will crash due to a nullptr dereference.
This happens because the function does not recognise nullptr as a valid
end pointer.

Obviously, nobody is going to pass null pointers here deliberately, but
it can happen indirectly (as it did for me), when calling these
functions on an ArrayRef, as a default-initialized empty ArrayRef will
have both begin() and end() pointers equal to nullptr.

The fix is to simply remove the nullptr check. Passing nullptr for "end"
with a valid "begin" pointer will still work, as one cannot reach
nullptr by incrementing a valid pointer without triggerring UB.

[labath]

Upload the diff properly

[dblaikie]

llvm/include/llvm/Support/LEB128.h

137

Is it possible this functionality originally existed for accepting unbounded buffers? (like the classic unix gets function) by passing nullptr as the end bound it'd just keep reading from the buffer until it decoded a ULEB128?

While such functionality would be pretty problematic from a memory safety perspective - worth checking if any users might be relying on that? I guess it's more likely this was just copied in from somewhere else/some example implementation and retained the "Bonus" functionality without it being intentional, an if all the tests pass I'm fine removing this quirk/feature.

[labath]

llvm/include/llvm/Support/LEB128.h

137

Sounds good. I'll try to create a patch for that.

llvm/include/llvm/Support/LEB128.h

inline uint64_t decodeULEB128(const uint8_t *p, unsigned *n = nullptr,
                               const uint8_t *end = nullptr,
                               const char **error = nullptr) {
   const uint8_t *orig_p = p;
   uint64_t Value = 0;
   unsigned Shift = 0;
   if (error)
     *error = nullptr;
   do {
-    if (end && p == end) {
+    if (p == end) {
       if (error)
         *error = "malformed uleb128, extends past end";
       if (n)
         *n = (unsigned)(p - orig_p);
       return 0;
     }
     uint64_t Slice = *p & 0x7f;
     if (Shift >= 64 || Slice << Shift >> Shift != Slice) {
inline int64_t decodeSLEB128(const uint8_t *p, unsigned *n = nullptr,
                              const char **error = nullptr) {
   const uint8_t *orig_p = p;
   int64_t Value = 0;
   unsigned Shift = 0;
   uint8_t Byte;
   if (error)
     *error = nullptr;
   do {
-    if (end && p == end) {
+    if (p == end) {
       if (error)
         *error = "malformed sleb128, extends past end";
       if (n)
         *n = (unsigned)(p - orig_p);
       return 0;
     }
     Byte = *p++;
     Value |= (uint64_t(Byte & 0x7f) << Shift);

llvm/unittests/Support/LEB128Test.cpp

#define EXPECT_DECODE_ULEB128_EQ(EXPECTED, VALUE) \
   do { \
     unsigned ActualSize = 0; \
     uint64_t Actual = decodeULEB128(reinterpret_cast<const uint8_t *>(VALUE), \
                                     &ActualSize); \
     EXPECT_EQ(sizeof(VALUE) - 1, ActualSize); \
     EXPECT_EQ(EXPECTED, Actual); \
   } while (0)
 
+  // Don't crash
+  EXPECT_EQ(0u, decodeULEB128(nullptr, nullptr, nullptr));
+
   // Decode ULEB128
   EXPECT_DECODE_ULEB128_EQ(0u, "\x00");
   EXPECT_DECODE_ULEB128_EQ(1u, "\x01");
   EXPECT_DECODE_ULEB128_EQ(63u, "\x3f");
   EXPECT_DECODE_ULEB128_EQ(64u, "\x40");
   EXPECT_DECODE_ULEB128_EQ(0x7fu, "\x7f");
   EXPECT_DECODE_ULEB128_EQ(0x80u, "\x80\x01");
   EXPECT_DECODE_ULEB128_EQ(0x81u, "\x81\x01");
#define EXPECT_DECODE_SLEB128_EQ(EXPECTED, VALUE) \
   do { \
     unsigned ActualSize = 0; \
     int64_t Actual = decodeSLEB128(reinterpret_cast<const uint8_t *>(VALUE), \
                                     &ActualSize); \
     EXPECT_EQ(sizeof(VALUE) - 1, ActualSize); \
     EXPECT_EQ(EXPECTED, Actual); \
   } while (0)
 
+  // Don't crash
+  EXPECT_EQ(0, decodeSLEB128(nullptr, nullptr, nullptr));
+
   // Decode SLEB128
   EXPECT_DECODE_SLEB128_EQ(0L, "\x00");
   EXPECT_DECODE_SLEB128_EQ(1L, "\x01");
   EXPECT_DECODE_SLEB128_EQ(63L, "\x3f");
   EXPECT_DECODE_SLEB128_EQ(-64L, "\x40");
   EXPECT_DECODE_SLEB128_EQ(-63L, "\x41");
   EXPECT_DECODE_SLEB128_EQ(-1L, "\x7f");
   EXPECT_DECODE_SLEB128_EQ(128L, "\x80\x01");