[Bug middle-end/60419] [4.8/4.9 Regression] ICE Segmentation fault

["jamborm at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60419

--- Comment #11 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Good job reducing the testcase to something this small!

Anyway, Jakub's analysis of what is going on is still correct and all
the high level decisions that we do are IMHO correct too.  The
invocation of symtab_remove_unreachable_symbols that removes the
callers is called with before_inlining_p set to false, even though
that is not technically right, we are still in the middle of inlining
because we are about to inline functions which are called only once.
But the comment in function ipa_inline says this is deliberate because
it helps to find the functions which are only called once.

However, this means that when symtab_remove_unreachable_symbols
correctly figures out that we can devirtualize to the offending thunk,
it assumes that cannot lead to any further inlining.  Because the
thunk is keyed in another compilation unit, it decides it only needs
to keep the symtab node but not the body.  In its terminology, the
node is in the "boundary".  The way the function cripples these
boundary nodes is actually the problem because it removes all callees
but not the thunk flag.  This leaves us with a thunk with no callee,
something that should not happen (and there is even a bit in the call
graph verifier that checks it).  Later on, we do perform this
devirtualization during inlining of functions only called once and
connect the crippled thunk to the call graph.  The first time we want
to follow through it, we segfault.

I believe that we do not want to allow thunks with no callees.  This
would mean changing every user of cgraph_function_node so that it can
cope with NULL return value and that is ugly and inconvenient.  This
leaves us with two options.  Either we make
symtab_remove_unreachable_symbols keep callees of thunks when it keeps
the thunk or we simply clear the thunk_p flag.  Given that thunks and
aliases are quite similar and we do clear the alias flag, and that
this should only ever affect nodes that we are not going to output, I
tend to believe the second approach is OK and of course it is much
simpler.  But before I propose the following fix on the mailing list,
I'll try to build some big C++ application with it:

diff --git a/gcc/ipa.c b/gcc/ipa.c
index 572dba1..164de0d 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -488,6 +488,7 @@ symtab_remove_unreachable_nodes (bool before_inlining_p,
FILE *file)
              node->definition = false;
              node->cpp_implicit_alias = false;
              node->alias = false;
+             node->thunk.thunk_p = false;
              node->weakref = false;
              if (!node->in_other_partition)
                node->local.local = false;