sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/index/Serialization.cpp:90
B = consume8();
+ assert((B & ~More) < (1 << (32 - Shift)) && "Invalid varint encoding");
Val |= (B & ~More) << Shift;
----------------
I'm fine with an assert, but this still leaves us with the suspected UB in
NDEBUG mode, which doesn't seem OK.
(As noted on D91258 I'm not sure this is actual UB under C++ rules)
The issue here is integral promotion rules say that uint_8 << anything uses
*signed* arithmetic. And same for a bunch of the other operations.
I think the clearest thing to do is explicitly type B as uint32_t, so no
expression it is in can ever be promoted to (signed) int.
(Unless int is bigger than 32 bits, in which case we've always got enough bits
to shift into)
================
Comment at: clang-tools-extra/clangd/unittests/SerializationTests.cpp:371
std::string CorruptSrcs =
- (Srcs->Data.take_front(Pos) + llvm::fromHex("8fffffff7f") +
+ (Srcs->Data.take_front(Pos) + llvm::fromHex("ffffffff0f") +
"some_random_garbage")
----------------
/shamecube
In my defense, I grew up on a 68k mac... little endian is weird
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91405/new/
https://reviews.llvm.org/D91405
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
