c++: Fix FE devirt with diamond inheritance [PR95158]
authorJason Merrill <jason@redhat.com>
Thu, 4 Jun 2020 03:50:06 +0000 (23:50 -0400)
committerJason Merrill <jason@redhat.com>
Thu, 4 Jun 2020 19:11:42 +0000 (15:11 -0400)
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
gcc/cp/class.c
gcc/cp/cp-tree.h
gcc/testsuite/g++.dg/template/virtual5.C [new file with mode: 0644]

index a51ebb5d9e39a1c3d4c9a2e28a89682f998d9796..2b393f96e5bda607bcf9d4846b2a03a478ab784c 100644 (file)
@@ -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.
index 757e010b6b7e4aa36f64fbc429d962860b434ec9..f8e38ec9d8c88c0ba0d53467874ab733fe23dbe5 100644 (file)
@@ -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
index 447d1349117b84471d063a904923a4aebcb69ee4..44cb10cfee5710e99f45f3e0de0fc146c68e999b 100644 (file)
@@ -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 (file)
index 0000000..4d90445
--- /dev/null
@@ -0,0 +1,31 @@
+// PR c++/95158
+// { dg-do run }
+
+class Base {
+    public:
+        virtual void foo()=0;
+};
+
+template <typename T>
+class MiddleA : virtual public Base {
+    public:
+        virtual void foo() {}
+};
+
+class MiddleB : virtual public Base {};
+
+template <typename T>
+class Derived : public MiddleA<T>, public MiddleB {
+    public:
+        void bar()
+        {
+         Derived d;
+         d.foo();
+        }
+};
+
+int main()
+{
+  Derived<void> a;
+  a.bar(); // Instantiate the template
+}