From 0ddb93ce77374004c49cdfbd748ba35867620cf1 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Wed, 3 Jun 2020 23:50:06 -0400 Subject: [PATCH] c++: Fix FE devirt with diamond inheritance [PR95158] This started breaking in GCC 8 because of the fix for PR15272; after that change, we (correctly) remember the lookup from template parsing time that found Base::foo through the non-dependent MiddleB base, and so we overlook the overrider in MiddleA. But given that, the devirtualization condition from the fix for PR59031 is insufficient; we know that d has to be a Derived, and we found Base::foo in Base, but forcing a non-virtual call gets the wrong function. Fixed by removing the PR59031 code that the PR67184 patch moved to build_over_call, and instead looking up the overrider in BINFO_VIRTUALS. gcc/cp/ChangeLog: PR c++/95158 * class.c (lookup_vfn_in_binfo): New. * call.c (build_over_call): Use it. * cp-tree.h (resolves_to_fixed_type_p): Add default argument. (lookup_vfn_in_binfo): Declare. gcc/testsuite/ChangeLog: PR c++/95158 * g++.dg/template/virtual5.C: New test. --- gcc/cp/call.c | 22 +++++++++-------- gcc/cp/class.c | 14 +++++++++++ gcc/cp/cp-tree.h | 3 ++- gcc/testsuite/g++.dg/template/virtual5.C | 31 ++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/virtual5.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index a51ebb5d9e3..2b393f96e5b 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -8704,19 +8704,21 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) return error_mark_node; } - /* Optimize away vtable lookup if we know that this - function can't be overridden. We need to check if - the context and the type where we found fn are the same, - actually FN might be defined in a different class - type because of a using-declaration. In this case, we - do not want to perform a non-virtual call. Note that - resolves_to_fixed_type_p checks CLASSTYPE_FINAL too. */ + /* See if the function member or the whole class type is declared + final and the call can be devirtualized. */ if (DECL_FINAL_P (fn) - || (resolves_to_fixed_type_p (arg, 0) - && same_type_ignoring_top_level_qualifiers_p - (DECL_CONTEXT (fn), BINFO_TYPE (cand->conversion_path)))) + || CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn)))) flags |= LOOKUP_NONVIRTUAL; + /* If we know the dynamic type of the object, look up the final overrider + in the BINFO. */ + if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0 + && resolves_to_fixed_type_p (arg)) + { + fn = lookup_vfn_in_binfo (DECL_VINDEX (fn), cand->conversion_path); + flags |= LOOKUP_NONVIRTUAL; + } + /* [class.mfct.non-static]: If a non-static member function of a class X is called for an object that is not of type X, or of a type derived from X, the behavior is undefined. diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 757e010b6b7..f8e38ec9d8c 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -2445,6 +2445,20 @@ get_vcall_index (tree fn, tree type) gcc_unreachable (); } +/* Given a DECL_VINDEX of a virtual function found in BINFO, return the final + overrider at that index in the vtable. This should only be used when we + know that BINFO is correct for the dynamic type of the object. */ + +tree +lookup_vfn_in_binfo (tree idx, tree binfo) +{ + int ix = tree_to_shwi (idx); + if (TARGET_VTABLE_USES_DESCRIPTORS) + ix /= MAX (TARGET_VTABLE_USES_DESCRIPTORS, 1); + tree virtuals = BINFO_VIRTUALS (binfo); + return TREE_VALUE (chain_index (ix, virtuals)); +} + /* Update an entry in the vtable for BINFO, which is in the hierarchy dominated by T. FN is the old function; VIRTUALS points to the corresponding position in the new BINFO_VIRTUALS list. IX is the index diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 447d1349117..44cb10cfee5 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6401,7 +6401,7 @@ extern tree outermost_open_class (void); extern tree current_nonlambda_class_type (void); extern tree finish_struct (tree, tree); extern void finish_struct_1 (tree); -extern int resolves_to_fixed_type_p (tree, int *); +extern int resolves_to_fixed_type_p (tree, int * = NULL); extern void init_class_processing (void); extern int is_empty_class (tree); extern bool is_really_empty_class (tree, bool); @@ -6415,6 +6415,7 @@ extern void pop_lang_context (void); extern tree instantiate_type (tree, tree, tsubst_flags_t); extern void build_self_reference (void); extern int same_signature_p (const_tree, const_tree); +extern tree lookup_vfn_in_binfo (tree, tree); extern void maybe_add_class_template_decl_list (tree, tree, int); extern void unreverse_member_declarations (tree); extern void invalidate_class_lookup_cache (void); diff --git a/gcc/testsuite/g++.dg/template/virtual5.C b/gcc/testsuite/g++.dg/template/virtual5.C new file mode 100644 index 00000000000..4d9044579ca --- /dev/null +++ b/gcc/testsuite/g++.dg/template/virtual5.C @@ -0,0 +1,31 @@ +// PR c++/95158 +// { dg-do run } + +class Base { + public: + virtual void foo()=0; +}; + +template +class MiddleA : virtual public Base { + public: + virtual void foo() {} +}; + +class MiddleB : virtual public Base {}; + +template +class Derived : public MiddleA, public MiddleB { + public: + void bar() + { + Derived d; + d.foo(); + } +}; + +int main() +{ + Derived a; + a.bar(); // Instantiate the template +} -- 2.30.2