re PR c++/43120 (Virtual inheritance with covariant return type confuses GCC)

	PR c++/43120
	* cp-tree.h (BV_LOST_PRIMARY): New macro.
	* class.c (update_vtable_entry_for_fn): Fix covariant thunk logic.
	Set BV_LOST_PRIMARY.
	(build_vtbl_initializer): Check BV_LOST_PRIMARY.

From-SVN: r162008

8 files changed, 121 insertions, 55 deletions

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 0a3f54a..bdae8fc 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,11 @@
+2010-07-09  Jason Merrill  <jason@redhat.com>
+
+	PR c++/43120
+	* cp-tree.h (BV_LOST_PRIMARY): New macro.
+	* class.c (update_vtable_entry_for_fn): Fix covariant thunk logic.
+	Set BV_LOST_PRIMARY.
+	(build_vtbl_initializer): Check BV_LOST_PRIMARY.
+
 2010-07-08  Jason Merrill  <jason@redhat.com>
 
 	PR c++/43120
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 20b8c12..dfb2cd9 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -2205,6 +2205,40 @@ update_vtable_entry_for_fn (tree t, tree binfo, tree fn, tree* virtuals,
     gcc_assert (DECL_INVALID_OVERRIDER_P (overrider_target) ||
 		!DECL_THUNK_P (fn));
 
+  /* If we need a covariant thunk, then we may need to adjust first_defn.
+     The ABI specifies that the thunks emitted with a function are
+     determined by which bases the function overrides, so we need to be
+     sure that we're using a thunk for some overridden base; even if we
+     know that the necessary this adjustment is zero, there may not be an
+     appropriate zero-this-adjusment thunk for us to use since thunks for
+     overriding virtual bases always use the vcall offset.
+
+     Furthermore, just choosing any base that overrides this function isn't
+     quite right, as this slot won't be used for calls through a type that
+     puts a covariant thunk here.  Calling the function through such a type
+     will use a different slot, and that slot is the one that determines
+     the thunk emitted for that base.
+
+     So, keep looking until we find the base that we're really overriding
+     in this slot: the nearest primary base that doesn't use a covariant
+     thunk in this slot.  */
+  if (overrider_target != overrider_fn)
+    {
+      if (BINFO_TYPE (b) == DECL_CONTEXT (overrider_target))
+	/* We already know that the overrider needs a covariant thunk.  */
+	b = get_primary_binfo (b);
+      for (; ; b = get_primary_binfo (b))
+	{
+	  tree main_binfo = TYPE_BINFO (BINFO_TYPE (b));
+	  tree bv = chain_index (ix, BINFO_VIRTUALS (main_binfo));
+	  if (BINFO_LOST_PRIMARY_P (b))
+	    lost = true;
+	  if (!DECL_THUNK_P (TREE_VALUE (bv)))
+	    break;
+	}
+      first_defn = b;
+    }
+
   /* Assume that we will produce a thunk that convert all the way to
      the final overrider, and not to an intermediate virtual base.  */
   virtual_base = NULL_TREE;
@@ -2229,38 +2263,6 @@ update_vtable_entry_for_fn (tree t, tree binfo, tree fn, tree* virtuals,
 	}
     }
 
-  if (overrider_fn != overrider_target && !virtual_base)
-    {
-      /* The ABI specifies that a covariant thunk includes a mangling
-	 for a this pointer adjustment.  This-adjusting thunks that
-	 override a function from a virtual base have a vcall
-	 adjustment.  When the virtual base in question is a primary
-	 virtual base, we know the adjustments are zero, (and in the
-	 non-covariant case, we would not use the thunk).
-	 Unfortunately we didn't notice this could happen, when
-	 designing the ABI and so never mandated that such a covariant
-	 thunk should be emitted.  Because we must use the ABI mandated
-	 name, we must continue searching from the binfo where we
-	 found the most recent definition of the function, towards the
-	 primary binfo which first introduced the function into the
-	 vtable.  If that enters a virtual base, we must use a vcall
-	 this-adjusting thunk.  Bleah! */
-      tree probe = first_defn;
-
-      while ((probe = get_primary_binfo (probe))
-	     && (unsigned) list_length (BINFO_VIRTUALS (probe)) > ix)
-	if (BINFO_VIRTUAL_P (probe))
-	  virtual_base = probe;
-
-      if (virtual_base)
-	/* OK, first_defn got this function from a (possibly lost) primary
-	   virtual base, so we're going to use the vcall offset for that
-	   primary virtual base.  But the caller is passing a first_defn*,
-	   not a virtual_base*, so the correct delta is the delta between
-	   first_defn* and itself, i.e. zero.  */
-	goto virtual_covariant;
-    }
-
   /* Compute the constant adjustment to the `this' pointer.  The
      `this' pointer, when this function is called, will point at BINFO
      (or one of its primary bases, which are at the same offset).  */
@@ -2275,7 +2277,6 @@ update_vtable_entry_for_fn (tree t, tree binfo, tree fn, tree* virtuals,
        entry in our vtable.  Except possibly in a constructor vtable,
        if we happen to get our primary back.  In that case, the offset
        will be zero, as it will be a primary base.  */
-   virtual_covariant:
     delta = size_zero_node;
   else
     /* The `this' pointer needs to be adjusted from pointing to
@@ -2293,6 +2294,9 @@ update_vtable_entry_for_fn (tree t, tree binfo, tree fn, tree* virtuals,
       = get_vcall_index (overrider_target, BINFO_TYPE (virtual_base));
   else
     BV_VCALL_INDEX (*virtuals) = NULL_TREE;
+
+  if (lost)
+    BV_LOST_PRIMARY (*virtuals) = true;
 }
 
 /* Called from modify_all_vtables via dfs_walk.  */
@@ -7648,7 +7652,7 @@ build_vtbl_initializer (tree binfo,
 			int* non_fn_entries_p,
 			VEC(constructor_elt,gc) **inits)
 {
-  tree v, b;
+  tree v;
   vtbl_init_data vid;
   unsigned ix, jx;
   tree vbinfo;
@@ -7762,20 +7766,8 @@ build_vtbl_initializer (tree binfo,
 	 zero out unused slots in ctor vtables, rather than filling them
 	 with erroneous values (though harmless, apart from relocation
 	 costs).  */
-      for (b = binfo; ; b = get_primary_binfo (b))
-	{
-	  /* We found a defn before a lost primary; go ahead as normal.  */
-	  if (look_for_overrides_here (BINFO_TYPE (b), fn_original))
-	    break;
-
-	  /* The nearest definition is from a lost primary; clear the
-	     slot.  */
-	  if (BINFO_LOST_PRIMARY_P (b))
-	    {
-	      init = size_zero_node;
-	      break;
-	    }
-	}
+      if (BV_LOST_PRIMARY (v))
+	init = size_zero_node;
 
       if (! init)
 	{
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 56e86be..08398aa 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -168,6 +168,9 @@ c-common.h, not after.
 
      The BV_FN is the declaration for the virtual function itself.
 
+     If BV_LOST_PRIMARY is set, it means that this entry is for a lost
+     primary virtual base and can be left null in the vtable.
+
    BINFO_VTABLE
      This is an expression with POINTER_TYPE that gives the value
      to which the vptr should be initialized.  Use get_vtbl_decl_for_binfo
@@ -1767,6 +1770,8 @@ struct GTY((variable_size)) lang_type {
 /* The function to call.  */
 #define BV_FN(NODE) (TREE_VALUE (NODE))
 
+/* Whether or not this entry is for a lost primary virtual base.  */
+#define BV_LOST_PRIMARY(NODE) (TREE_LANG_FLAG_0 (NODE))
 
 /* For FUNCTION_TYPE or METHOD_TYPE, a list of the exceptions that
    this type can raise.  Each TREE_VALUE is a _TYPE.  The TREE_VALUE
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 16123cf..483e9ca 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2010-07-09  Jason Merrill  <jason@redhat.com>
+
+	* g++.dg/abi/covariant6.C: New.
+	* g++.dg/inherit/covariant17.C: Test both bases.
+	* g++.dg/inherit/covariant7.C: Check vtable layout.
+
 2010-07-09  Tom de Vries  <tjvries@xs4all.nl>
 
 	* gcc.dg/debug/dwarf2/pr31230.c: New testcase.
diff --git a/gcc/testsuite/g++.dg/abi/covariant1.C b/gcc/testsuite/g++.dg/abi/covariant1.C
index 42522c1..ae8c5e6 100644
--- a/gcc/testsuite/g++.dg/abi/covariant1.C
+++ b/gcc/testsuite/g++.dg/abi/covariant1.C
@@ -1,8 +1,8 @@
 // { dg-do compile }
 // { dg-options "-w" }
 
-// We don't want to use a covariant thunk to have a virtual
-// primary base
+// If a covariant thunk is overriding a virtual primary base, we have to
+// use the vcall offset even though we know it will be 0.
 
 struct c4 {};
 
diff --git a/gcc/testsuite/g++.dg/abi/covariant6.C b/gcc/testsuite/g++.dg/abi/covariant6.C
new file mode 100644
index 0000000..9dfc5ba
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/covariant6.C
@@ -0,0 +1,34 @@
+struct A
+{
+  virtual A* f();
+};
+
+struct B: virtual A
+{
+  virtual A* f();
+};
+
+struct C: B
+{
+  virtual C* f();
+};
+
+C* C::f() { return 0; }
+
+// When we emit C::f, we should emit both thunks: one for B and one for A.
+// { dg-final { scan-assembler "_ZTch0_v0_n16_N1C1fEv" { target ilp32 } } }
+// { dg-final { scan-assembler "_ZTch0_v0_n32_N1C1fEv" { target lp64 } } }
+// { dg-final { scan-assembler "_ZTcv0_n12_v0_n16_N1C1fEv" { target ilp32 } } }
+// { dg-final { scan-assembler "_ZTcv0_n24_v0_n32_N1C1fEv" { target lp64 } } }
+
+struct D: B
+{
+  virtual void dummy ();
+  virtual D* f();
+};
+
+void D::dummy() { }
+
+// When we emit the D vtable, it should refer to the thunk for B.
+// { dg-final { scan-assembler "_ZTch0_v0_n16_N1D1fEv" { target ilp32 } } }
+// { dg-final { scan-assembler "_ZTch0_v0_n32_N1D1fEv" { target lp64 } } }
diff --git a/gcc/testsuite/g++.dg/inherit/covariant17.C b/gcc/testsuite/g++.dg/inherit/covariant17.C
index 26031d5..b2de15f 100644
--- a/gcc/testsuite/g++.dg/inherit/covariant17.C
+++ b/gcc/testsuite/g++.dg/inherit/covariant17.C
@@ -18,7 +18,7 @@ struct B {
 };
 
 struct C : public virtual B {
-  virtual B *clone() const = 0;
+  virtual C *clone() const = 0;
 };
 
 struct E* ep;
@@ -28,13 +28,16 @@ struct E : public A, public C {
   virtual E *clone() const {
     if (this != ep)
       abort();
-    return new E(*this);
+    return 0;
   }
 };
 
 int main() {
   E *a = new E(123);
-  B *c = a;
-  B *d = c->clone();
+  C *c = a;
+  B *b = a;
+  c->clone();
+  b->clone();
+  delete a;
   return 0;
 }
diff --git a/gcc/testsuite/g++.dg/inherit/covariant7.C b/gcc/testsuite/g++.dg/inherit/covariant7.C
index cbd58bb..4d519ed 100644
--- a/gcc/testsuite/g++.dg/inherit/covariant7.C
+++ b/gcc/testsuite/g++.dg/inherit/covariant7.C
@@ -1,4 +1,6 @@
 // { dg-do compile }
+// { dg-prune-output "direct base" }
+// { dg-options "-fdump-class-hierarchy" }
 
 // Copyright (C) 2002 Free Software Foundation, Inc.
 // Contributed by Nathan Sidwell 27 Dec 2002 <nathan@codesourcery.com>
@@ -27,7 +29,23 @@ struct c4 : virtual c3, virtual c0, virtual c1
   int m;
 };
 
-struct c6 : c0, c3, c4		// { dg-warning "direct base" "" }
+struct c6 : c0, c3, c4
 {
   virtual c1 &f2() volatile;
 };
+
+// f2 appears four times in the c6 vtables:
+// once in c1-in-c3-in-c6 - covariant, virtual base, uses c1 vcall offset and c0 vbase offset
+// { dg-final { scan-tree-dump "24    c6::_ZTcv0_n16_v0_n12_NV2c62f2Ev" "class" { target ilp32 } } }
+// { dg-final { scan-tree-dump "48    c6::_ZTcv0_n32_v0_n24_NV2c62f2Ev" "class" { target lp64 } } }
+// once in c3-in-c6 - non-covariant, non-virtual base, calls f2 directly
+// { dg-final { scan-tree-dump "28    c6::f2" "class" { target ilp32 } } }
+// { dg-final { scan-tree-dump "56    c6::f2" "class" { target lp64 } } }
+// once in c1-in-c3-in-c4-in-c6 - lost primary
+// { dg-final { scan-tree-dump "80    0u" "class" { target ilp32 } } }
+// { dg-final { scan-tree-dump "160   0u" "class" { target lp64 } } }
+// once in c3-in-c4-in-c6 - c3 vcall offset
+// { dg-final { scan-tree-dump "84    c6::_ZTv0_n16_NV2c62f2Ev" "class" { target ilp32 } } }
+// { dg-final { scan-tree-dump "168   c6::_ZTv0_n32_NV2c62f2Ev" "class" { target lp64 } } }
+
+// { dg-final { cleanup-tree-dump "class" } }