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