diff mbox

[fortran,4.4/4.5/4.6,regression] ICE in resolve_equivalence

Message ID 4C4C5993.1090802@sfr.fr
State New
Headers show

Commit Message

Mikael Morin July 25, 2010, 3:34 p.m. UTC 
Hello,

this patch fixes pr44660 where a symbol pointer is kept in the gfc_equiv 
list even if the statement is rejected and the symbol freed.
The patch restores the previous equivalence list if the statement is 
rejected.

There is no testcase as the ones in the pr were not giving any ICE on my 
platform. I can add them if someone prefers. I have checked that the 
valgrind errors are gone for both of them anyway.

Regression tested on x86_64-unknown-freebsd8.0. OK for trunk/4.5/4.4 ?
I need release manager approval for accelerated commit on 4.5 (if not 
too late).

Mikael.
2010-07-25  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/44660
	* gfortran.h (gfc_namespace): New field old_equiv.
	(gfc_free_equiv_until): New prototype.
	* match.c (gfc_free_equiv_until): New, renamed from gfc_free_equiv with
	a parameterized stop condition.
	(gfc_free_equiv): Use gfc_free_equiv_until.
	* parse.c (next_statement): Save equivalence list.
	(reject_statement): Restore equivalence list.

Comments

Tobias Burnus July 25, 2010, 3:55 p.m. UTC | #1 
Mikael Morin wrote:
> There is no testcase as the ones in the pr were not giving any ICE on
> my platform. I can add them if someone prefers. I have checked that
> the valgrind errors are gone for both of them anyway.
>
> Regression tested on x86_64-unknown-freebsd8.0. OK for trunk/4.5/4.4 ?
>
> I need release manager approval for accelerated commit on 4.5 (if not
> too late).

OK. (In general, I prefer test cases, however, for this PR, I do not
feel strong about it.)

Tobias
Mikael Morin July 25, 2010, 5:08 p.m. UTC | #2 
Le 25.07.2010 17:55, Tobias Burnus a écrit :
>
> Mikael Morin wrote:
>> There is no testcase as the ones in the pr were not giving any ICE on
>> my platform. I can add them if someone prefers. I have checked that
>> the valgrind errors are gone for both of them anyway.
>>
>> Regression tested on x86_64-unknown-freebsd8.0. OK for trunk/4.5/4.4 ?
>>
>> I need release manager approval for accelerated commit on 4.5 (if not
>> too late).
>
> OK. (In general, I prefer test cases, however, for this PR, I do not
> feel strong about it.)
Committed (without testcase) as revision 162516.

The question is whether such a memory problem (access to freed memory) 
can be caught by the testsuite (maybe with valgrind-checking enabled?). 
If so, then it makes sense to add a testcase, even if a normal build 
would not notice a regression there.

Mikael
Mikael Morin July 27, 2010, 12:14 p.m. UTC | #3 
Le 25.07.2010 17:34, Mikael Morin a écrit :
> Regression tested on x86_64-unknown-freebsd8.0. OK for trunk/4.5/4.4 ?
Committed on trunk, but:
> I need release manager approval for accelerated commit on 4.5 (if not
> too late).
for the patch:
http://gcc.gnu.org/ml/gcc-patches/2010-07/msg01989.html

Mikael
Richard Biener July 27, 2010, 12:52 p.m. UTC | #4 
On Tue, Jul 27, 2010 at 2:14 PM, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 25.07.2010 17:34, Mikael Morin a écrit :
>>
>> Regression tested on x86_64-unknown-freebsd8.0. OK for trunk/4.5/4.4 ?
>
> Committed on trunk, but:
>>
>> I need release manager approval for accelerated commit on 4.5 (if not
>> too late).
>
> for the patch:
> http://gcc.gnu.org/ml/gcc-patches/2010-07/msg01989.html

Please wait until after the 4.5.1 release.

Thanks,
Richard.

> Mikael
>
>
diff mbox

Patch

Index: gfortran.h
===================================================================
--- gfortran.h	(revision 162478)
+++ gfortran.h	(working copy)
@@ -1355,7 +1355,7 @@  typedef struct gfc_namespace
   struct gfc_code *code;
 
   /* Points to the equivalences set up in this namespace.  */
-  struct gfc_equiv *equiv;
+  struct gfc_equiv *equiv, *old_equiv;
 
   /* Points to the equivalence groups produced by trans_common.  */
   struct gfc_equiv_list *equiv_lists;
@@ -2611,6 +2611,7 @@  void gfc_free_forall_iterator (gfc_forall_iterator
 void gfc_free_alloc_list (gfc_alloc *);
 void gfc_free_namelist (gfc_namelist *);
 void gfc_free_equiv (gfc_equiv *);
+void gfc_free_equiv_until (gfc_equiv *, gfc_equiv *);
 void gfc_free_data (gfc_data *);
 void gfc_free_case_list (gfc_case *);
 
Index: match.c
===================================================================
--- match.c	(revision 162478)
+++ match.c	(working copy)
@@ -4035,18 +4035,25 @@  gfc_match_module (void)
    do this.  */
 
 void
-gfc_free_equiv (gfc_equiv *eq)
+gfc_free_equiv_until (gfc_equiv *eq, gfc_equiv *stop)
 {
-  if (eq == NULL)
+  if (eq == stop)
     return;
 
   gfc_free_equiv (eq->eq);
-  gfc_free_equiv (eq->next);
+  gfc_free_equiv_until (eq->next, stop);
   gfc_free_expr (eq->expr);
   gfc_free (eq);
 }
 
 
+void
+gfc_free_equiv (gfc_equiv *eq)
+{
+  gfc_free_equiv_until (eq, NULL);
+}
+
+
 /* Match an EQUIVALENCE statement.  */
 
 match
Index: parse.c
===================================================================
--- parse.c	(revision 162478)
+++ parse.c	(working copy)
@@ -892,6 +892,7 @@  next_statement (void)
   gfc_new_block = NULL;
 
   gfc_current_ns->old_cl_list = gfc_current_ns->cl_list;
+  gfc_current_ns->old_equiv = gfc_current_ns->equiv;
   for (;;)
     {
       gfc_statement_label = NULL;
@@ -1651,6 +1652,9 @@  reject_statement (void)
   gfc_free_charlen (gfc_current_ns->cl_list, gfc_current_ns->old_cl_list);
   gfc_current_ns->cl_list = gfc_current_ns->old_cl_list;
 
+  gfc_free_equiv_until (gfc_current_ns->equiv, gfc_current_ns->old_equiv);
+  gfc_current_ns->equiv = gfc_current_ns->old_equiv;
+
   gfc_new_block = NULL;
   gfc_undo_symbols ();
   gfc_clear_warning ();