r218340 - MS ABI: Pure virtual functions don't contribute to vtordisps
Timur Iskhodzhanov
timurrrr at google.com
Wed Sep 24 06:53:49 PDT 2014
Maybe we should add a vftable test for this case too?
2014-09-24 2:58 GMT+04:00 David Majnemer <david.majnemer at gmail.com>:
> Author: majnemer
> Date: Tue Sep 23 17:58:15 2014
> New Revision: 218340
>
> URL: http://llvm.org/viewvc/llvm-project?rev=218340&view=rev
> Log:
> 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.
>
> Modified:
> cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
> cfe/trunk/test/Layout/ms-x86-vtordisp.cpp
>
> Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=218340&r1=218339&r2=218340&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
> +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Sep 23 17:58:15 2014
> @@ -2181,8 +2181,9 @@ public:
> FieldOffsets.push_back(FieldOffset);
> }
> /// \brief Compute the set of virtual bases for which vtordisps are required.
> - llvm::SmallPtrSet<const CXXRecordDecl *, 2>
> - computeVtorDispSet(const CXXRecordDecl *RD);
> + void computeVtorDispSet(
> + llvm::SmallPtrSetImpl<const CXXRecordDecl *> &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::layou
> }
> VtorDispAlignment = std::max(VtorDispAlignment, RequiredAlignment);
> // Compute the vtordisp set.
> - llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet =
> - computeVtorDispSet(RD);
> + llvm::SmallPtrSet<const CXXRecordDecl *, 2> 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::SmallPtrSet
> return false;
> }
>
> -llvm::SmallPtrSet<const CXXRecordDecl *, 2>
> -MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
> - llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet;
> -
> +void MicrosoftRecordLayoutBuilder::computeVtorDispSet(
> + llvm::SmallPtrSetImpl<const CXXRecordDecl *> &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::computeVto
> 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::computeVto
> // * #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::computeVto
> // vtordisp.
> llvm::SmallPtrSet<const CXXMethodDecl *, 8> Work;
> llvm::SmallPtrSet<const CXXRecordDecl *, 2> 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<CXXDestructorDecl>(MD))
> + if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD) && !MD->isPure())
> Work.insert(MD);
> while (!Work.empty()) {
> const CXXMethodDecl *MD = *Work.begin();
> @@ -2736,7 +2736,6 @@ MicrosoftRecordLayoutBuilder::computeVto
> RequiresVtordisp(BasesWithOverriddenMethods, BaseDecl))
> HasVtordispSet.insert(BaseDecl);
> }
> - return HasVtordispSet;
> }
>
> /// \brief Get or compute information about the layout of the specified record
>
> Modified: cfe/trunk/test/Layout/ms-x86-vtordisp.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-vtordisp.cpp?rev=218340&r1=218339&r2=218340&view=diff
> ==============================================================================
> --- cfe/trunk/test/Layout/ms-x86-vtordisp.cpp (original)
> +++ cfe/trunk/test/Layout/ms-x86-vtordisp.cpp Tue Sep 23 17:58:15 2014
> @@ -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];
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list