llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> Add a path and an `IsDerivedMember` member to our `MemberPointer` class. Fix base-to-derived/derived-to-base casts. Add tests and unit tests, since regular tests allow a lot and we want to check the path size exactly. --- Patch is 27.94 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/179050.diff 10 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+43-17) - (modified) clang/lib/AST/ByteCode/Interp.cpp (+105-7) - (modified) clang/lib/AST/ByteCode/Interp.h (+14-6) - (modified) clang/lib/AST/ByteCode/InterpState.h (+5) - (modified) clang/lib/AST/ByteCode/MemberPointer.cpp (+8-7) - (modified) clang/lib/AST/ByteCode/MemberPointer.h (+75-20) - (modified) clang/lib/AST/ByteCode/Opcodes.td (+6-2) - (modified) clang/lib/AST/ByteCode/PrimType.h (+4-2) - (modified) clang/test/AST/ByteCode/memberpointers.cpp (+27) - (modified) clang/unittests/AST/ByteCode/toAPValue.cpp (+89-5) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index da1db48c55a07..7500db2c29c89 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -269,34 +269,61 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) { } case CK_DerivedToBaseMemberPointer: { - assert(classifyPrim(CE->getType()) == PT_MemberPtr); - assert(classifyPrim(SubExpr->getType()) == PT_MemberPtr); - const auto *FromMP = SubExpr->getType()->castAs<MemberPointerType>(); - const auto *ToMP = CE->getType()->castAs<MemberPointerType>(); - - unsigned DerivedOffset = - Ctx.collectBaseOffset(ToMP->getMostRecentCXXRecordDecl(), - FromMP->getMostRecentCXXRecordDecl()); + assert(classifyPrim(CE) == PT_MemberPtr); + assert(classifyPrim(SubExpr) == PT_MemberPtr); if (!this->delegate(SubExpr)) return false; - return this->emitGetMemberPtrBasePop(DerivedOffset, CE); + const CXXRecordDecl *CurDecl = SubExpr->getType() + ->castAs<MemberPointerType>() + ->getMostRecentCXXRecordDecl(); + for (const CXXBaseSpecifier *B : CE->path()) { + const CXXRecordDecl *ToDecl = B->getType()->getAsCXXRecordDecl(); + unsigned DerivedOffset = Ctx.collectBaseOffset(ToDecl, CurDecl); + + if (!this->emitCastMemberPtrBasePop(DerivedOffset, ToDecl, CE)) + return false; + CurDecl = ToDecl; + } + + return true; } case CK_BaseToDerivedMemberPointer: { assert(classifyPrim(CE) == PT_MemberPtr); assert(classifyPrim(SubExpr) == PT_MemberPtr); - const auto *FromMP = SubExpr->getType()->castAs<MemberPointerType>(); - const auto *ToMP = CE->getType()->castAs<MemberPointerType>(); - - unsigned DerivedOffset = - Ctx.collectBaseOffset(FromMP->getMostRecentCXXRecordDecl(), - ToMP->getMostRecentCXXRecordDecl()); if (!this->delegate(SubExpr)) return false; - return this->emitGetMemberPtrBasePop(-DerivedOffset, CE); + + const CXXRecordDecl *CurDecl = SubExpr->getType() + ->castAs<MemberPointerType>() + ->getMostRecentCXXRecordDecl(); + // Base-to-derived member pointer casts store the path in derived-to-base + // order, so iterate backwards. The CXXBaseSpecifier also provides us with + // the wrong end of the derived->base arc, so stagger the path by one class. + typedef std::reverse_iterator<CastExpr::path_const_iterator> ReverseIter; + for (ReverseIter PathI(CE->path_end() - 1), PathE(CE->path_begin()); + PathI != PathE; ++PathI) { + const CXXRecordDecl *ToDecl = (*PathI)->getType()->getAsCXXRecordDecl(); + unsigned DerivedOffset = Ctx.collectBaseOffset(CurDecl, ToDecl); + + if (!this->emitCastMemberPtrDerivedPop(-DerivedOffset, ToDecl, CE)) + return false; + CurDecl = ToDecl; + } + + const CXXRecordDecl *ToDecl = CE->getType() + ->castAs<MemberPointerType>() + ->getMostRecentCXXRecordDecl(); + assert(ToDecl != CurDecl); + unsigned DerivedOffset = Ctx.collectBaseOffset(CurDecl, ToDecl); + + if (!this->emitCastMemberPtrDerivedPop(-DerivedOffset, ToDecl, CE)) + return false; + + return true; } case CK_UncheckedDerivedToBase: @@ -7559,7 +7586,6 @@ bool Compiler<Emitter>::emitDestructionPop(const Descriptor *Desc, template <class Emitter> bool Compiler<Emitter>::emitDummyPtr(const DeclTy &D, const Expr *E) { assert(!DiscardResult && "Should've been checked before"); - unsigned DummyID = P.getOrCreateDummy(D); if (!this->emitGetPtrGlobal(DummyID, E)) diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 2a495a475c378..699cf3c041464 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -2336,7 +2336,6 @@ bool arePotentiallyOverlappingStringLiterals(const Pointer &LHS, static void copyPrimitiveMemory(InterpState &S, const Pointer &Ptr, PrimType T) { - if (T == PT_IntAPS) { auto &Val = Ptr.deref<IntegralAP<true>>(); if (!Val.singleWord()) { @@ -2355,16 +2354,34 @@ static void copyPrimitiveMemory(InterpState &S, const Pointer &Ptr, uint64_t *NewMemory = new (S.P) uint64_t[Val.numWords()]; Val.take(NewMemory); } + } else if (T == PT_MemberPtr) { + auto &Val = Ptr.deref<MemberPointer>(); + unsigned PathLength = Val.getPathLength(); + auto *NewPath = new (S.P) const CXXRecordDecl *[PathLength]; + for (unsigned I = 0; I != PathLength; ++I) { + NewPath[I] = Val.getPathEntry(I); + } + Val.takePath(NewPath); } } template <typename T> static void copyPrimitiveMemory(InterpState &S, const Pointer &Ptr) { assert(needsAlloc<T>()); - auto &Val = Ptr.deref<T>(); - if (!Val.singleWord()) { - uint64_t *NewMemory = new (S.P) uint64_t[Val.numWords()]; - Val.take(NewMemory); + if constexpr (std::is_same_v<T, MemberPointer>) { + auto &Val = Ptr.deref<MemberPointer>(); + unsigned PathLength = Val.getPathLength(); + auto *NewPath = new (S.P) const CXXRecordDecl *[PathLength]; + for (unsigned I = 0; I != PathLength; ++I) { + NewPath[I] = Val.getPathEntry(I); + } + Val.takePath(NewPath); + } else { + auto &Val = Ptr.deref<T>(); + if (!Val.singleWord()) { + uint64_t *NewMemory = new (S.P) uint64_t[Val.numWords()]; + Val.take(NewMemory); + } } } @@ -2375,9 +2392,9 @@ static void finishGlobalRecurse(InterpState &S, const Pointer &Ptr) { TYPE_SWITCH_ALLOC(Fi.Desc->getPrimType(), { copyPrimitiveMemory<T>(S, Ptr.atField(Fi.Offset)); }); - copyPrimitiveMemory(S, Ptr.atField(Fi.Offset), Fi.Desc->getPrimType()); - } else + } else { finishGlobalRecurse(S, Ptr.atField(Fi.Offset)); + } } return; } @@ -2491,6 +2508,87 @@ bool Destroy(InterpState &S, CodePtr OpPC, uint32_t I) { return true; } +// Perform a cast towards the class of the Decl (either up or down the +// hierarchy). +static bool castBackMemberPointer(InterpState &S, + const MemberPointer &MemberPtr, + int32_t BaseOffset, + const RecordDecl *BaseDecl) { + const CXXRecordDecl *Expected; + if (MemberPtr.getPathLength() >= 2) + Expected = MemberPtr.getPathEntry(MemberPtr.getPathLength() - 2); + else + Expected = MemberPtr.getRecordDecl(); + + assert(Expected); + if (Expected->getCanonicalDecl() != BaseDecl->getCanonicalDecl()) { + // C++11 [expr.static.cast]p12: In a conversion from (D::*) to (B::*), + // if B does not contain the original member and is not a base or + // derived class of the class containing the original member, the result + // of the cast is undefined. + // C++11 [conv.mem]p2 does not cover this case for a cast from (B::*) to + // (D::*). We consider that to be a language defect. + return false; + } + + unsigned OldPathLength = MemberPtr.getPathLength(); + unsigned NewPathLength = OldPathLength - 1; + bool IsDerivedMember = NewPathLength != 0; + auto NewPath = S.allocMemberPointerPath(NewPathLength); + for (unsigned I = 0; I != NewPathLength; ++I) { + NewPath[I] = MemberPtr.getPathEntry(I); + } + + S.Stk.push<MemberPointer>(MemberPtr.atInstanceBase(BaseOffset, NewPathLength, + NewPath, IsDerivedMember)); + return true; +} + +static bool appendToMemberPointer(InterpState &S, + const MemberPointer &MemberPtr, + int32_t BaseOffset, + const RecordDecl *BaseDecl, + bool IsDerivedMember) { + unsigned OldPathLength = MemberPtr.getPathLength(); + unsigned NewPathLength = OldPathLength + 1; + + auto NewPath = S.allocMemberPointerPath(NewPathLength); + for (unsigned I = 0; I != OldPathLength; ++I) { + NewPath[I] = MemberPtr.getPathEntry(I); + } + NewPath[OldPathLength] = cast<CXXRecordDecl>(BaseDecl); + + S.Stk.push<MemberPointer>(MemberPtr.atInstanceBase(BaseOffset, NewPathLength, + NewPath, IsDerivedMember)); + return true; +} + +/// DerivedToBaseMemberPointer +bool CastMemberPtrBasePop(InterpState &S, CodePtr OpPC, int32_t Off, + const RecordDecl *BaseDecl) { + const auto &Ptr = S.Stk.pop<MemberPointer>(); + + if (!Ptr.isDerivedMember() && Ptr.hasPath()) + return castBackMemberPointer(S, Ptr, Off, BaseDecl); + + bool IsDerivedMember = Ptr.isDerivedMember() || !Ptr.hasPath(); + return appendToMemberPointer(S, Ptr, Off, BaseDecl, IsDerivedMember); +} + +/// BaseToDerivedMemberPointer +bool CastMemberPtrDerivedPop(InterpState &S, CodePtr OpPC, int32_t Off, + const RecordDecl *BaseDecl) { + const auto &Ptr = S.Stk.pop<MemberPointer>(); + + if (!Ptr.isDerivedMember()) { + // Simply append. + return appendToMemberPointer(S, Ptr, Off, BaseDecl, + /*IsDerivedMember=*/false); + } + + return castBackMemberPointer(S, Ptr, Off, BaseDecl); +} + // https://github.com/llvm/llvm-project/issues/102513 #if defined(_MSC_VER) && !defined(__clang__) && !defined(NDEBUG) #pragma optimize("", off) diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index d856cd7c0a2d9..6a04bdbf73bb1 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -217,6 +217,12 @@ bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR); bool InvalidDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR, bool InitializerFailed); +/// DerivedToBaseMemberPointer +bool CastMemberPtrBasePop(InterpState &S, CodePtr OpPC, int32_t Off, + const RecordDecl *BaseDecl); +/// BaseToDerivedMemberPointer +bool CastMemberPtrDerivedPop(InterpState &S, CodePtr OpPC, int32_t Off, + const RecordDecl *BaseDecl); enum class ArithOp { Add, Sub }; //===----------------------------------------------------------------------===// @@ -1544,6 +1550,14 @@ bool InitGlobal(InterpState &S, CodePtr OpPC, uint32_t I) { Val.take(NewMemory); } + } else if constexpr (std::is_same_v<T, MemberPointer>) { + auto &Val = P.deref<MemberPointer>(); + unsigned PathLength = Val.getPathLength(); + auto *NewPath = new (S.P) const CXXRecordDecl *[PathLength]; + for (unsigned I = 0; I != PathLength; ++I) { + NewPath[I] = Val.getPathEntry(I); + } + Val.takePath(NewPath); } else if constexpr (needsAlloc<T>()) { auto &Val = P.deref<T>(); if (!Val.singleWord()) { @@ -1869,12 +1883,6 @@ inline bool GetPtrBasePop(InterpState &S, CodePtr OpPC, uint32_t Off, return true; } -inline bool GetMemberPtrBasePop(InterpState &S, CodePtr OpPC, int32_t Off) { - const auto &Ptr = S.Stk.pop<MemberPointer>(); - S.Stk.push<MemberPointer>(Ptr.atInstanceBase(Off)); - return true; -} - inline bool GetPtrThisBase(InterpState &S, CodePtr OpPC, uint32_t Off) { if (S.checkingPotentialConstantExpression()) return false; diff --git a/clang/lib/AST/ByteCode/InterpState.h b/clang/lib/AST/ByteCode/InterpState.h index 231b51124c70d..4ba1a2666307b 100644 --- a/clang/lib/AST/ByteCode/InterpState.h +++ b/clang/lib/AST/ByteCode/InterpState.h @@ -146,6 +146,11 @@ class InterpState final : public State, public SourceMapper { return Floating(Mem, llvm::APFloatBase::SemanticsToEnum(Sem)); } + const CXXRecordDecl **allocMemberPointerPath(unsigned Length) { + return reinterpret_cast<const CXXRecordDecl **>( + this->allocate(Length * sizeof(CXXRecordDecl *))); + } + private: friend class EvaluationResult; friend class InterpStateCCOverride; diff --git a/clang/lib/AST/ByteCode/MemberPointer.cpp b/clang/lib/AST/ByteCode/MemberPointer.cpp index 8b1b0187818e9..ddd654238af17 100644 --- a/clang/lib/AST/ByteCode/MemberPointer.cpp +++ b/clang/lib/AST/ByteCode/MemberPointer.cpp @@ -16,9 +16,9 @@ namespace clang { namespace interp { std::optional<Pointer> MemberPointer::toPointer(const Context &Ctx) const { - if (!Dcl || isa<FunctionDecl>(Dcl)) + if (!getDecl() || isa<FunctionDecl>(getDecl())) return Base; - assert((isa<FieldDecl, IndirectFieldDecl>(Dcl))); + assert((isa<FieldDecl, IndirectFieldDecl>(getDecl()))); if (!Base.isBlockPointer()) return std::nullopt; @@ -42,7 +42,7 @@ std::optional<Pointer> MemberPointer::toPointer(const Context &Ctx) const { unsigned Offset = 0; Offset += BlockMDSize; - if (const auto *FD = dyn_cast<FieldDecl>(Dcl)) { + if (const auto *FD = dyn_cast<FieldDecl>(getDecl())) { if (FD->getParent() == BaseRecord->getDecl()) return CastedBase.atField(BaseRecord->getField(FD)->Offset); @@ -58,7 +58,7 @@ std::optional<Pointer> MemberPointer::toPointer(const Context &Ctx) const { Offset += Ctx.collectBaseOffset(FieldParent, BaseDecl); } else { - const auto *IFD = cast<IndirectFieldDecl>(Dcl); + const auto *IFD = cast<IndirectFieldDecl>(getDecl()); for (const NamedDecl *ND : IFD->chain()) { const FieldDecl *F = cast<FieldDecl>(ND); @@ -77,7 +77,8 @@ std::optional<Pointer> MemberPointer::toPointer(const Context &Ctx) const { } FunctionPointer MemberPointer::toFunctionPointer(const Context &Ctx) const { - return FunctionPointer(Ctx.getProgram().getFunction(cast<FunctionDecl>(Dcl))); + return FunctionPointer( + Ctx.getProgram().getFunction(cast<FunctionDecl>(getDecl()))); } APValue MemberPointer::toAPValue(const ASTContext &ASTCtx) const { @@ -88,8 +89,8 @@ APValue MemberPointer::toAPValue(const ASTContext &ASTCtx) const { if (hasBase()) return Base.toAPValue(ASTCtx); - return APValue(getDecl(), /*IsDerivedMember=*/false, - /*Path=*/{}); + return APValue(getDecl(), /*IsDerivedMember=*/isDerivedMember(), + /*Path=*/ArrayRef(Path, PathLength)); } } // namespace interp diff --git a/clang/lib/AST/ByteCode/MemberPointer.h b/clang/lib/AST/ByteCode/MemberPointer.h index 8dd75cad092c0..8062cdd7c1f78 100644 --- a/clang/lib/AST/ByteCode/MemberPointer.h +++ b/clang/lib/AST/ByteCode/MemberPointer.h @@ -10,10 +10,12 @@ #define LLVM_CLANG_AST_INTERP_MEMBER_POINTER_H #include "Pointer.h" +#include "llvm/ADT/PointerIntPair.h" #include <optional> namespace clang { class ASTContext; +class CXXRecordDecl; namespace interp { class Context; @@ -22,21 +24,33 @@ class FunctionPointer; class MemberPointer final { private: Pointer Base; - const ValueDecl *Dcl = nullptr; + /// The member declaration, and a flag indicating + /// whether the member is a member of some class derived from the class type + /// of the member pointer. + llvm::PointerIntPair<const ValueDecl *, 1, bool> DeclAndIsDerivedMember; + /// The path of base/derived classes from the member declaration's + /// class (exclusive) to the class type of the member pointer (inclusive). + /// This a allocated by the InterpState or the Program. + const CXXRecordDecl **Path = nullptr; int32_t PtrOffset = 0; + uint8_t PathLength = 0; - MemberPointer(Pointer Base, const ValueDecl *Dcl, int32_t PtrOffset) - : Base(Base), Dcl(Dcl), PtrOffset(PtrOffset) {} + MemberPointer(Pointer Base, const ValueDecl *Dcl, int32_t PtrOffset, + uint8_t PathLength = 0, const CXXRecordDecl **Path = nullptr, + bool IsDerived = false) + : Base(Base), DeclAndIsDerivedMember(Dcl, IsDerived), Path(Path), + PtrOffset(PtrOffset), PathLength(PathLength) {} public: MemberPointer() = default; - MemberPointer(Pointer Base, const ValueDecl *Dcl) : Base(Base), Dcl(Dcl) {} + MemberPointer(Pointer Base, const ValueDecl *Dcl) + : Base(Base), DeclAndIsDerivedMember(Dcl) {} MemberPointer(uint32_t Address, const Descriptor *D) { // We only reach this for Address == 0, when creating a null member pointer. assert(Address == 0); } - MemberPointer(const ValueDecl *D) : Dcl(D) { + MemberPointer(const ValueDecl *D) : DeclAndIsDerivedMember(D) { assert((isa<FieldDecl, IndirectFieldDecl, CXXMethodDecl>(D))); } @@ -47,6 +61,24 @@ class MemberPointer final { return 17; } + bool hasDecl() const { return DeclAndIsDerivedMember.getPointer(); } + bool isDerivedMember() const { return DeclAndIsDerivedMember.getInt(); } + const ValueDecl *getDecl() const { + return DeclAndIsDerivedMember.getPointer(); + } + bool hasPath() const { return PathLength != 0; } + unsigned getPathLength() const { return PathLength; } + const CXXRecordDecl *getPathEntry(unsigned Index) const { + return Path[Index]; + } + void takePath(const CXXRecordDecl **NewPath) { + assert(Path != NewPath); + Path = NewPath; + } + + // Pretend we always have a path. + bool singleWord() const { return false; } + std::optional<Pointer> toPointer(const Context &Ctx) const; FunctionPointer toFunctionPointer(const Context &Ctx) const; @@ -63,32 +95,44 @@ class MemberPointer final { return Base.atFieldSub(PtrOffset); } bool isMemberFunctionPointer() const { - return isa_and_nonnull<CXXMethodDecl>(Dcl); + return isa_and_nonnull<CXXMethodDecl>(DeclAndIsDerivedMember.getPointer()); } const CXXMethodDecl *getMemberFunction() const { - return dyn_cast_if_present<CXXMethodDecl>(Dcl); + return dyn_cast_if_present<CXXMethodDecl>( + DeclAndIsDerivedMember.getPointer()); } const FieldDecl *getField() const { - return dyn_cast_if_present<FieldDecl>(Dcl); + return dyn_cast_if_present<FieldDecl>(DeclAndIsDerivedMember.getPointer()); } - bool hasDecl() const { return Dcl; } - const ValueDecl *getDecl() const { return Dcl; } + const CXXRecordDecl *getRecordDecl() const { + if (const FieldDecl *FD = getField()) + return cast<CXXRecordDecl>(FD->getParent()); - MemberPointer atInstanceBase(unsigned Offset) const { + if (const CXXMethodDecl *MD = getMemberFunction()) + return MD->getParent(); + return nullptr; + } + + MemberPointer atInstanceBase(unsigned Offset, uint8_t PathLength = 0, + const CXXRecordDecl **Path = nullptr, + bool NewIsDerived = false) const { if (Base.isZero()) - return MemberPointer(Base, Dcl, Offset); - return MemberPointer(this->Base, Dcl, Offset + PtrOffset); + return MemberPointer(Base, DeclAndIsDerivedMember.getPointer(), Offset, + PathLength, Path, NewIsDerived); + return MemberPointer(this->Base, DeclAndIsDerivedMember.getPointer(), + Offset + PtrOffset, PathLength, Path, NewIsDerived); } MemberPointer takeInstance(Pointer Instance) const { assert(this->Base.isZero()); - return MemberPointer(Instance, this->Dcl, this->PtrOffset); + return MemberPointer(Instance, DeclAndIsDerivedMember.getPointer(), + this->PtrOffset); } APValue toAPValue(const ASTContext &) const; - bool isZero() const { return Base.isZero() && !Dcl; } + bool isZero() const { return Base.isZero() && !hasDecl(); } bool hasBase() const { return !Base.isZero(); } bool isWeak() const { if (const auto *MF = getMemberFunction()) @@ -97,22 +141,33 @@ class MemberPointer final { } void print(llvm::raw_ostream &OS) const { - OS << "MemberPtr(" << Base << " " << (const void *)Dcl << " + " << PtrOffset - << ")"; + OS << "MemberPtr(" << Base << " " << (const void *)getDecl() << " + " + << PtrOffset << ")"; } std::string toDiagnosticString(const ASTContext &Ctx) const { - return toAPValue(Ctx).getAsString(Ctx, Dcl->getType()); + return toAPValue(Ctx).getAsString(Ctx, getDecl()->getType()); } ComparisonCategoryResult compare(const MemberPointer &RHS) const { - if (this->Dcl == RHS.Dcl) + if (this->getDecl() == RHS.getDecl()) { + + if (this->PathLength != RHS.PathLength) + return ComparisonCategoryResult::Unordered; + + if (PathLength != 0 && + std::memcmp(Path, RHS.Path, PathLength * sizeof(CXXRecordD... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/179050 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
