r206077 - [MS-ABI] Update to vtordisp computation
Warren Hunt
whunt at google.com
Fri Apr 11 15:05:29 PDT 2014
Author: whunt
Date: Fri Apr 11 17:05:28 2014
New Revision: 206077
URL: http://llvm.org/viewvc/llvm-project?rev=206077&view=rev
Log:
[MS-ABI] Update to vtordisp computation
A portion of the vtordisp computation that was previously unguarded by a
test for the declaration of user defined constructors/destructors was
erroniously adding vtordisps to things that shouldn't have them. This
patch correctly guards that codepath. In addition, it updates the
comments to make them more clear. Test case is included.
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=206077&r1=206076&r2=206077&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Fri Apr 11 17:05:28 2014
@@ -2655,16 +2655,20 @@ void MicrosoftRecordLayoutBuilder::final
}
}
-static bool
-RequiresVtordisp(const llvm::SmallPtrSet<const CXXRecordDecl *, 2> &HasVtordisp,
- const CXXRecordDecl *RD) {
- if (HasVtordisp.count(RD))
+// Recursively walks the non-virtual bases of a class and determines if any of
+// them are in the bases with overridden methods set.
+static bool RequiresVtordisp(
+ const llvm::SmallPtrSet<const CXXRecordDecl *, 2> &
+ BasesWithOverriddenMethods,
+ const CXXRecordDecl *RD) {
+ if (BasesWithOverriddenMethods.count(RD))
return true;
// If any of a virtual bases non-virtual bases (recursively) requires a
// vtordisp than so does this virtual base.
for (const auto &I : RD->bases())
if (!I.isVirtual() &&
- RequiresVtordisp(HasVtordisp, I.getType()->getAsCXXRecordDecl()))
+ RequiresVtordisp(BasesWithOverriddenMethods,
+ I.getType()->getAsCXXRecordDecl()))
return true;
return false;
}
@@ -2703,38 +2707,38 @@ MicrosoftRecordLayoutBuilder::computeVto
if (bi.second.hasVtorDisp())
HasVtordispSet.insert(bi.first);
}
- // If we define a constructor or destructor and override a function that is
- // defined in a virtual base's vtable, that virtual bases need a vtordisp.
- // Here we collect a list of classes with vtables for which our virtual bases
- // actually live. The virtual bases with this property will require
- // vtordisps. In addition, virtual bases that contain non-virtual bases that
- // define functions we override also require vtordisps, this case is checked
- // explicitly below.
- if (RD->hasUserDeclaredConstructor() || RD->hasUserDeclaredDestructor()) {
- llvm::SmallPtrSet<const CXXMethodDecl *, 8> Work;
- // Seed the working set with our non-destructor virtual methods.
- for (const auto *I : RD->methods())
- if (I->isVirtual() && !isa<CXXDestructorDecl>(I))
- Work.insert(I);
- while (!Work.empty()) {
- const CXXMethodDecl *MD = *Work.begin();
- CXXMethodDecl::method_iterator i = MD->begin_overridden_methods(),
- e = MD->end_overridden_methods();
- if (i == e)
- // If a virtual method has no-overrides it lives in its parent's vtable.
- HasVtordispSet.insert(MD->getParent());
- else
- Work.insert(i, e);
- // We've finished processing this element, remove it from the working set.
- Work.erase(MD);
- }
+ // If we do not have a user declared constructor or destructor then we don't
+ // introduce any additional vtordisps.
+ if (!RD->hasUserDeclaredConstructor() && !RD->hasUserDeclaredDestructor())
+ return HasVtordispSet;
+ // Compute a set of base classes which define methods we override. A virtual
+ // base in this set will require a vtordisp. A virtual base that transitively
+ // contains one of these bases as a non-virtual base will also require a
+ // vtordisp.
+ llvm::SmallPtrSet<const CXXMethodDecl *, 8> Work;
+ llvm::SmallPtrSet<const CXXRecordDecl *, 2> BasesWithOverriddenMethods;
+ // Seed the working set with our non-destructor virtual methods.
+ for (const auto *I : RD->methods())
+ if (I->isVirtual() && !isa<CXXDestructorDecl>(I))
+ Work.insert(I);
+ while (!Work.empty()) {
+ const CXXMethodDecl *MD = *Work.begin();
+ CXXMethodDecl::method_iterator i = MD->begin_overridden_methods(),
+ e = MD->end_overridden_methods();
+ // If a virtual method has no-overrides it lives in its parent's vtable.
+ if (i == e)
+ BasesWithOverriddenMethods.insert(MD->getParent());
+ else
+ Work.insert(i, e);
+ // We've finished processing this element, remove it from the working set.
+ Work.erase(MD);
}
- // Re-check all of our vbases for vtordisp requirements (in case their
- // non-virtual bases have vtordisp requirements).
+ // For each of our virtual bases, check if it is in the set of overridden
+ // bases or if it transitively contains a non-virtual base that is.
for (const auto &I : RD->vbases()) {
const CXXRecordDecl *BaseDecl = I.getType()->getAsCXXRecordDecl();
if (!HasVtordispSet.count(BaseDecl) &&
- RequiresVtordisp(HasVtordispSet, BaseDecl))
+ RequiresVtordisp(BasesWithOverriddenMethods, BaseDecl))
HasVtordispSet.insert(BaseDecl);
}
return HasVtordispSet;
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=206077&r1=206076&r2=206077&view=diff
==============================================================================
--- cfe/trunk/test/Layout/ms-x86-vtordisp.cpp (original)
+++ cfe/trunk/test/Layout/ms-x86-vtordisp.cpp Fri Apr 11 17:05:28 2014
@@ -234,6 +234,9 @@ struct C : virtual B { int c; };
// CHECK-NEXT: 24 | int b
// CHECK-NEXT: | [sizeof=28, align=4
// CHECK-NEXT: | nvsize=8, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
}
namespace pragma_test2 {
@@ -260,6 +263,9 @@ struct C : virtual B { int c; };
// CHECK-NEXT: 32 | int b
// CHECK-NEXT: | [sizeof=36, align=4
// CHECK-NEXT: | nvsize=8, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
}
namespace pragma_test3 {
@@ -284,6 +290,9 @@ struct C : virtual B { int c; };
// CHECK-NEXT: 24 | int b
// CHECK-NEXT: | [sizeof=28, align=4
// CHECK-NEXT: | nvsize=8, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
}
namespace pragma_test4 {
@@ -324,8 +333,54 @@ struct C : virtual B<int> { int c; };
// CHECK-NEXT: 28 | int b
// CHECK-NEXT: | [sizeof=32, align=4
// CHECK-NEXT: | nvsize=8, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
}
+struct GA {
+ virtual void fun() {}
+};
+struct GB: public GA {};
+struct GC: public virtual GA {
+ virtual void fun() {}
+ GC() {}
+};
+struct GD: public virtual GC, public virtual GB {};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct GD
+// CHECK-NEXT: 0 | (GD vbtable pointer)
+// CHECK-NEXT: 4 | (vtordisp for vbase GA)
+// CHECK-NEXT: 8 | struct GA (virtual base)
+// CHECK-NEXT: 8 | (GA vftable pointer)
+// CHECK-NEXT: 12 | struct GC (virtual base)
+// CHECK-NEXT: 12 | (GC vbtable pointer)
+// CHECK-NEXT: 16 | struct GB (virtual base)
+// CHECK-NEXT: 16 | struct GA (primary base)
+// CHECK-NEXT: 16 | (GA vftable pointer)
+// CHECK-NEXT: | [sizeof=20, align=4
+// CHECK-NEXT: | nvsize=4, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT: 0 | struct GD
+// CHECK-X64-NEXT: 0 | (GD vbtable pointer)
+// CHECK-X64-NEXT: 12 | (vtordisp for vbase GA)
+// CHECK-X64-NEXT: 16 | struct GA (virtual base)
+// CHECK-X64-NEXT: 16 | (GA vftable pointer)
+// CHECK-X64-NEXT: 24 | struct GC (virtual base)
+// CHECK-X64-NEXT: 24 | (GC vbtable pointer)
+// CHECK-X64-NEXT: 32 | struct GB (virtual base)
+// CHECK-X64-NEXT: 32 | struct GA (primary base)
+// CHECK-X64-NEXT: 32 | (GA vftable pointer)
+// CHECK-X64-NEXT: | [sizeof=40, align=8
+// CHECK-X64-NEXT: | nvsize=8, nvalign=8]
+
int a[
sizeof(A)+
sizeof(C)+
@@ -335,4 +390,6 @@ sizeof(XC)+
sizeof(pragma_test1::C)+
sizeof(pragma_test2::C)+
sizeof(pragma_test3::C)+
-sizeof(pragma_test4::C)];
+sizeof(pragma_test4::C)+
+sizeof(GD)+
+0];
More information about the cfe-commits
mailing list