r178283 - [ms-cxxabi] Correctly compute the size of member pointers
John McCall
rjmccall at apple.com
Thu Mar 28 13:25:08 PDT 2013
On Mar 28, 2013, at 1:02 PM, Reid Kleckner <reid at kleckner.net> 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.
More information about the cfe-commits
mailing list