[Bug fortran/40246] New: ICE on invalid SOURCE= using NULLIFY

[Bug fortran/40246] New: ICE on invalid SOURCE= using NULLIFY

[burnus at gcc dot gnu dot org]

real, pointer :: ptr
    nullify(ptr, mesh%coarser)
    end

gives an ICE:

    nullify(ptr, mesh%coarser)
   1
Internal Error at (1):
free_expr0(): Bad expr type


-- 
           Summary: ICE on invalid SOURCE= using NULLIFY
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Keywords: ice-on-invalid-code
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: burnus at gcc dot gnu dot org


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

[Bug fortran/40246] ICE on invalid SOURCE= using NULLIFY

[burnus at gcc dot gnu dot org]

------- Comment #1 from burnus at gcc dot gnu dot org  2009-05-26 08:32 -------
The following patch to gfc_match_nullify fixes it; however, I think one should
additionally add

  gfc_free_expr (new_st.expr1);
  new_st.expr1 = NULL;
  gfc_free_expr (new_st.expr2);
  new_st.expr2 = NULL;

to prevent a memory leak.

--- match.c     (revision 147861)
+++ match.c     (working copy)
@@ -2418,6 +2470,7 @@ syntax:

 cleanup:
   gfc_free_statements (new_st.next);
+  new_st.next = NULL;
   return MATCH_ERROR;
 }


-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |burnus at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-05-26 08:32:21
               date|                            |


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

[Bug fortran/40246] ICE on invalid SOURCE= using NULLIFY

[kargl at gcc dot gnu dot org]

------- Comment #2 from kargl at gcc dot gnu dot org  2009-05-26 14:18 -------
(In reply to comment #1)
> The following patch to gfc_match_nullify fixes it; however, I think one should
> additionally add
> 
>   gfc_free_expr (new_st.expr1);
>   new_st.expr1 = NULL;
>   gfc_free_expr (new_st.expr2);
>   new_st.expr2 = NULL;
> 
> to prevent a memory leak.
>

AFAICT, there is no leak.

gfc_free_statement (gfc_code *p)
{
  if (p->expr1)
    gfc_free_expr (p->expr1);
  if (p->expr2)
    gfc_free_expr (p->expr2);


gfc_free_expr (gfc_expr *e)
{
  if (e == NULL)
    return;
  if (e->con_by_offset)
    splay_tree_delete (e->con_by_offset); 
  free_expr0 (e);
  gfc_free (e);
}

The last gfc_free(e) should prevent a leak.


-- 


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

[Bug fortran/40246] ICE on invalid SOURCE= using NULLIFY

[burnus at gcc dot gnu dot org]

------- Comment #3 from burnus at gcc dot gnu dot org  2009-05-26 14:45 -------
> > The following patch to gfc_match_nullify fixes it; however, I think one 
> > should additionally add
> >   gfc_free_expr (new_st.expr1);
> >   gfc_free_expr (new_st.expr2);
> > to prevent a memory leak.
> 
> AFAICT, there is no leak.
> 
> gfc_free_statement (gfc_code *p)

Well, I only see a call to "gfc_free_statement (new_st.next)". Thus only
"new_st->next" and not "new_st" is touched. However, we added two expressions
to new_st, viz expr1 and expr2, which I believe need to be freed.

(Seemingly, we cannot free new_st itself, which would be simpler.)


-- 


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

[Bug fortran/40246] ICE on invalid SOURCE= using NULLIFY

[kargl at gcc dot gnu dot org]

------- Comment #4 from kargl at gcc dot gnu dot org  2009-05-26 15:11 -------
(In reply to comment #3)
> > > The following patch to gfc_match_nullify fixes it; however, I think one 
> > > should additionally add
> > >   gfc_free_expr (new_st.expr1);
> > >   gfc_free_expr (new_st.expr2);
> > > to prevent a memory leak.
> > 
> > AFAICT, there is no leak.
> > 
> > gfc_free_statement (gfc_code *p)
> 
> Well, I only see a call to "gfc_free_statement (new_st.next)". Thus only
> "new_st->next" and not "new_st" is touched. However, we added two expressions
> to new_st, viz expr1 and expr2, which I believe need to be freed.
> 
> (Seemingly, we cannot free new_st itself, which would be simpler.)

I conflated your patch with new_st.next with your aside about the
memory leak.  Having looked at the code, I think you are correct
that in the cleanup: code path, we may need to expr1 and expr2.

valgrind doesn't run on FreeBSD.  Can you see if valgrind detects a
leak?


-- 


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

[Bug fortran/40246] ICE on invalid SOURCE= using NULLIFY

[burnus at gcc dot gnu dot org]

------- Comment #5 from burnus at gcc dot gnu dot org  2009-05-26 19:23 -------
Subject: Bug 40246

Author: burnus
Date: Tue May 26 19:23:45 2009
New Revision: 147879

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147879
Log:
2009-05-26  Tobias Burnus  <burnus@net-b.de>

        PR fortran/40246
        * match.c (gfc_match_nullify): NULLify freed pointer.

2009-05-26  Tobias Burnus  <burnus@net-b.de>

        PR fortran/40246
        * gfortran.dg/nullify_4.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/nullify_4.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/match.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug fortran/40246] ICE on invalid SOURCE= using NULLIFY

[burnus at gcc dot gnu dot org]

------- Comment #6 from burnus at gcc dot gnu dot org  2009-05-26 19:30 -------
Fixed on the trunk (4.5). Thanks Xavier for reporting it!


-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


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