diff --git a/clang/include/clang/Interpreter/Value.h b/clang/include/clang/Interpreter/Value.h index b91301e6096eb..23ef123ded8ee 100644 --- a/clang/include/clang/Interpreter/Value.h +++ b/clang/include/clang/Interpreter/Value.h @@ -98,7 +98,7 @@ class REPL_EXTERNAL_VISIBILITY Value { REPL_BUILTIN_TYPES #undef X void *m_Ptr; - unsigned char m_RawBits[sizeof(long double) * 8]; // widest type + unsigned char m_RawBits[sizeof(long double)]; // widest typed member }; public: @@ -140,7 +140,10 @@ class REPL_EXTERNAL_VISIBILITY Value { void *getPtr() const; void setPtr(void *Ptr) { Data.m_Ptr = Ptr; } - void setRawBits(void *Ptr, unsigned NBits = sizeof(Storage)); + /// Copy `NBytes` bytes from `Ptr` into the raw storage. Default copies + /// the full Storage width. Used by the value printer to read a single + /// array element through a typed lens without an extra heap allocation. + void setRawBits(void *Ptr, unsigned NBytes = sizeof(Storage)); #define X(type, name) \ void set##name(type Val) { Data.m_##name = Val; } \ diff --git a/clang/lib/Interpreter/InterpreterValuePrinter.cpp b/clang/lib/Interpreter/InterpreterValuePrinter.cpp index 1754e7812469a..79f1e2b6571c6 100644 --- a/clang/lib/Interpreter/InterpreterValuePrinter.cpp +++ b/clang/lib/Interpreter/InterpreterValuePrinter.cpp @@ -204,7 +204,7 @@ std::string Interpreter::ValueDataToString(const Value &V) const { if (ElemTy->isBuiltinType()) { // Single dim arrays, advancing. uintptr_t Offset = (uintptr_t)V.getPtr() + Idx * ElemSize; - InnerV.setRawBits((void *)Offset, ElemSize * 8); + InnerV.setRawBits((void *)Offset, ElemSize); } else { // Multi dim arrays, position to the next dimension. size_t Stride = ElemCount / N; diff --git a/clang/lib/Interpreter/Value.cpp b/clang/lib/Interpreter/Value.cpp index d4c9d51ffcb61..b985361ed748a 100644 --- a/clang/lib/Interpreter/Value.cpp +++ b/clang/lib/Interpreter/Value.cpp @@ -229,9 +229,9 @@ void *Value::getPtr() const { return Data.m_Ptr; } -void Value::setRawBits(void *Ptr, unsigned NBits /*= sizeof(Storage)*/) { - assert(NBits <= sizeof(Storage) && "Greater than the total size"); - memcpy(/*dest=*/Data.m_RawBits, /*src=*/Ptr, /*nbytes=*/NBits / 8); +void Value::setRawBits(void *Ptr, unsigned NBytes /*= sizeof(Storage)*/) { + assert(NBytes <= sizeof(Storage) && "Greater than the total size"); + memcpy(/*dest=*/Data.m_RawBits, /*src=*/Ptr, /*nbytes=*/NBytes); } QualType Value::getType() const { diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp b/clang/unittests/Interpreter/InterpreterTest.cpp index 9ff9092524d21..2df29b3d5def5 100644 --- a/clang/unittests/Interpreter/InterpreterTest.cpp +++ b/clang/unittests/Interpreter/InterpreterTest.cpp @@ -421,6 +421,37 @@ TEST_F(InterpreterTest, Value) { EXPECT_STREQ(prettyPrint.c_str(), "(D) (One) : unsigned int 1\n"); } +// Regression: Value::setRawBits's NBytes parameter must be interpreted as a +// byte count end-to-end. Before this was fixed, the parameter was named +// NBits and the memcpy divided by 8, so a caller passing sizeof(T) (the +// natural byte count) ended up copying only sizeof(T)/8 bytes -- leaving +// the upper bytes uninitialised. The only in-tree caller compensated by +// multiplying by 8, hiding the bug. +TEST_F(InterpreterTest, ValueSetRawBitsCopiesByteCount) { + std::vector Args; + std::unique_ptr Interp = createInterpreter(Args); + + // Explicit byte count: writing sizeof(long long) bytes must round-trip + // every byte. Pre-fix this copied 1 byte (8 / 8) and left the upper 7 + // bytes stale. + Value V; + llvm::cantFail(Interp->ParseAndExecute("long long x = 0; x", &V)); + ASSERT_EQ(V.getKind(), Value::K_LongLong); + long long Src = 0x0123456789ABCDEFLL; + V.setRawBits(&Src, sizeof(Src)); + EXPECT_EQ(V.getLongLong(), Src); + + // Default NBytes argument copies sizeof(Storage). Pre-fix this copied + // sizeof(Storage) / 8 bytes, dropping the high half of an 8-byte payload. + Value V2; + llvm::cantFail(Interp->ParseAndExecute("long long y = 0; y", &V2)); + ASSERT_EQ(V2.getKind(), Value::K_LongLong); + unsigned char Buf[sizeof(long double)] = {}; + std::memcpy(Buf, &Src, sizeof(Src)); + V2.setRawBits(Buf); + EXPECT_EQ(V2.getLongLong(), Src); +} + TEST_F(InterpreterTest, TranslationUnit_CanonicalDecl) { std::vector Args; std::unique_ptr Interp = createInterpreter(Args);