On Mar 28, 2013, at 1:02 PM, Reid Kleckner <[email protected]> wrote: > Author: rnk > Date: Thu Mar 28 15:02:56 2013 > New Revision: 178283 > > URL: http://llvm.org/viewvc/llvm-project?rev=178283&view=rev > Log: > [ms-cxxabi] Correctly compute the size of member pointers > > Summary: > This also relaxes the requirement on Windows that the member pointer > class type be a complete type (http://llvm.org/PR12070). We still ask > for a complete type to instantiate any templates (MSVC does this), but > if that fails we continue as normal, relying on any inheritance > attributes on the declaration. > > Reviewers: rjmccall > > CC: triton, timurrrr, cfe-commits > > Differential Revision: http://llvm-reviews.chandlerc.com/D568 > > Modified: > cfe/trunk/lib/AST/ASTContext.cpp > cfe/trunk/lib/AST/CXXABI.h > cfe/trunk/lib/AST/ItaniumCXXABI.cpp > cfe/trunk/lib/AST/MicrosoftCXXABI.cpp > cfe/trunk/lib/Sema/SemaType.cpp > cfe/trunk/test/SemaCXX/member-pointer-ms.cpp > > Modified: cfe/trunk/lib/AST/ASTContext.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=178283&r1=178282&r2=178283&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/ASTContext.cpp (original) > +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Mar 28 15:02:56 2013 > @@ -1496,10 +1496,7 @@ ASTContext::getTypeInfoImpl(const Type * > } > case Type::MemberPointer: { > const MemberPointerType *MPT = cast<MemberPointerType>(T); > - std::pair<uint64_t, unsigned> PtrDiffInfo = > - getTypeInfo(getPointerDiffType()); > - Width = PtrDiffInfo.first * ABI->getMemberPointerSize(MPT); > - Align = PtrDiffInfo.second; > + llvm::tie(Width, Align) = ABI->getMemberPointerWidthAndAlign(MPT); > break; > } > case Type::Complex: { > > Modified: cfe/trunk/lib/AST/CXXABI.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXABI.h?rev=178283&r1=178282&r2=178283&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/CXXABI.h (original) > +++ cfe/trunk/lib/AST/CXXABI.h Thu Mar 28 15:02:56 2013 > @@ -27,9 +27,9 @@ class CXXABI { > public: > virtual ~CXXABI(); > > - /// Returns the size of a member pointer in multiples of the target > - /// pointer size. > - virtual unsigned getMemberPointerSize(const MemberPointerType *MPT) const > = 0; > + /// Returns the width and alignment of a member pointer in bits. > + virtual std::pair<uint64_t, unsigned> > + getMemberPointerWidthAndAlign(const MemberPointerType *MPT) const = 0; > > /// Returns the default calling convention for C++ methods. > virtual CallingConv getDefaultMethodCallConv(bool isVariadic) const = 0; > > Modified: cfe/trunk/lib/AST/ItaniumCXXABI.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumCXXABI.cpp?rev=178283&r1=178282&r2=178283&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/ItaniumCXXABI.cpp (original) > +++ cfe/trunk/lib/AST/ItaniumCXXABI.cpp Thu Mar 28 15:02:56 2013 > @@ -33,10 +33,15 @@ protected: > public: > ItaniumCXXABI(ASTContext &Ctx) : Context(Ctx) { } > > - unsigned getMemberPointerSize(const MemberPointerType *MPT) const { > - QualType Pointee = MPT->getPointeeType(); > - if (Pointee->isFunctionType()) return 2; > - return 1; > + std::pair<uint64_t, unsigned> > + getMemberPointerWidthAndAlign(const MemberPointerType *MPT) const { > + const TargetInfo &Target = Context.getTargetInfo(); > + TargetInfo::IntType PtrDiff = Target.getPtrDiffType(0); > + uint64_t Width = Target.getTypeWidth(PtrDiff); > + unsigned Align = Target.getTypeAlign(PtrDiff); > + if (MPT->getPointeeType()->isFunctionType()) > + Width = 2 * Width; > + return std::make_pair(Width, Align); > } > > CallingConv getDefaultMethodCallConv(bool isVariadic) const { > > Modified: cfe/trunk/lib/AST/MicrosoftCXXABI.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftCXXABI.cpp?rev=178283&r1=178282&r2=178283&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/MicrosoftCXXABI.cpp (original) > +++ cfe/trunk/lib/AST/MicrosoftCXXABI.cpp Thu Mar 28 15:02:56 2013 > @@ -13,6 +13,7 @@ > //===----------------------------------------------------------------------===// > > #include "CXXABI.h" > +#include "clang/AST/Attr.h" > #include "clang/AST/ASTContext.h" > #include "clang/AST/DeclCXX.h" > #include "clang/AST/RecordLayout.h" > @@ -27,7 +28,8 @@ class MicrosoftCXXABI : public CXXABI { > public: > MicrosoftCXXABI(ASTContext &Ctx) : Context(Ctx) { } > > - unsigned getMemberPointerSize(const MemberPointerType *MPT) const; > + std::pair<uint64_t, unsigned> > + getMemberPointerWidthAndAlign(const MemberPointerType *MPT) const; > > CallingConv getDefaultMethodCallConv(bool isVariadic) const { > if (!isVariadic && Context.getTargetInfo().getTriple().getArch() == > llvm::Triple::x86) > @@ -52,17 +54,77 @@ public: > }; > } > > -unsigned MicrosoftCXXABI::getMemberPointerSize(const MemberPointerType *MPT) > const { > - QualType Pointee = MPT->getPointeeType(); > - CXXRecordDecl *RD = MPT->getClass()->getAsCXXRecordDecl(); > - if (RD->getNumVBases() > 0) { > - if (Pointee->isFunctionType()) > - return 3; > - else > - return 2; > - } else if (RD->getNumBases() > 1 && Pointee->isFunctionType()) > - return 2; > - return 1; > +// getNumBases() seems to only give us the number of direct bases, and not > the > +// total. This function tells us if we inherit from anybody that uses MI, > or if > +// we have a non-primary base class, which uses the multiple inheritance > model. > +static bool usesMultipleInheritanceModel(const CXXRecordDecl *RD) { > + while (RD->getNumBases() > 0) { > + if (RD->getNumBases() > 1) > + return true; > + assert(RD->getNumBases() == 1); > + const CXXRecordDecl *Base = > RD->bases_begin()->getType()->getAsCXXRecordDecl(); > + if (RD->isPolymorphic() && !Base->isPolymorphic()) > + return true; > + RD = Base; > + } > + return false; > +} > + > +std::pair<uint64_t, unsigned> > +MicrosoftCXXABI::getMemberPointerWidthAndAlign(const MemberPointerType *MPT) > const { > + const CXXRecordDecl *RD = MPT->getClass()->getAsCXXRecordDecl(); > + const TargetInfo &Target = Context.getTargetInfo(); > + > + assert(Target.getTriple().getArch() == llvm::Triple::x86 || > + Target.getTriple().getArch() == llvm::Triple::x86_64); > + > + Attr *IA = RD->getAttr<MSInheritanceAttr>(); > + attr::Kind Inheritance; > + if (IA) { > + Inheritance = IA->getKind(); > + } else if (RD->getNumVBases() > 0) { > + Inheritance = attr::VirtualInheritance; > + } else if (MPT->getPointeeType()->isFunctionType() && > + usesMultipleInheritanceModel(RD)) { > + Inheritance = attr::MultipleInheritance; > + } else { > + Inheritance = attr::SingleInheritance; > + } > + > + unsigned PtrSize = Target.getPointerWidth(0); > + unsigned IntSize = Target.getIntWidth(); > + uint64_t Width; > + unsigned Align; > + if (MPT->getPointeeType()->isFunctionType()) { > + // Member function pointers are a struct of a function pointer followed > by a > + // variable number of ints depending on the inheritance model used. The > + // function pointer is a real function if it is non-virtual and a vftable > + // slot thunk if it is virtual. The ints select the object base passed > for > + // the 'this' pointer. > + Align = Target.getPointerAlign(0); > + switch (Inheritance) { > + case attr::SingleInheritance: Width = PtrSize; break; > + case attr::MultipleInheritance: Width = PtrSize + 1 * IntSize; break; > + case attr::VirtualInheritance: Width = PtrSize + 2 * IntSize; break; > + case attr::UnspecifiedInheritance: Width = PtrSize + 3 * IntSize; break; > + default: llvm_unreachable("unknown inheritance model"); > + } > + } else { > + // Data pointers are an aggregate of ints. The first int is an offset > + // followed by vbtable-related offsets. > + Align = Target.getIntAlign(); > + switch (Inheritance) { > + case attr::SingleInheritance: // Same as multiple inheritance. > + case attr::MultipleInheritance: Width = 1 * IntSize; break; > + case attr::VirtualInheritance: Width = 2 * IntSize; break; > + case attr::UnspecifiedInheritance: Width = 3 * IntSize; break; > + default: llvm_unreachable("unknown inheritance model"); > + } > + } > + Width = llvm::RoundUpToAlignment(Width, Align); > + > + // FIXME: Verify that our alignment matches MSVC. > + return std::make_pair(Width, Align); > } > > CXXABI *clang::CreateMicrosoftCXXABI(ASTContext &Ctx) { > > Modified: cfe/trunk/lib/Sema/SemaType.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=178283&r1=178282&r2=178283&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaType.cpp (original) > +++ cfe/trunk/lib/Sema/SemaType.cpp Thu Mar 28 15:02:56 2013 > @@ -1728,13 +1728,29 @@ QualType Sema::BuildMemberPointerType(Qu > // according to the class type, which means that we really need a > // complete type if possible, which means we need to instantiate templates. > // > - // For now, just require a complete type, which will instantiate > - // templates. This will also error if the type is just forward-declared, > - // which is a bug, but it's a bug that saves us from dealing with some > - // complexities at the moment. > - if (Context.getTargetInfo().getCXXABI().isMicrosoft() && > - RequireCompleteType(Loc, Class, diag::err_incomplete_type)) > - return QualType(); > + // If template instantiation fails or the type is just incomplete, we have > to > + // add an extra slot to the member pointer. Yes, this does cause problems > + // when passing pointers between TUs that disagree about the size. > + if (Context.getTargetInfo().getCXXABI().isMicrosoft()) { > + CXXRecordDecl *RD = Class->getAsCXXRecordDecl(); > + if (!RD->hasAttr<MSInheritanceAttr>()) { > + // Lock in the inheritance model on the first use of a member pointer. > + // Otherwise we may disagree about the size at different points in the > TU. > + // FIXME: MSVC picks a model on the first use that needs to know the > size, > + // rather than on the first mention of the type, e.g. typedefs. > + SourceRange DeclRange = RD->getSourceRange(); > + if (RequireCompleteType(Loc, Class, 0) && !RD->isBeingDefined()) {
I missed this, but DeclRange is unused (and should be sunk out of the fast path even if it weren't). John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
