From 343a26b56dfef51cbd16561220a5c307a43e23db Mon Sep 17 00:00:00 2001
From: Anders Carlsson <andersca@mac.com>
Date: Sun, 11 Apr 2010 20:04:11 +0000
Subject: [PATCH] Fix a bug where we were adding too many vcall offsets in some
 cases.

llvm-svn: 100985
---
 clang/lib/CodeGen/CGVTables.cpp         | 27 +++-------
 clang/test/CodeGenCXX/vtable-layout.cpp | 68 +++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 20 deletions(-)

diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 26ae777c58b3..0d9c3ad4aeb2 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -982,30 +982,17 @@ void VCallAndVBaseOffsetBuilder::AddVCallOffsets(BaseSubobject Base,
   const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase();
 
   // Handle the primary base first.
-  if (PrimaryBase) {
-    uint64_t PrimaryBaseOffset;
-    
+  // We only want to add vcall offsets if the base is non-virtual; a virtual
+  // primary base will have its vcall and vbase offsets emitted already.
+  if (PrimaryBase && !Layout.getPrimaryBaseWasVirtual()) {
     // Get the base offset of the primary base.
-    if (Layout.getPrimaryBaseWasVirtual()) {
-      assert(Layout.getVBaseClassOffset(PrimaryBase) == 0 &&
-             "Primary vbase should have a zero offset!");
-      
-      const ASTRecordLayout &MostDerivedClassLayout =
-        Context.getASTRecordLayout(MostDerivedClass);
-      
-      PrimaryBaseOffset = 
-        MostDerivedClassLayout.getVBaseClassOffset(PrimaryBase);
-    } else {
-      assert(Layout.getBaseClassOffset(PrimaryBase) == 0 &&
-             "Primary base should have a zero offset!");
+    assert(Layout.getBaseClassOffset(PrimaryBase) == 0 &&
+           "Primary base should have a zero offset!");
 
-      PrimaryBaseOffset = Base.getBaseOffset();
-    }
-    
-    AddVCallOffsets(BaseSubobject(PrimaryBase, PrimaryBaseOffset),
+    AddVCallOffsets(BaseSubobject(PrimaryBase, Base.getBaseOffset()),
                     VBaseOffset);
   }
-
+  
   // Add the vcall offsets.
   for (CXXRecordDecl::method_iterator I = RD->method_begin(),
        E = RD->method_end(); I != E; ++I) {
diff --git a/clang/test/CodeGenCXX/vtable-layout.cpp b/clang/test/CodeGenCXX/vtable-layout.cpp
index 243d373d3a2d..a8c767ede082 100644
--- a/clang/test/CodeGenCXX/vtable-layout.cpp
+++ b/clang/test/CodeGenCXX/vtable-layout.cpp
@@ -1387,3 +1387,71 @@ struct E : C, virtual D {
 void E::f() { }
 
 }
+
+namespace Test33 {
+
+// Test that we don't emit too many vcall offsets in 'Test32::F'.
+
+struct A {
+  virtual void a();
+};
+
+struct B {
+  virtual void b();
+};
+
+struct C : virtual A, virtual B {
+  virtual void c();
+};
+
+struct D : virtual C { };
+
+struct E : A, D { 
+  virtual void e();
+};
+
+// CHECK:      Vtable for 'Test33::F' (30 entries).
+// CHECK-NEXT:    0 | vbase_offset (24)
+// CHECK-NEXT:    1 | vbase_offset (16)
+// CHECK-NEXT:    2 | vbase_offset (16)
+// CHECK-NEXT:    3 | vbase_offset (8)
+// CHECK-NEXT:    4 | offset_to_top (0)
+// CHECK-NEXT:    5 | Test33::F RTTI
+// CHECK-NEXT:        -- (Test33::A, 0) vtable address --
+// CHECK-NEXT:        -- (Test33::F, 0) vtable address --
+// CHECK-NEXT:    6 | void Test33::A::a()
+// CHECK-NEXT:    7 | void Test33::F::f()
+// CHECK-NEXT:    8 | vcall_offset (0)
+// CHECK-NEXT:    9 | vcall_offset (0)
+// CHECK-NEXT:   10 | vbase_offset (16)
+// CHECK-NEXT:   11 | vbase_offset (8)
+// CHECK-NEXT:   12 | vbase_offset (8)
+// CHECK-NEXT:   13 | offset_to_top (-8)
+// CHECK-NEXT:   14 | Test33::F RTTI
+// CHECK-NEXT:        -- (Test33::A, 8) vtable address --
+// CHECK-NEXT:        -- (Test33::E, 8) vtable address --
+// CHECK-NEXT:   15 | void Test33::A::a()
+// CHECK-NEXT:   16 | void Test33::E::e()
+// CHECK-NEXT:   17 | vbase_offset (0)
+// CHECK-NEXT:   18 | vcall_offset (0)
+// CHECK-NEXT:   19 | vbase_offset (8)
+// CHECK-NEXT:   20 | vbase_offset (0)
+// CHECK-NEXT:   21 | vcall_offset (0)
+// CHECK-NEXT:   22 | offset_to_top (-16)
+// CHECK-NEXT:   23 | Test33::F RTTI
+// CHECK-NEXT:        -- (Test33::A, 16) vtable address --
+// CHECK-NEXT:        -- (Test33::C, 16) vtable address --
+// CHECK-NEXT:        -- (Test33::D, 16) vtable address --
+// CHECK-NEXT:   24 | void Test33::A::a()
+// CHECK-NEXT:   25 | void Test33::C::c()
+// CHECK-NEXT:   26 | vcall_offset (0)
+// CHECK-NEXT:   27 | offset_to_top (-24)
+// CHECK-NEXT:   28 | Test33::F RTTI
+// CHECK-NEXT:        -- (Test33::B, 24) vtable address --
+// CHECK-NEXT:   29 | void Test33::B::b()
+struct F : virtual E, A {
+  virtual void f();
+};
+void F::f() { }
+
+}