From 4f37ed5ff6c18c8cf0de244bb064d5f4f60065a4 Mon Sep 17 00:00:00 2001 From: Jessica Clarke Date: Fri, 18 Oct 2024 16:50:22 +0100 Subject: [PATCH 1/2] [clang] Make LazyOffsetPtr more portable LazyOffsetPtr currently relies on uint64_t being able to store a pointer and, unless sizeof(uint64_t) == sizeof(void *), little endianness, since getAddressOfPointer reinterprets the memory as a pointer. This also doesn't properly respect the C++ object model. As removing getAddressOfPointer would have wide-reaching implications, improve the implementation to account for these problems by using placement new and a suitably sized-and-aligned buffer, "right"-aligning the objects on big-endian platforms so the LSBs are in the same place for use as the discriminator. Fixes: bc73ef0031b50f7443615fef614fb4ecaaa4bd11 Fixes: https://github.com/llvm/llvm-project/issues/111993 --- clang/include/clang/AST/ExternalASTSource.h | 48 +++++++++++++++------ 1 file changed, 35 insertions(+), 13 deletions(-) diff --git clang/include/clang/AST/ExternalASTSource.h clang/include/clang/AST/ExternalASTSource.h index 385c32edbae0fd..caf37144d5eb73 100644 --- clang/include/clang/AST/ExternalASTSource.h +++ clang/include/clang/AST/ExternalASTSource.h @@ -25,10 +25,12 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/iterator.h" #include "llvm/Support/PointerLikeTypeTraits.h" +#include #include #include #include #include +#include #include #include @@ -326,29 +328,49 @@ struct LazyOffsetPtr { /// /// If the low bit is clear, a pointer to the AST node. If the low /// bit is set, the upper 63 bits are the offset. - mutable uint64_t Ptr = 0; + static constexpr size_t DataSize = std::max(sizeof(uint64_t), sizeof(T *)); + alignas(uint64_t) alignas(T *) mutable unsigned char Data[DataSize] = {}; + + unsigned char GetLSB() const { + return Data[llvm::sys::IsBigEndianHost ? DataSize - 1 : 0]; + } + + template U &As(bool New) const { + unsigned char *Obj = + Data + (llvm::sys::IsBigEndianHost ? DataSize - sizeof(U) : 0); + if (New) + return *new (Obj) U; + return *std::launder(reinterpret_cast(Obj)); + } + + T *&GetPtr() const { return As(false); } + uint64_t &GetU64() const { return As(false); } + void SetPtr(T *Ptr) const { As(true) = Ptr; } + void SetU64(uint64_t U64) const { As(true) = U64; } public: LazyOffsetPtr() = default; - explicit LazyOffsetPtr(T *Ptr) : Ptr(reinterpret_cast(Ptr)) {} + explicit LazyOffsetPtr(T *Ptr) : Data() { SetPtr(Ptr); } - explicit LazyOffsetPtr(uint64_t Offset) : Ptr((Offset << 1) | 0x01) { + explicit LazyOffsetPtr(uint64_t Offset) : Data() { assert((Offset << 1 >> 1) == Offset && "Offsets must require < 63 bits"); if (Offset == 0) - Ptr = 0; + SetPtr(NULL); + else + SetU64((Offset << 1) | 0x01); } LazyOffsetPtr &operator=(T *Ptr) { - this->Ptr = reinterpret_cast(Ptr); + SetPtr(Ptr); return *this; } LazyOffsetPtr &operator=(uint64_t Offset) { assert((Offset << 1 >> 1) == Offset && "Offsets must require < 63 bits"); if (Offset == 0) - Ptr = 0; + SetPtr(NULL); else - Ptr = (Offset << 1) | 0x01; + SetU64((Offset << 1) | 0x01); return *this; } @@ -356,15 +378,15 @@ struct LazyOffsetPtr { /// Whether this pointer is non-NULL. /// /// This operation does not require the AST node to be deserialized. - explicit operator bool() const { return Ptr != 0; } + explicit operator bool() const { return isOffset() || GetPtr() != NULL; } /// Whether this pointer is non-NULL. /// /// This operation does not require the AST node to be deserialized. - bool isValid() const { return Ptr != 0; } + bool isValid() const { return isOffset() || GetPtr() != NULL; } /// Whether this pointer is currently stored as an offset. - bool isOffset() const { return Ptr & 0x01; } + bool isOffset() const { return GetLSB() & 0x01; } /// Retrieve the pointer to the AST node that this lazy pointer points to. /// @@ -375,9 +397,9 @@ struct LazyOffsetPtr { if (isOffset()) { assert(Source && "Cannot deserialize a lazy pointer without an AST source"); - Ptr = reinterpret_cast((Source->*Get)(Ptr >> 1)); + SetPtr((Source->*Get)(GetU64() >> 1)); } - return reinterpret_cast(Ptr); + return GetPtr(); } /// Retrieve the address of the AST node pointer. Deserializes the pointee if @@ -385,7 +407,7 @@ struct LazyOffsetPtr { T **getAddressOfPointer(ExternalASTSource *Source) const { // Ensure the integer is in pointer form. (void)get(Source); - return reinterpret_cast(&Ptr); + return &GetPtr(); } }; From 35eed5f3f5e09c4275cabac09e277a167a98f742 Mon Sep 17 00:00:00 2001 From: Jessica Clarke Date: Fri, 18 Oct 2024 17:45:28 +0100 Subject: [PATCH 2/2] NULL -> nullptr --- clang/include/clang/AST/ExternalASTSource.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git clang/include/clang/AST/ExternalASTSource.h clang/include/clang/AST/ExternalASTSource.h index caf37144d5eb73..582ed7c65f58ca 100644 --- clang/include/clang/AST/ExternalASTSource.h +++ clang/include/clang/AST/ExternalASTSource.h @@ -355,7 +355,7 @@ struct LazyOffsetPtr { explicit LazyOffsetPtr(uint64_t Offset) : Data() { assert((Offset << 1 >> 1) == Offset && "Offsets must require < 63 bits"); if (Offset == 0) - SetPtr(NULL); + SetPtr(nullptr); else SetU64((Offset << 1) | 0x01); } @@ -368,7 +368,7 @@ struct LazyOffsetPtr { LazyOffsetPtr &operator=(uint64_t Offset) { assert((Offset << 1 >> 1) == Offset && "Offsets must require < 63 bits"); if (Offset == 0) - SetPtr(NULL); + SetPtr(nullptr); else SetU64((Offset << 1) | 0x01); @@ -378,12 +378,12 @@ struct LazyOffsetPtr { /// Whether this pointer is non-NULL. /// /// This operation does not require the AST node to be deserialized. - explicit operator bool() const { return isOffset() || GetPtr() != NULL; } + explicit operator bool() const { return isOffset() || GetPtr() != nullptr; } /// Whether this pointer is non-NULL. /// /// This operation does not require the AST node to be deserialized. - bool isValid() const { return isOffset() || GetPtr() != NULL; } + bool isValid() const { return isOffset() || GetPtr() != nullptr; } /// Whether this pointer is currently stored as an offset. bool isOffset() const { return GetLSB() & 0x01; }