[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

llvm-svn: 178283

5 files changed, 110 insertions, 30 deletions

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index eee7ed2fb88..6ac18a8d88e 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1496,10 +1496,7 @@ ASTContext::getTypeInfoImpl(const Type *T) const {
   }
   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: {
diff --git a/clang/lib/AST/CXXABI.h b/clang/lib/AST/CXXABI.h
index 0d9c869d87c..6d67d9a12b5 100644
--- a/clang/lib/AST/CXXABI.h
+++ b/clang/lib/AST/CXXABI.h
@@ -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;
diff --git a/clang/lib/AST/ItaniumCXXABI.cpp b/clang/lib/AST/ItaniumCXXABI.cpp
index 7973177ad21..894eb3bff5f 100644
--- a/clang/lib/AST/ItaniumCXXABI.cpp
+++ b/clang/lib/AST/ItaniumCXXABI.cpp
@@ -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 {
diff --git a/clang/lib/AST/MicrosoftCXXABI.cpp b/clang/lib/AST/MicrosoftCXXABI.cpp
index 51308ea0c0f..2f41cec7180 100644
--- a/clang/lib/AST/MicrosoftCXXABI.cpp
+++ b/clang/lib/AST/MicrosoftCXXABI.cpp
@@ -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) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 2bf40235485..bf289074814 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -1728,13 +1728,29 @@ QualType Sema::BuildMemberPointerType(QualType T, QualType Class,
   // 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()) {
+        // We know it doesn't have an attribute and it's incomplete, so use the
+        // unspecified inheritance model.  If we're in the record body, we can
+        // figure out the inheritance model.
+        for (CXXRecordDecl::redecl_iterator I = RD->redecls_begin(),
+             E = RD->redecls_end(); I != E; ++I) {
+          I->addAttr(::new (Context) UnspecifiedInheritanceAttr(
+              RD->getSourceRange(), Context));
+        }
+      }
+    }
+  }
 
   return Context.getMemberPointerType(T, Class.getTypePtr());
 }