On Thu, 20 Mar 2014, Martin Jambor wrote:
> 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.
I agree with Jakub, the following variant is ok.
Thanks,
Richard.
> Thanks,
>
> Martin
>
> 2014-03-20 Martin Jambor <mjam...@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();
> +}
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
