[PATCH, PR 59176] Mark "zombie" call graph nodes to remove verifier false positive

[Martin Jambor]

Hi,

On Thu, Mar 20, 2014 at 07:40:56PM +0100, Jakub Jelinek wrote:
> On Thu, Mar 20, 2014 at 05:07:32PM +0100, Martin Jambor wrote:
> > in the PR, verifier claims an edge is pointing to a wrong declaration
> > even though it has successfully verified the edge multiple times
> > before.  The reason is that symtab_remove_unreachable_nodes decides to
> > "remove the body" of a node and also clear any information that it is
> > an alias of another in the process (more detailed analysis in comment
> > #9 of the bug).
> > 
> > In bugzilla Honza wrote that "silencing the verifier" is the way to
> > go.  Either we can dedicate a new flag in each cgraph_node or
> > symtab_node just for the purpose of verification or do something more
> > hackish like the patch below which re-uses the former_clone_of field
> > for this purpose.  Since clones are always private nodes, they should
> > always either survive removal of unreachable nodes or be completely
> > killed by it and should never enter the in_border zombie state.
> > Therefore their former_clone_of must always be NULL.  So I added a new
> > special value, error_mark_node, to mark this zombie state and taught
> > the verifier to be happy with such nodes.
> > 
> > Bootstrapped and tested on x86_64-linux.  What do you think?
> 
> Don't we have like 22 spare bits in cgraph_node and 20 spare bits in
> symtab_node?  I'd find it clearer if you just used a new flag to mark the
> zombie nodes.  Though, I'll let Richard or Honza to decide, don't feel
> strongly about it.
> 

I guess you are right, here is the proper version which is currently
undergoing bootstrap and testing.

Thanks,

Martin

2014-03-20  Martin Jambor  <mjambor@suse.cz>

	PR ipa/59176
	* cgraph.h (symtab_node): New flag body_removed.
	* ipa.c (symtab_remove_unreachable_nodes): Set body_removed flag
	when removing bodies.
	* symtab.c (dump_symtab_base): Dump body_removed flag.
	* cgraph.c (verify_edge_corresponds_to_fndecl): Skip nodes which
	had their bodies removed.

testsuite/
        * g++.dg/torture/pr59176.C: New test.

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index a15b6bc..fb6880c 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -2601,8 +2601,13 @@ verify_edge_corresponds_to_fndecl (struct cgraph_edge *e, tree decl)
   node = cgraph_get_node (decl);
 
   /* We do not know if a node from a different partition is an alias or what it
-     aliases and therefore cannot do the former_clone_of check reliably.  */
-  if (!node || node->in_other_partition || e->callee->in_other_partition)
+     aliases and therefore cannot do the former_clone_of check reliably.  When
+     body_removed is set, we have lost all information about what was alias or
+     thunk of and also cannot proceed.  */
+  if (!node
+      || node->body_removed
+      || node->in_other_partition
+      || e->callee->in_other_partition)
     return false;
   node = cgraph_function_or_thunk_node (node, NULL);
 
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 32b1ee1..59d9ce6 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -91,7 +91,9 @@ public:
   unsigned forced_by_abi : 1;
   /* True when the name is known to be unique and thus it does not need mangling.  */
   unsigned unique_name : 1;
-
+  /* True when body and other characteristics have been removed by
+     symtab_remove_unreachable_nodes. */
+  unsigned body_removed : 1;
 
   /*** WHOPR Partitioning flags.
        These flags are used at ltrans stage when only part of the callgraph is
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 572dba1..4a8c6b7 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -484,6 +484,7 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
 	    {
 	      if (file)
 		fprintf (file, " %s", node->name ());
+	      node->body_removed = true;
 	      node->analyzed = false;
 	      node->definition = false;
 	      node->cpp_implicit_alias = false;
@@ -542,6 +543,7 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
 		fprintf (file, " %s", vnode->name ());
 	      changed = true;
 	    }
+	  vnode->body_removed = true;
 	  vnode->definition = false;
 	  vnode->analyzed = false;
 	  vnode->aux = NULL;
diff --git a/gcc/symtab.c b/gcc/symtab.c
index 5d69803..0ce8e8e 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -601,6 +601,8 @@ dump_symtab_base (FILE *f, symtab_node *node)
 	     ? IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME
 				     (node->alias_target))
 	     : IDENTIFIER_POINTER (node->alias_target));
+  if (node->body_removed)
+    fprintf (f, "\n  Body removed by symtab_remove_unreachable_nodes");
   fprintf (f, "\n  Visibility:");
   if (node->in_other_partition)
     fprintf (f, " in_other_partition");
diff --git a/gcc/testsuite/g++.dg/ipa/pr59176.C b/gcc/testsuite/g++.dg/ipa/pr59176.C
new file mode 100644
index 0000000..d576bc3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr59176.C
@@ -0,0 +1,41 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+template <class> class A {
+protected:
+  void m_fn2();
+  ~A() { m_fn2(); }
+  virtual void m_fn1();
+};
+
+class D : A<int> {};
+template <class Key> void A<Key>::m_fn2() {
+  m_fn1();
+  m_fn1();
+  m_fn1();
+}
+
+#pragma interface
+class B {
+  D m_cellsAlreadyProcessed;
+  D m_cellsNotToProcess;
+
+public:
+  virtual ~B() {}
+  void m_fn1();
+};
+
+class C {
+  unsigned long m_fn1();
+  B m_fn2();
+  unsigned long m_fn3();
+};
+unsigned long C::m_fn1() {
+CellHierarchy:
+  m_fn2().m_fn1();
+}
+
+unsigned long C::m_fn3() {
+CellHierarchy:
+  m_fn2().m_fn1();
+}