[cfe-commits] r97306 - in /cfe/trunk: lib/CodeGen/CGVtable.cpp test/CodeGenCXX/vtable-layout.cpp

[Anders Carlsson]

Author: andersca
Date: Sat Feb 27 00:38:03 2010
New Revision: 97306

URL: http://llvm.org/viewvc/llvm-project?rev=97306&view=rev
Log:
Fix another vtable layout bug; we weren't looking hard enough for overriden functions when determining if an overrider will ever be used.

Modified:
    cfe/trunk/lib/CodeGen/CGVtable.cpp
    cfe/trunk/test/CodeGenCXX/vtable-layout.cpp

Modified: cfe/trunk/lib/CodeGen/CGVtable.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVtable.cpp?rev=97306&r1=97305&r2=97306&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVtable.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVtable.cpp Sat Feb 27 00:38:03 2010
@@ -1247,12 +1247,12 @@
   void dumpLayout(llvm::raw_ostream&);
 };
 
-/// OverridesMethodInPrimaryBase - Checks whether whether this virtual member 
-/// function overrides a member function in a direct or indirect primary base.
+/// OverridesMethodInBases - Checks whether whether this virtual member 
+/// function overrides a member function in any of the given bases.
 /// Returns the overridden member function, or null if none was found.
 static const CXXMethodDecl * 
-OverridesMethodInPrimaryBase(const CXXMethodDecl *MD,
-                             VtableBuilder::PrimaryBasesSetTy &PrimaryBases) {
+OverridesMethodInBases(const CXXMethodDecl *MD,
+                       VtableBuilder::PrimaryBasesSetTy &Bases) {
   for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
        E = MD->end_overridden_methods(); I != E; ++I) {
     const CXXMethodDecl *OverriddenMD = *I;
@@ -1260,7 +1260,7 @@
     assert(OverriddenMD->isCanonicalDecl() &&
            "Should have the canonical decl of the overridden RD!");
     
-    if (PrimaryBases.count(OverriddenRD))
+    if (Bases.count(OverriddenRD))
       return OverriddenMD;
   }
       
@@ -1352,6 +1352,38 @@
   }
 }
 
+/// OverridesIndirectMethodInBase - Return whether the given member function
+/// overrides any methods in the set of given bases. 
+/// Unlike OverridesMethodInBase, this checks "overriders of overriders".
+/// For example, if we have:
+///
+/// struct A { virtual void f(); }
+/// struct B : A { virtual void f(); }
+/// struct C : B { virtual void f(); }
+///
+/// OverridesIndirectMethodInBase will return true if given C::f as the method 
+/// and { A } as the set of  bases.
+static bool
+OverridesIndirectMethodInBases(const CXXMethodDecl *MD,
+                               VtableBuilder::PrimaryBasesSetTy &Bases) {
+  for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
+       E = MD->end_overridden_methods(); I != E; ++I) {
+    const CXXMethodDecl *OverriddenMD = *I;
+    const CXXRecordDecl *OverriddenRD = OverriddenMD->getParent();
+    assert(OverriddenMD->isCanonicalDecl() &&
+           "Should have the canonical decl of the overridden RD!");
+    
+    if (Bases.count(OverriddenRD))
+      return true;
+    
+    // Check "indirect overriders".
+    if (OverridesIndirectMethodInBases(OverriddenMD, Bases))
+      return true;
+  }
+   
+  return false;
+}
+
 bool 
 VtableBuilder::IsOverriderUsed(BaseSubobject Base, 
                                BaseSubobject FirstBaseInPrimaryBaseChain,
@@ -1406,7 +1438,7 @@
   
   // If the final overrider is an override of one of the primary bases,
   // then we know that it will be used.
-  return OverridesMethodInPrimaryBase(Overrider.Method, PrimaryBases);
+  return OverridesIndirectMethodInBases(Overrider.Method, PrimaryBases);
 }
 
 void 
@@ -1457,7 +1489,7 @@
     // base. If this is the case, and the return type doesn't require adjustment
     // then we can just use the member function from the primary base.
     if (const CXXMethodDecl *OverriddenMD = 
-        OverridesMethodInPrimaryBase(MD, PrimaryBases)) {
+          OverridesMethodInBases(MD, PrimaryBases)) {
       if (ComputeReturnAdjustmentBaseOffset(Context, MD, 
                                             OverriddenMD).isEmpty())
         continue;
@@ -2959,7 +2991,7 @@
 
     // Check if this method overrides a method in the primary base.
     if (const CXXMethodDecl *OverriddenMD = 
-          OverridesMethodInPrimaryBase(MD, PrimaryBases)) {
+          OverridesMethodInBases(MD, PrimaryBases)) {
       // Check if converting from the return type of the method to the 
       // return type of the overridden method requires conversion.
       if (ComputeReturnAdjustmentBaseOffset(CGM.getContext(), MD, 

Modified: cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-layout.cpp?rev=97306&r1=97305&r2=97306&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/vtable-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/vtable-layout.cpp Sat Feb 27 00:38:03 2010
@@ -609,3 +609,39 @@
 void D::f() { } 
 
 }
+
+namespace Test17 {
+
+// Test that we don't mark E::f in the C-in-E vtable as unused.
+struct A { virtual void f(); };
+struct B : virtual A { virtual void f(); };
+struct C : virtual A { virtual void f(); };
+struct D : virtual B, virtual C { virtual void f(); };
+
+// CHECK:      Vtable for 'Test17::E' (13 entries).
+// CHECK-NEXT:    0 | vbase_offset (0)
+// CHECK-NEXT:    1 | vbase_offset (8)
+// CHECK-NEXT:    2 | vbase_offset (0)
+// CHECK-NEXT:    3 | vbase_offset (0)
+// CHECK-NEXT:    4 | vcall_offset (0)
+// CHECK-NEXT:    5 | offset_to_top (0)
+// CHECK-NEXT:    6 | Test17::E RTTI
+// CHECK-NEXT:        -- (Test17::A, 0) vtable address --
+// CHECK-NEXT:        -- (Test17::B, 0) vtable address --
+// CHECK-NEXT:        -- (Test17::D, 0) vtable address --
+// CHECK-NEXT:        -- (Test17::E, 0) vtable address --
+// CHECK-NEXT:    7 | void Test17::E::f()
+// CHECK-NEXT:    8 | vbase_offset (-8)
+// CHECK-NEXT:    9 | vcall_offset (-8)
+// CHECK-NEXT:   10 | offset_to_top (-8)
+// CHECK-NEXT:   11 | Test17::E RTTI
+// CHECK-NEXT:        -- (Test17::A, 8) vtable address --
+// CHECK-NEXT:        -- (Test17::C, 8) vtable address --
+// CHECK-NEXT:   12 | void Test17::E::f()
+// CHECK-NEXT:        [this adjustment: 0 non-virtual, -24 vcall offset offset]
+class E : virtual D {
+  virtual void f();  
+};
+void E::f() {}
+
+}