Created attachment 11587 [details]
test case

test case... still fairly large but I've been running delta for 24 hours now

Janis, do you think you can do a regression hunt on this bug?

Works for me with r114332 on i686.

I was using revision 114238.  Do you know if there has been a change that might have fixed this?

Subject: Re:  [4.2 regression] segfault in
 ipa-inline.c, if (e->callee->local.disregard_inline_limits

On Sat, 3 Jun 2006, tbm at cyrius dot com wrote:

> I was using revision 114238.  Do you know if there has been a change that might
> have fixed this?

I don't see anything obvious.

Created attachment 11591 [details]
less reduced test case

This (less reduced) test case still shows the segfault with current SVN.  It takes fairly long though.

A regression hunt on powerpc-linux using the less-reduced test case with the options "-O2 --param ggc-min-expand=0 --param ggc-min-heapsize=0" identified this patch:

    http://gcc.gnu.org/viewcvs?view=rev&rev=112753

    r112753 | hubicka | 2006-04-07 15:24:39 +0000 (Fri, 07 Apr 2006)

I am going to reduce this further.

Created attachment 11620 [details]
shorest testcase I can reduce to

This is the shorest testcase I could reduce this to, I did it on powerpc-darwin but I found removing some templates causes this to go "right" which seems wrong.

The last time I ran into this was back in 2005, and I had committed the following patch:
2005-08-29  Andrew Pinski  <pinskia@physics.uc.edu>

        PR middle-end/23408
        * ipa-inline.c (cgraph_decide_inlining_incrementally): Remove the
        call to ggc_collect.

So I had almost thought the gc problem would be an issue again :(.

The dtor for "Ref<std::vector<Ref<Node>, std::allocator<Ref<Node> > > >" is the node which has been freed.

Wait in tree-inline.c, we do:
  /* Update callgraph if needed.  */
  cgraph_remove_node (cg_edge->callee);

Isn't that wrong as we could inline the callee a couple of times?
Don't we want to do:
  /* Update callgraph if needed.  */
  cgraph_remove_edge (cg_edge);

???

Nothing I have tried so far has worked and I don't understand how we could remove a node from here.

Oh, I see remove node is correct, we duplicate the nodes which I did not know about until now.

Subject: Re:  [4.2 regression] segfault in ipa-inline.c, if (e->callee->local.disregard_inline_limits

> 
> 
> ------- Comment #12 from pinskia at gcc dot gnu dot org  2006-06-07 06:00 -------
> Wait in tree-inline.c, we do:
>   /* Update callgraph if needed.  */
>   cgraph_remove_node (cg_edge->callee);
> 
> Isn't that wrong as we could inline the callee a couple of times?

It should be OK - if we inline multiple times, we create multiple nodes.
I will look into this.

Honza

Note the problem is possibly at least latent on the 4.1 branch.

I just got this segfault with when compiling another application.  Should I attach the preprocessed source to this PR or do you have enough information already to fix it?

This segfault also shows up when compiling the Linux kernel (compiling file net/tipc/net.c).

A pre-processed C test case would be nice.

Created attachment 11654 [details]
C test case

Here's a C test case (from the Linux kernel).

5289:tbm@reyes: ~] /usr/local/bin/gcc -c -O2 --param ggc-min-expand=0 --param ggc-min-heapsize=0 net.i
net/tipc/net.c:311: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
zsh: exit 1     /usr/local/bin/gcc -c -O2 --param ggc-min-expand=0 --param ggc-min-heapsize=0

Finally, a *small* test case.

5336:tbm@reyes: ~/tmp/delta/bin] /usr/local/bin/gcc -c -O1 --param ggc-min-expand=0 --param ggc-min-heapsize=0 mini.c
mini.c:27: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
5337:tbm@reyes: ~/tmp/delta/bin] /usr/local/bin/gcc -c --param ggc-min-expand=0 --param ggc-min-heapsize=0 mini.c
5338:tbm@reyes: ~/tmp/delta/bin] cat mini.c
struct sk_buff
{
}
raw_hdlc_proto;
     struct tipc_msg *buf_msg (struct sk_buff *skb)
{
}
     void buf_discard (struct sk_buff *skb)
{
}
void tipc_net_route_msg (struct sk_buff *buf);
static inline __attribute__ ((always_inline))
     tipc_port_recv_msg (struct sk_buff *buf)
{
    {
      tipc_net_route_msg (buf);
    }
}
void
tipc_net_route_msg (struct sk_buff *buf)
{
    {
        {
            tipc_port_recv_msg (buf);
        }
    }
}
5339:tbm@reyes: ~/tmp/delta/bin]

I cannot reproduce the ICE with the original testcase.
I can reproduce it with the testcase from comment #19 and #20, though.
Here's an even shorter version:

=======================
void foo();

static inline bar()
{
    foo();
}

void foo()
{
    bar();
}
=======================

In addition I can only reproduce the problem on the 4.1 branch, starting
with GCC 4.1.1, but not on mainline (probably latent, though).

I see what the problem is.  cgraph_postorder doesn't help us if we're having a cycle like here, so

  nnodes = cgraph_postorder (order);
  for (i = nnodes - 1; i >= 0; i--)
    {
      node = order[i];
      if (node->analyzed && node->local.inlinable
          && (node->needed || node->reachable)
          && node->callers)
        {
          if (cgraph_decide_inlining_incrementally (node, true))
            ggc_collect ();
        }
    }

after inlining into foo, which we do first, bar is no longer referenced
and is collected.  We still reference it from order[0] though.

I have a patch:

Index: ipa-inline.c
===================================================================
--- ipa-inline.c        (revision 115554)
+++ ipa-inline.c        (working copy)
@@ -1133,6 +1133,7 @@ cgraph_early_inlining (void)
   struct cgraph_node **order =
     xcalloc (cgraph_n_nodes, sizeof (struct cgraph_node *));
   int i;
+  htab_t cycles;
 
   if (sorrycount || errorcount)
     return;
@@ -1142,6 +1143,8 @@ cgraph_early_inlining (void)
 #endif
 
   nnodes = cgraph_postorder (order);
+  cycles = htab_create (7, htab_hash_pointer, htab_eq_pointer, NULL);
+  cgraph_find_cycles (cgraph_nodes, cycles);
   for (i = nnodes - 1; i >= 0; i--)
     {
       node = order[i];
@@ -1149,10 +1152,13 @@ cgraph_early_inlining (void)
          && (node->needed || node->reachable)
          && node->callers)
        {
-         if (cgraph_decide_inlining_incrementally (node, true))
+         if (cgraph_decide_inlining_incrementally (node, true)
+             /* Avoid collecting if inlining in a cycle.  */
+             && !htab_find (cycles, node))
            ggc_collect ();
        }
     }
+  htab_delete (cycles);
   cgraph_remove_unreachable_nodes (true, dump_file);
 #ifdef ENABLE_CHECKING
   for (node = cgraph_nodes; node; node = node->next)

Hmm, it needs to find the root of the callgraph first.  Or all root nodes, and find cycles starting from there.

Simpler one goes along

@@ -1148,9 +1149,13 @@ cgraph_early_inlining (void)
       if (node->analyzed && node->local.inlinable
          && (node->needed || node->reachable)
          && node->callers)
+       inlined |= cgraph_decide_inlining_incrementally (node, true);
+      /* Collect at cgraph roots, which avoid collecting inside cycles.  */
+      if (inlined
+         && !node->callers)
        {
-         if (cgraph_decide_inlining_incrementally (node, true))
-           ggc_collect ();
+         ggc_collect ();
+         inlined = false;
        }
     }
   cgraph_remove_unreachable_nodes (true, dump_file);

but also has a greater impact on when we collect and when not possibly.

Well, I don't like much the limiting of inlining in recursive functions (where it is rather interesting)
and I can't convince myself that the second patch is safe (ie the cycle don't have to be in consetuctive entries in the postorder I would say).
I will test patch that simply make the array GGC root....

Thanks for looking into this,
Honza

I guess the patch format made it hard to see what the result is.  It looks like

  nnodes = cgraph_postorder (order);
  for (i = nnodes - 1; i >= 0; i--)
    {
      node = order[i];
      if (node->analyzed && node->local.inlinable
          && (node->needed || node->reachable)
          && node->callers)
        inlined |= cgraph_decide_inlining_incrementally (node, true);
      /* Collect at cgraph roots, which avoids collecting inside cycles.  */
      if (inlined
          && !node->callers)
        {
          ggc_collect ();
          inlined = false;
        }
    }

i.e. inlining is unchanged, only collection is done at a different point.
(for pure luck this saves some kB of peak memory usage and compile-time on tramp3d).  For the postorder I assumed that inlining for cgraph roots
(nodes with no callers) is decided not during deciding for inlining inside
a cycle.  One could extend this to also collect for cgraph tails, I think,
making it !node->callers || !node->callees.

Anyway, the patch gcac tested for C and C++ on x86_64-unknown-linux-gnu.

And I suspect that if you make 'order' gc allocated you'll loose the
ability to effectively collect garbage here.

Subject: Bug 27882

Author: hubicka
Date: Wed Jul 26 20:17:32 2006
New Revision: 115763

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115763
Log:
	PR tree-optimization/27882
	* cgraph.c (cgraph_remove_node): Clear needed, reachable, next, previous
	and decl fields.
	* cgraphunit.c (cgraph_reset_node): Expect cgraph_remove_node to kill
	next pointer
	(cgraph_analyze_compilation_unit): Likewise.
	* ipa.c (cgraph_remove_unreachable_nodes): Likewise.
	* ipa-inline.c (cgraph_decide_recursive_inlining): Likewise.
	(cgraph_early_inlinine): Make order garbage collected.
	* Makefile.in (gt-ipa-inline): New garbagecollected file.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/Makefile.in
    trunk/gcc/cgraph.c
    trunk/gcc/cgraphunit.c
    trunk/gcc/ipa-inline.c
    trunk/gcc/ipa.c

Closing 4.1 branch.