From 02dea3ffc6e0f26963db8255ea003d0ede0918cc Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Fri, 9 Jul 2010 15:36:19 -0400 Subject: [PATCH] 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 --- gcc/cp/ChangeLog | 8 +++ gcc/cp/class.c | 88 ++++++++++++++---------------- gcc/cp/cp-tree.h | 5 ++ gcc/testsuite/ChangeLog | 6 ++ gcc/testsuite/g++.dg/abi/covariant1.C | 4 +- gcc/testsuite/g++.dg/abi/covariant6.C | 34 ++++++++++++ gcc/testsuite/g++.dg/inherit/covariant17.C | 11 ++-- gcc/testsuite/g++.dg/inherit/covariant7.C | 20 ++++++- 8 files changed, 121 insertions(+), 55 deletions(-) create mode 100644 gcc/testsuite/g++.dg/abi/covariant6.C 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 + + 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 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 + + * 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 * 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 @@ -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" } } -- 2.7.4