From d1307f56875dd2f239ff85c5f86ffbf2694872dc Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Tue, 23 Sep 2014 22:58:15 +0000 Subject: [PATCH] MS ABI: Pure virtual functions don't contribute to vtordisps Usually, overriding a virtual function defined in a virtual base required emission of a vtordisp slot in the record. However no vtordisp is needed if the overriding function is pure; it should be impossible to observe the pure virtual method. This fixes PR21046. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@218340 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/RecordLayoutBuilder.cpp | 27 +++++++++++++-------------- test/Layout/ms-x86-vtordisp.cpp | 26 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index 2753a304c8..7623356f58 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -2181,8 +2181,9 @@ public: FieldOffsets.push_back(FieldOffset); } /// \brief Compute the set of virtual bases for which vtordisps are required. - llvm::SmallPtrSet - computeVtorDispSet(const CXXRecordDecl *RD); + void computeVtorDispSet( + llvm::SmallPtrSetImpl &HasVtorDispSet, + const CXXRecordDecl *RD) const; const ASTContext &Context; /// \brief The size of the record being laid out. CharUnits Size; @@ -2605,14 +2606,14 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { } VtorDispAlignment = std::max(VtorDispAlignment, RequiredAlignment); // Compute the vtordisp set. - llvm::SmallPtrSet HasVtordispSet = - computeVtorDispSet(RD); + llvm::SmallPtrSet HasVtorDispSet; + computeVtorDispSet(HasVtorDispSet, RD); // Iterate through the virtual bases and lay them out. const ASTRecordLayout *PreviousBaseLayout = nullptr; for (const CXXBaseSpecifier &VBase : RD->vbases()) { const CXXRecordDecl *BaseDecl = VBase.getType()->getAsCXXRecordDecl(); const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl); - bool HasVtordisp = HasVtordispSet.count(BaseDecl); + bool HasVtordisp = HasVtorDispSet.count(BaseDecl) > 0; // Insert padding between two bases if the left first one is zero sized or // contains a zero sized subobject and the right is zero sized or one leads // with a zero sized base. The padding between virtual bases is 4 @@ -2671,10 +2672,9 @@ RequiresVtordisp(const llvm::SmallPtrSetImpl & return false; } -llvm::SmallPtrSet -MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) { - llvm::SmallPtrSet HasVtordispSet; - +void MicrosoftRecordLayoutBuilder::computeVtorDispSet( + llvm::SmallPtrSetImpl &HasVtordispSet, + const CXXRecordDecl *RD) const { // /vd2 or #pragma vtordisp(2): Always use vtordisps for virtual bases with // vftables. if (RD->getMSVtorDispMode() == MSVtorDispAttr::ForVFTable) { @@ -2684,7 +2684,7 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) { if (Layout.hasExtendableVFPtr()) HasVtordispSet.insert(BaseDecl); } - return HasVtordispSet; + return; } // If any of our bases need a vtordisp for this type, so do we. Check our @@ -2701,7 +2701,7 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) { // * #pragma vtordisp(0) or the /vd0 flag are in use. if ((!RD->hasUserDeclaredConstructor() && !RD->hasUserDeclaredDestructor()) || RD->getMSVtorDispMode() == MSVtorDispAttr::Never) - return HasVtordispSet; + return; // /vd1 or #pragma vtordisp(1): Try to guess based on whether we think it's // possible for a partially constructed object with virtual base overrides to // escape a non-trivial constructor. @@ -2712,9 +2712,9 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) { // vtordisp. llvm::SmallPtrSet Work; llvm::SmallPtrSet BasesWithOverriddenMethods; - // Seed the working set with our non-destructor virtual methods. + // Seed the working set with our non-destructor, non-pure virtual methods. for (const CXXMethodDecl *MD : RD->methods()) - if (MD->isVirtual() && !isa(MD)) + if (MD->isVirtual() && !isa(MD) && !MD->isPure()) Work.insert(MD); while (!Work.empty()) { const CXXMethodDecl *MD = *Work.begin(); @@ -2736,7 +2736,6 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) { RequiresVtordisp(BasesWithOverriddenMethods, BaseDecl)) HasVtordispSet.insert(BaseDecl); } - return HasVtordispSet; } /// \brief Get or compute information about the layout of the specified record diff --git a/test/Layout/ms-x86-vtordisp.cpp b/test/Layout/ms-x86-vtordisp.cpp index 60779fb197..1b78d9fe84 100644 --- a/test/Layout/ms-x86-vtordisp.cpp +++ b/test/Layout/ms-x86-vtordisp.cpp @@ -416,6 +416,31 @@ struct HC : virtual HB {}; // CHECK-X64-NEXT: | [sizeof=32, align=8 // CHECK-X64-NEXT: | nvsize=8, nvalign=8] +struct IA { + virtual void f(); +}; +struct __declspec(dllexport) IB : virtual IA { + virtual void f() = 0; + IB() {} +}; + +// CHECK: *** Dumping AST Record Layout +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct IB +// CHECK-NEXT: 0 | (IB vbtable pointer) +// CHECK-NEXT: 4 | struct IA (virtual base) +// CHECK-NEXT: 4 | (IA vftable pointer) +// CHECK-NEXT: | [sizeof=8, align=4 +// CHECK-NEXT: | nvsize=4, nvalign=4] +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64: *** Dumping AST Record Layout +// CHECK-X64-NEXT: 0 | struct IB +// CHECK-X64-NEXT: 0 | (IB vbtable pointer) +// CHECK-X64-NEXT: 8 | struct IA (virtual base) +// CHECK-X64-NEXT: 8 | (IA vftable pointer) +// CHECK-X64-NEXT: | [sizeof=16, align=8 +// CHECK-X64-NEXT: | nvsize=8, nvalign=8] + int a[ sizeof(A)+ sizeof(C)+ @@ -428,4 +453,5 @@ sizeof(pragma_test3::C)+ sizeof(pragma_test4::C)+ sizeof(GD)+ sizeof(HC)+ +sizeof(IB)+ 0]; -- 2.40.0