Fix verifier ICE on wrong comdat local flag [PR93347]

gcc/ChangeLog:

2020-03-20  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/93347
	* cgraph.c (symbol_table::create_edge): Update calls_comdat_local flag.
	(cgraph_edge::redirect_callee): Move here; likewise.
	(cgraph_node::remove_callees): Update calls_comdat_local flag.
	(cgraph_node::verify_node): Verify that calls_comdat_local flag match
	reality.
	(cgraph_node::check_calls_comdat_local_p): New member function.
	* cgraph.h (cgraph_node::check_calls_comdat_local_p): Declare.
	(cgraph_edge::redirect_callee): Move offline.
	* ipa-fnsummary.c (compute_fn_summary): Do not compute
	calls_comdat_local flag here.
	* ipa-inline-transform.c (inline_call): Fix updating of
	calls_comdat_local flag.
	* ipa-split.c (split_function): Use true instead of 1 to set the flag.
	* symtab.c (symtab_node::add_to_same_comdat_group): Update
	calls_comdat_local flag.

gcc/testsuite/ChangeLog:

2020-03-20  Jan Hubicka  <hubicka@ucw.cz>

	* g++.dg/torture/pr93347.C: New test.

9 files changed, 407 insertions, 30 deletions

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 967e454..7f00a13 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,22 @@
+2020-03-20  Jan Hubicka  <hubicka@ucw.cz>
+
+	PR ipa/93347
+	* cgraph.c (symbol_table::create_edge): Update calls_comdat_local flag.
+	(cgraph_edge::redirect_callee): Move here; likewise.
+	(cgraph_node::remove_callees): Update calls_comdat_local flag.
+	(cgraph_node::verify_node): Verify that calls_comdat_local flag match
+	reality.
+	(cgraph_node::check_calls_comdat_local_p): New member function.
+	* cgraph.h (cgraph_node::check_calls_comdat_local_p): Declare.
+	(cgraph_edge::redirect_callee): Move offline.
+	* ipa-fnsummary.c (compute_fn_summary): Do not compute
+	calls_comdat_local flag here.
+	* ipa-inline-transform.c (inline_call): Fix updating of
+	calls_comdat_local flag.
+	* ipa-split.c (split_function): Use true instead of 1 to set the flag.
+	* symtab.c (symtab_node::add_to_same_comdat_group): Update
+	calls_comdat_local flag.
+
 2020-03-20  Richard Biener  <rguenther@suse.de>
 
 	* tree-vect-slp.c (vect_analyze_slp_instance): Dump SLP tree
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index b41dea1..6b780f8 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -557,7 +557,8 @@ cgraph_node::get_create (tree decl)
 }
 
 /* Mark ALIAS as an alias to DECL.  DECL_NODE is cgraph node representing
-   the function body is associated with (not necessarily cgraph_node (DECL).  */
+   the function body is associated with
+   (not necessarily cgraph_node (DECL)).  */
 
 cgraph_node *
 cgraph_node::create_alias (tree alias, tree target)
@@ -914,6 +915,10 @@ symbol_table::create_edge (cgraph_node *caller, cgraph_node *callee,
   else
     edge->in_polymorphic_cdtor = caller->thunk.thunk_p;
 
+  if (callee && symtab->state != LTO_STREAMING
+      && edge->callee->comdat_local_p ())
+    edge->caller->calls_comdat_local = true;
+
   return edge;
 }
 
@@ -1341,6 +1346,34 @@ cgraph_edge::make_direct (cgraph_edge *edge, cgraph_node *callee)
   return edge;
 }
 
+/* Redirect callee of the edge to N.  The function does not update underlying
+   call expression.  */
+
+void
+cgraph_edge::redirect_callee (cgraph_node *n)
+{
+  bool loc = callee->comdat_local_p ();
+  /* Remove from callers list of the current callee.  */
+  remove_callee ();
+
+  /* Insert to callers list of the new callee.  */
+  set_callee (n);
+
+  if (!inline_failed)
+    return;
+  if (!loc && n->comdat_local_p ())
+    {
+      cgraph_node *to = caller->inlined_to ? caller->inlined_to : caller;
+      to->calls_comdat_local = true;
+    }
+  else if (loc && !n->comdat_local_p ())
+    {
+      cgraph_node *to = caller->inlined_to ? caller->inlined_to : caller;
+      gcc_checking_assert (to->calls_comdat_local);
+      to->calls_comdat_local = to->check_calls_comdat_local_p ();
+    }
+}
+
 /* If necessary, change the function declaration in the call statement
    associated with E so that it corresponds to the edge callee.  Speculations
    can be resolved in the process and EDGE can be removed and deallocated.
@@ -1674,6 +1707,8 @@ cgraph_node::remove_callees (void)
 {
   cgraph_edge *e, *f;
 
+  calls_comdat_local = false;
+
   /* It is sufficient to remove the edges from the lists of callers of
      the callees.  The callee list of the node can be zapped with one
      assignment.  */
@@ -3369,10 +3404,18 @@ cgraph_node::verify_node (void)
       error ("inline clone is forced to output");
       error_found = true;
     }
-  if (calls_comdat_local && !same_comdat_group)
+  if (symtab->state != LTO_STREAMING)
     {
-      error ("calls_comdat_local is set outside of a comdat group");
-      error_found = true;
+      if (calls_comdat_local && !same_comdat_group)
+	{
+	  error ("calls_comdat_local is set outside of a comdat group");
+	  error_found = true;
+	}
+      if (!inlined_to && calls_comdat_local != check_calls_comdat_local_p ())
+	{
+	  error ("invalid calls_comdat_local flag");
+	  error_found = true;
+	}
     }
   if (DECL_IS_MALLOC (decl)
       && !POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (decl))))
@@ -4044,6 +4087,19 @@ cgraph_edge::num_speculative_call_targets_p (void)
   return indirect_info ? indirect_info->num_speculative_call_targets : 0;
 }
 
+/* Check if function calls comdat local.  This is used to recompute
+   calls_comdat_local flag after function transformations.  */
+bool
+cgraph_node::check_calls_comdat_local_p ()
+{
+  for (cgraph_edge *e = callees; e; e = e->next_callee)
+    if (e->inline_failed
+	? e->callee->comdat_local_p ()
+	: e->callee->check_calls_comdat_local_p ())
+      return true;
+  return false;
+}
+
 /* A stashed copy of "symtab" for use by selftest::symbol_table_test.
    This needs to be a global so that it can be a GC root, and thus
    prevent the stashed copy from being garbage-collected if the GC runs
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index aa4cdc9..43de3b4 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1326,6 +1326,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node
   /* Return true if this node represents a former, i.e. an expanded, thunk.  */
   inline bool former_thunk_p (void);
 
+  /* Check if function calls comdat local.  This is used to recompute
+     calls_comdat_local flag after function transformations.  */
+  bool check_calls_comdat_local_p ();
+
   /* Return true if function should be optimized for size.  */
   bool optimize_for_size_p (void);
 
@@ -3298,19 +3302,6 @@ cgraph_edge::set_callee (cgraph_node *n)
   callee = n;
 }
 
-/* Redirect callee of the edge to N.  The function does not update underlying
-   call expression.  */
-
-inline void
-cgraph_edge::redirect_callee (cgraph_node *n)
-{
-  /* Remove from callers list of the current callee.  */
-  remove_callee ();
-
-  /* Insert to callers list of the new callee.  */
-  set_callee (n);
-}
-
 /* Return true when the edge represents a direct recursion.  */
 
 inline bool
diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index 059ea74..b411bc4 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -2944,10 +2944,6 @@ compute_fn_summary (struct cgraph_node *node, bool early)
        analyze_function_body (node, early);
        pop_cfun ();
      }
-  for (e = node->callees; e; e = e->next_callee)
-    if (e->callee->comdat_local_p ())
-      break;
-  node->calls_comdat_local = (e != NULL);
 
   /* Inlining characteristics are maintained by the cgraph_mark_inline.  */
   size_info->size = size_info->self_size;
diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index fa5015d..eed992d 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -504,14 +504,7 @@ inline_call (struct cgraph_edge *e, bool update_original,
   if (callee->calls_comdat_local)
     to->calls_comdat_local = true;
   else if (to->calls_comdat_local && comdat_local)
-    {
-      struct cgraph_edge *se = to->callees;
-      for (; se; se = se->next_callee)
-	if (se->inline_failed && se->callee->comdat_local_p ())
-	  break;
-      if (se == NULL)
-	to->calls_comdat_local = false;
-    }
+    to->calls_comdat_local = to->check_calls_comdat_local_p ();
 
   /* FIXME: This assert suffers from roundoff errors, disable it for GCC 5
      and revisit it after conversion to sreals in GCC 6.
diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c
index 87a0989..973e72c 100644
--- a/gcc/ipa-split.c
+++ b/gcc/ipa-split.c
@@ -1363,7 +1363,7 @@ split_function (basic_block return_bb, class split_point *split_point,
     {
       /* TODO: call is versionable if we make sure that all
 	 callers are inside of a comdat group.  */
-      cur_node->calls_comdat_local = 1;
+      cur_node->calls_comdat_local = true;
       node->add_to_same_comdat_group (cur_node);
     }
 
diff --git a/gcc/symtab.c b/gcc/symtab.c
index a879c09..3022acf 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -473,6 +473,17 @@ symtab_node::add_to_same_comdat_group (symtab_node *old_node)
 	;
       n->same_comdat_group = this;
     }
+
+  cgraph_node *n;
+  if (comdat_local_p ()
+      && (n = dyn_cast <cgraph_node *> (this)) != NULL)
+    {
+      for (cgraph_edge *e = n->callers; e; e = e->next_caller)
+	if (e->caller->inlined_to)
+	  e->caller->inlined_to->calls_comdat_local = true;
+	else
+	  e->caller->calls_comdat_local = true;
+    }
 }
 
 /* Dissolve the same_comdat_group list in which NODE resides.  */
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 76b93b5..50c42e2b 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-20  Jan Hubicka  <hubicka@ucw.cz>
+
+	PR ipa/93347
+	* g++.dg/torture/pr93347.C: New test.
+
 2020-03-20  Patrick Palka  <ppalka@redhat.com>
 
 	PR c++/69694
diff --git a/gcc/testsuite/g++.dg/torture/pr93347.C b/gcc/testsuite/g++.dg/torture/pr93347.C
new file mode 100644
index 0000000..3b5cc26
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr93347.C
@@ -0,0 +1,306 @@
+// { dg-additional-options "--param early-inlining-insns=3 --param ipa-cp-eval-threshold=100" }
+
+namespace Test1 {
+  struct A {
+    virtual int f() final;
+  };
+
+  // CHECK-LABEL: define i32 @_ZN5Test11fEPNS_1AE
+  int f(A *a) {
+    // CHECK: call i32 @_ZN5Test11A1fEv
+    return a->f();
+  }
+}
+
+namespace Test2 {
+  struct A final {
+    virtual int f();
+  };
+
+  // CHECK-LABEL: define i32 @_ZN5Test21fEPNS_1AE
+  int f(A *a) {
+    // CHECK: call i32 @_ZN5Test21A1fEv
+    return a->f();
+  }
+}
+
+namespace Test2a {
+  struct A {
+    virtual ~A() final {}
+    virtual int f();
+  };
+
+  // CHECK-LABEL: define i32 @_ZN6Test2a1fEPNS_1AE
+  int f(A *a) {
+    // CHECK: call i32 @_ZN6Test2a1A1fEv
+    return a->f();
+  }
+}
+
+
+namespace Test3 {
+  struct A {
+    virtual int f();  };
+
+  struct B final : A { };
+
+  // CHECK-LABEL: define i32 @_ZN5Test31fEPNS_1BE
+  int f(B *b) {
+    // CHECK: call i32 @_ZN5Test31A1fEv
+    return b->f();
+  }
+
+  // CHECK-LABEL: define i32 @_ZN5Test31fERNS_1BE
+  int f(B &b) {
+    // CHECK: call i32 @_ZN5Test31A1fEv
+    return b.f();
+  }
+
+  // CHECK-LABEL: define i32 @_ZN5Test31fEPv
+  int f(void *v) {
+    // CHECK: call i32 @_ZN5Test31A1fEv
+    return static_cast<B*>(v)->f();
+  }
+}
+
+namespace Test4 {
+  struct A {
+    virtual void f();
+    virtual int operator-();
+  };
+
+  struct B final : A {
+    virtual void f();
+    virtual int operator-();
+  };
+
+  // CHECK-LABEL: define void @_ZN5Test41fEPNS_1BE
+  void f(B* d) {
+    // CHECK: call void @_ZN5Test41B1fEv
+    static_cast<A*>(d)->f();
+    // CHECK: call i32 @_ZN5Test41BngEv
+    -static_cast<A&>(*d);
+  }
+}
+
+namespace Test5 {
+  struct A {
+    virtual void f();
+    virtual int operator-();
+  };
+
+  struct B : A {
+    virtual void f();
+    virtual int operator-();
+  };
+
+  struct C final : B {
+  };
+
+  // CHECK-LABEL: define void @_ZN5Test51fEPNS_1CE
+  void f(C* d) {
+    // FIXME: It should be possible to devirtualize this case, but that is
+    // not implemented yet.
+    // CHECK: getelementptr
+    // CHECK-NEXT: %[[FUNC:.*]] = load
+    // CHECK-NEXT: call void %[[FUNC]]
+    static_cast<A*>(d)->f();
+  }
+  // CHECK-LABEL: define void @_ZN5Test53fopEPNS_1CE
+  void fop(C* d) {
+    // FIXME: It should be possible to devirtualize this case, but that is
+    // not implemented yet.
+    // CHECK: getelementptr
+    // CHECK-NEXT: %[[FUNC:.*]] = load
+    // CHECK-NEXT: call i32 %[[FUNC]]
+    -static_cast<A&>(*d);
+  }
+}
+
+namespace Test6 {
+  struct A {
+    virtual ~A();
+  };
+
+  struct B : public A {
+    virtual ~B();
+  };
+
+  struct C {
+    virtual ~C();
+  };
+
+  struct D final : public C, public B {
+  };
+
+  // CHECK-LABEL: define void @_ZN5Test61fEPNS_1DE
+  void f(D* d) {
+    // CHECK: call void @_ZN5Test61DD1Ev
+    static_cast<A*>(d)->~A();
+  }
+}
+
+namespace Test7 {
+  struct foo {
+    virtual void g() {}
+  };
+
+  struct bar {
+    virtual int f() { return 0; }
+  };
+
+  struct zed final : public foo, public bar {
+    int z;
+    virtual int f() {return z;}
+  };
+
+  // CHECK-LABEL: define i32 @_ZN5Test71fEPNS_3zedE
+  int f(zed *z) {
+    // CHECK: alloca
+    // CHECK-NEXT: store
+    // CHECK-NEXT: load
+    // CHECK-NEXT: call i32 @_ZN5Test73zed1fEv
+    // CHECK-NEXT: ret
+    return static_cast<bar*>(z)->f();
+  }
+}
+
+namespace Test8 {
+  struct A { virtual ~A() {} };
+  struct B {
+    int b;
+    virtual int foo() { return b; }
+  };
+  struct C final : A, B {  };
+  // CHECK-LABEL: define i32 @_ZN5Test84testEPNS_1CE
+  int test(C *c) {
+    // CHECK: %[[THIS:.*]] = phi
+    // CHECK-NEXT: call i32 @_ZN5Test81B3fooEv(%"struct.Test8::B"* %[[THIS]])
+    return static_cast<B*>(c)->foo();
+  }
+}
+
+namespace Test9 {
+  struct A {
+    int a;
+  };
+  struct B {
+    int b;
+  };
+  struct C : public B, public A {
+  };
+  struct RA {
+    virtual A *f() {
+      return 0;
+    }
+    virtual A *operator-() {
+      return 0;
+    }
+  };
+  struct RC final : public RA {
+    virtual C *f() {
+      C *x = new C();
+      x->a = 1;
+      x->b = 2;
+      return x;
+    }
+    virtual C *operator-() {
+      C *x = new C();
+      x->a = 1;
+      x->b = 2;
+      return x;
+    }
+  };
+  // CHECK: define {{.*}} @_ZN5Test91fEPNS_2RCE
+  A *f(RC *x) {
+    // FIXME: It should be possible to devirtualize this case, but that is
+    // not implemented yet.
+    // CHECK: load
+    // CHECK: bitcast
+    // CHECK: [[F_PTR_RA:%.+]] = bitcast
+    // CHECK: [[VTABLE:%.+]] = load {{.+}} [[F_PTR_RA]]
+    // CHECK: [[VFN:%.+]] = getelementptr inbounds {{.+}} [[VTABLE]], i{{[0-9]+}} 0
+    // CHECK-NEXT: %[[FUNC:.*]] = load {{.+}} [[VFN]]
+    // CHECK-NEXT: = call {{.*}} %[[FUNC]]
+    return static_cast<RA*>(x)->f();
+  }
+  // CHECK: define {{.*}} @_ZN5Test93fopEPNS_2RCE
+  A *fop(RC *x) {
+    // FIXME: It should be possible to devirtualize this case, but that is
+    // not implemented yet.
+    // CHECK: load
+    // CHECK: bitcast
+    // CHECK: [[F_PTR_RA:%.+]] = bitcast
+    // CHECK: [[VTABLE:%.+]] = load {{.+}} [[F_PTR_RA]]
+    // CHECK: [[VFN:%.+]] = getelementptr inbounds {{.+}} [[VTABLE]], i{{[0-9]+}} 1
+    // CHECK-NEXT: %[[FUNC:.*]] = load {{.+}} [[VFN]]
+    // CHECK-NEXT: = call {{.*}} %[[FUNC]]
+    return -static_cast<RA&>(*x);
+  }
+}
+
+namespace Test10 {
+  struct A {
+    virtual int f();
+  };
+
+  struct B : A {
+    int f() final;
+  };
+
+  // CHECK-LABEL: define i32 @_ZN6Test101fEPNS_1BE
+  int f(B *b) {
+    // CHECK: call i32 @_ZN6Test101B1fEv
+    return static_cast<A *>(b)->f();
+  }
+}
+
+namespace Test11 {
+  // Check that the definitions of Derived's operators are emitted.
+
+  // CHECK-LABEL: define linkonce_odr void @_ZN6Test111SIiE4foo1Ev(
+  // CHECK: call void @_ZN6Test111SIiE7DerivedclEv(
+  // CHECK: call zeroext i1 @_ZN6Test111SIiE7DerivedeqERKNS_4BaseE(
+  // CHECK: call zeroext i1 @_ZN6Test111SIiE7DerivedntEv(
+  // CHECK: call dereferenceable(4) %"class.Test11::Base"* @_ZN6Test111SIiE7DerivedixEi(
+  // CHECK: define linkonce_odr void @_ZN6Test111SIiE7DerivedclEv(
+  // CHECK: define linkonce_odr zeroext i1 @_ZN6Test111SIiE7DerivedeqERKNS_4BaseE(
+  // CHECK: define linkonce_odr zeroext i1 @_ZN6Test111SIiE7DerivedntEv(
+  // CHECK: define linkonce_odr dereferenceable(4) %"class.Test11::Base"* @_ZN6Test111SIiE7DerivedixEi(
+  class Base {
+  public:
+    virtual void operator()() {}
+    virtual bool operator==(const Base &other) { return false; }
+    virtual bool operator!() { return false; }
+    virtual Base &operator[](int i) { return *this; }
+  };
+
+  template<class T>
+  struct S {
+    class Derived final : public Base {
+    public:
+      void operator()() override {}
+      bool operator==(const Base &other) override { return true; }
+      bool operator!() override { return true; }
+      Base &operator[](int i) override { return *this; }
+    };
+
+    Derived *ptr = nullptr, *ptr2 = nullptr;
+
+    void foo1() {
+      if (ptr && ptr2) {
+        // These calls get devirtualized. Linkage fails if the definitions of
+        // the called functions are not emitted.
+        (*ptr)();
+        (void)(*ptr == *ptr2);
+        (void)(!(*ptr));
+        (void)((*ptr)[1]);
+      }
+    }
+  };
+
+  void foo2() {
+    S<int> *s = new S<int>;
+    s->foo1();
+  }
+}