diff mbox

Fix realloc_on_assign_2.f03, random segfaults/ICEs

Message ID Pine.LNX.4.64.1103301815260.19760@wotan.suse.de
State New
Headers show

Commit Message

Michael Matz March 30, 2011, 4:21 p.m. UTC 
Hello,

I noticed this some time ago already, but it went away.  Now it reoccured 
and made me suspicious.  "It" being random ICEs in fold_convert_loc, or 
segfaults, sometimes reproducable, sometimes not, on the 
realloc_on_assign_2.f03 testcase.

The issue is that the frontend leaks the address of a local variable to 
outside its containing scope.  Depending on the phase of moon, 
optimization factor and the like this stack space then is or is not 
overwritten at the use site.  For me loop->to[0] was overwritten with a 
type node, ultimately ICEing in fold_convert when it tries to convert a 
type to a, well, type :)

I haven't audited all users of gfc_copy_loopinfo_to_se, many seem to store 
the address of a local variable into the se structure.  But those I looked 
at seem to be safe, in the sense of not using the se structure 
outside places where loop is also defined.  Except for this one use I'm 
patching below, by moving the loop variable to the place that contains 
also the se structure.

Okay for trunk?  (regstrapping on x86_64-linux in progress)


Ciao,
Michael.

Comments

Tobias Burnus March 30, 2011, 5:05 p.m. UTC | #1 
On 03/30/2011 06:21 PM, Michael Matz wrote:
> Okay for trunk?  (regstrapping on x86_64-linux in progress)

OK. Thanks for tracing the bug. I think the issue could be the reason 
for the elusive PR 47516 - thus, you might consider adding that PR to 
the ChangeLog and close the PR afterwards.

Tobias
Michael Matz March 30, 2011, 5:33 p.m. UTC | #2 
Hi,

On Wed, 30 Mar 2011, Tobias Burnus wrote:

> On 03/30/2011 06:21 PM, Michael Matz wrote:
> > Okay for trunk?  (regstrapping on x86_64-linux in progress)
> 
> OK. Thanks for tracing the bug. I think the issue could be the reason for the
> elusive PR 47516 - thus, you might consider adding that PR to the ChangeLog
> and close the PR afterwards.

(r171736) Hmm, it seems to also be a problem for 4.6.0.  Should I merge it 
to the 4.6 branch too?


Ciao,
Michael.
Tobias Burnus March 30, 2011, 5:36 p.m. UTC | #3 
On 03/30/2011 07:33 PM, Michael Matz wrote:
> Hi,
>
> On Wed, 30 Mar 2011, Tobias Burnus wrote:
>
>> On 03/30/2011 06:21 PM, Michael Matz wrote:
>>> Okay for trunk?  (regstrapping on x86_64-linux in progress)
>> OK. Thanks for tracing the bug. I think the issue could be the reason for the
>> elusive PR 47516 - thus, you might consider adding that PR to the ChangeLog
>> and close the PR afterwards.
> (r171736) Hmm, it seems to also be a problem for 4.6.0.  Should I merge it
> to the 4.6 branch too?

That's fine with me - the fix is really simple (after one found the 
cause). Fortunately, the bug only rarely surfaces.

Tobias
diff mbox

Patch

Index: fortran/trans-expr.c
===================================================================
--- fortran/trans-expr.c	(revision 171734)
+++ fortran/trans-expr.c	(working copy)
@@ -5510,20 +5512,20 @@  arrayfunc_assign_needs_temporary (gfc_ex
    reallocatable assignments from extrinsic function calls.  */
 
 static void
-realloc_lhs_loop_for_fcn_call (gfc_se *se, locus *where, gfc_ss **ss)
+realloc_lhs_loop_for_fcn_call (gfc_se *se, locus *where, gfc_ss **ss,
+			       gfc_loopinfo *loop)
 {
-  gfc_loopinfo loop;
   /* Signal that the function call should not be made by
      gfc_conv_loop_setup. */
   se->ss->is_alloc_lhs = 1;
-  gfc_init_loopinfo (&loop);
-  gfc_add_ss_to_loop (&loop, *ss);
-  gfc_add_ss_to_loop (&loop, se->ss);
-  gfc_conv_ss_startstride (&loop);
-  gfc_conv_loop_setup (&loop, where);
-  gfc_copy_loopinfo_to_se (se, &loop);
-  gfc_add_block_to_block (&se->pre, &loop.pre);
-  gfc_add_block_to_block (&se->pre, &loop.post);
+  gfc_init_loopinfo (loop);
+  gfc_add_ss_to_loop (loop, *ss);
+  gfc_add_ss_to_loop (loop, se->ss);
+  gfc_conv_ss_startstride (loop);
+  gfc_conv_loop_setup (loop, where);
+  gfc_copy_loopinfo_to_se (se, loop);
+  gfc_add_block_to_block (&se->pre, &loop->pre);
+  gfc_add_block_to_block (&se->pre, &loop->post);
   se->ss->is_alloc_lhs = 0;
 }
 
@@ -5591,6 +5593,7 @@  gfc_trans_arrayfunc_assign (gfc_expr * e
   gfc_se se;
   gfc_ss *ss;
   gfc_component *comp = NULL;
+  gfc_loopinfo loop;
 
   if (arrayfunc_assign_needs_temporary (expr1, expr2))
     return NULL;
@@ -5641,7 +5644,7 @@  gfc_trans_arrayfunc_assign (gfc_expr * e
     {
       if (!expr2->value.function.isym)
 	{
-	  realloc_lhs_loop_for_fcn_call (&se, &expr1->where, &ss);
+	  realloc_lhs_loop_for_fcn_call (&se, &expr1->where, &ss, &loop);
 	  ss->is_alloc_lhs = 1;
 	}
       else