r178283 - [ms-cxxabi] Correctly compute the size of member pointers
Reid Kleckner
reid at kleckner.net
Thu Mar 28 13:02:56 PDT 2013
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()) {
+ // 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());
}
Modified: cfe/trunk/test/SemaCXX/member-pointer-ms.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-pointer-ms.cpp?rev=178283&r1=178282&r2=178283&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/member-pointer-ms.cpp (original)
+++ cfe/trunk/test/SemaCXX/member-pointer-ms.cpp Thu Mar 28 15:02:56 2013
@@ -1,14 +1,134 @@
-// RUN: %clang_cc1 -cxx-abi microsoft -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -cxx-abi microsoft -fms-compatibility -fsyntax-only -triple=i386-pc-win32 -verify %s
+// RUN: %clang_cc1 -std=c++11 -cxx-abi microsoft -fms-compatibility -fsyntax-only -triple=x86_64-pc-win32 -verify %s
+//
+// This file should also give no diagnostics when run through cl.exe from MSVS
+// 2012, which supports C++11 and static_assert. It should pass for both 64-bit
+// and 32-bit x86.
+//
+// expected-no-diagnostics
-// Test that we reject pointers to members of incomplete classes (for now)
-struct A; //expected-note{{forward declaration of 'A'}}
-int A::*pai1; //expected-error{{incomplete type 'A'}}
-
-// Test that we don't allow reinterpret_casts from pointers of one size to
-// pointers of a different size.
-struct A {};
-struct B {};
-struct C: A, B {};
+// Test the size of various member pointer combinations:
+// - complete and incomplete
+// - single, multiple, and virtual inheritance (and unspecified for incomplete)
+// - data and function pointers
+// - templated with declared specializations with annotations
+// - template that can be instantiated
-void (A::*paf)();
-void (C::*pcf)() = reinterpret_cast<void (C::*)()>(paf); //expected-error{{cannot reinterpret_cast from member pointer type}}
+// http://llvm.org/PR12070
+struct Foo {
+ typedef int Foo::*FooInt;
+ int f;
+};
+
+enum {
+ kSingleDataSize = 1 * sizeof(int),
+ kSingleFunctionSize = 1 * sizeof(void *),
+ kMultipleDataSize = 1 * sizeof(int),
+ kMultipleFunctionSize = 2 * sizeof(void *),
+ kVirtualDataSize = 2 * sizeof(int),
+ kVirtualFunctionSize = 2 * sizeof(int) + 1 * sizeof(void *),
+ // Unspecified is weird, it's 1 more slot than virtual.
+ kUnspecifiedDataSize = kVirtualDataSize + 1 * sizeof(int),
+ kUnspecifiedFunctionSize = kVirtualFunctionSize + 1 * sizeof(void *),
+};
+
+// incomplete types
+class __single_inheritance IncSingle;
+class __multiple_inheritance IncMultiple;
+class __virtual_inheritance IncVirtual;
+static_assert(sizeof(int IncSingle::*) == kSingleDataSize, "");
+static_assert(sizeof(int IncMultiple::*) == kMultipleDataSize, "");
+static_assert(sizeof(int IncVirtual::*) == kVirtualDataSize, "");
+static_assert(sizeof(void (IncSingle::*)()) == kSingleFunctionSize, "");
+static_assert(sizeof(void (IncMultiple::*)()) == kMultipleFunctionSize, "");
+static_assert(sizeof(void (IncVirtual::*)()) == kVirtualFunctionSize, "");
+
+// An incomplete type with an unspecified inheritance model seems to take one
+// more slot than virtual. It's not clear what it's used for yet.
+class IncUnspecified;
+static_assert(sizeof(int IncUnspecified::*) == kUnspecifiedDataSize, "");
+static_assert(sizeof(void (IncUnspecified::*)()) == kUnspecifiedFunctionSize, "");
+
+// complete types
+struct B1 { };
+struct B2 { };
+struct Single { };
+struct Multiple : B1, B2 { };
+struct Virtual : virtual B1 { };
+static_assert(sizeof(int Single::*) == kSingleDataSize, "");
+static_assert(sizeof(int Multiple::*) == kMultipleDataSize, "");
+static_assert(sizeof(int Virtual::*) == kVirtualDataSize, "");
+static_assert(sizeof(void (Single::*)()) == kSingleFunctionSize, "");
+static_assert(sizeof(void (Multiple::*)()) == kMultipleFunctionSize, "");
+static_assert(sizeof(void (Virtual::*)()) == kVirtualFunctionSize, "");
+
+// Test both declared and defined templates.
+template <typename T> class X;
+template <> class __single_inheritance X<IncSingle>;
+template <> class __multiple_inheritance X<IncMultiple>;
+template <> class __virtual_inheritance X<IncVirtual>;
+// Don't declare X<IncUnspecified>.
+static_assert(sizeof(int X<IncSingle>::*) == kSingleDataSize, "");
+static_assert(sizeof(int X<IncMultiple>::*) == kMultipleDataSize, "");
+static_assert(sizeof(int X<IncVirtual>::*) == kVirtualDataSize, "");
+static_assert(sizeof(int X<IncUnspecified>::*) == kUnspecifiedDataSize, "");
+static_assert(sizeof(void (X<IncSingle>::*)()) == kSingleFunctionSize, "");
+static_assert(sizeof(void (X<IncMultiple>::*)()) == kMultipleFunctionSize, "");
+static_assert(sizeof(void (X<IncVirtual>::*)()) == kVirtualFunctionSize, "");
+static_assert(sizeof(void (X<IncUnspecified>::*)()) == kUnspecifiedFunctionSize, "");
+
+template <typename T>
+struct Y : T { };
+static_assert(sizeof(int Y<Single>::*) == kSingleDataSize, "");
+static_assert(sizeof(int Y<Multiple>::*) == kMultipleDataSize, "");
+static_assert(sizeof(int Y<Virtual>::*) == kVirtualDataSize, "");
+static_assert(sizeof(void (Y<Single>::*)()) == kSingleFunctionSize, "");
+static_assert(sizeof(void (Y<Multiple>::*)()) == kMultipleFunctionSize, "");
+static_assert(sizeof(void (Y<Virtual>::*)()) == kVirtualFunctionSize, "");
+
+struct A { int x; void bar(); };
+struct B : A { virtual void foo(); };
+static_assert(sizeof(int B::*) == kSingleDataSize, "");
+// A non-primary base class uses the multiple inheritance model for member
+// pointers.
+static_assert(sizeof(void (B::*)()) == kMultipleFunctionSize, "");
+
+struct AA { int x; virtual void foo(); };
+struct BB : AA { void bar(); };
+struct CC : BB { virtual void baz(); };
+static_assert(sizeof(void (CC::*)()) == kSingleFunctionSize, "");
+
+// We start out unspecified.
+struct ForwardDecl1;
+struct ForwardDecl2;
+
+// Re-declare to force us to iterate decls when adding attributes.
+struct ForwardDecl1;
+struct ForwardDecl2;
+
+typedef int ForwardDecl1::*MemPtr1;
+typedef int ForwardDecl2::*MemPtr2;
+MemPtr1 variable_forces_sizing;
+
+struct ForwardDecl1 : B {
+ virtual void foo();
+};
+struct ForwardDecl2 : B {
+ virtual void foo();
+};
+
+static_assert(sizeof(variable_forces_sizing) == kUnspecifiedDataSize, "");
+static_assert(sizeof(MemPtr1) == kUnspecifiedDataSize, "");
+// FIXME: Clang fails this assert because it locks in the inheritance model at
+// the point of the typedef instead of the first usage, while MSVC does not.
+//static_assert(sizeof(MemPtr2) == kSingleDataSize, "");
+
+struct MemPtrInBody {
+ typedef int MemPtrInBody::*MemPtr;
+ int a;
+ operator MemPtr() const {
+ return a ? &MemPtrInBody::a : 0;
+ }
+};
+
+static_assert(sizeof(MemPtrInBody::MemPtr) == kSingleDataSize, "");
More information about the cfe-commits
mailing list