[PATCH] D91405: [clangd] Assert on varint encoding

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 305094.
kadircet marked an inline comment as done.
kadircet added a comment.

- Increase width of `B` to prevent integer promotion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91405/new/

https://reviews.llvm.org/D91405

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp


Index: clang-tools-extra/clangd/unittests/SerializationTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -366,9 +366,9 @@
   Pos += FileDigest.size();
 
   // Varints are little-endian base-128 numbers, where the top-bit of each byte
-  // indicates whether there are more. 8fffffff7f -> 0xffffffff.
+  // indicates whether there are more. ffffffff0f -> 0xffffffff.
   std::string CorruptSrcs =
-      (Srcs->Data.take_front(Pos) + llvm::fromHex("8fffffff7f") +
+      (Srcs->Data.take_front(Pos) + llvm::fromHex("ffffffff0f") +
        "some_random_garbage")
           .str();
   Srcs->Data = CorruptSrcs;
Index: clang-tools-extra/clangd/index/Serialization.cpp
===================================================================
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -21,6 +21,7 @@
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
+#include <cstdint>
 #include <vector>
 
 namespace clang {
@@ -81,12 +82,16 @@
 
   uint32_t consumeVar() {
     constexpr static uint8_t More = 1 << 7;
-    uint8_t B = consume8();
+
+    // Use a 32 bit unsigned here to prevent promotion to signed int (unless int
+    // is wider than 32 bits).
+    uint32_t B = consume8();
     if (LLVM_LIKELY(!(B & More)))
       return B;
     uint32_t Val = B & ~More;
     for (int Shift = 7; B & More && Shift < 32; Shift += 7) {
       B = consume8();
+      assert((B & ~More) < (1 << (32 - Shift)) && "Invalid varint encoding");
       Val |= (B & ~More) << Shift;
     }
     return Val;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91405.305094.patch
Type: text/x-patch
Size: 1754 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201113/b9b1b04d/attachment.bin>