Re: [Patch, Fortran] Plug memory leaks; fix tree-check ICE for PR

[Tobias Burnus <burnus at net-b dot de>]

On 08/27/2012 03:19 PM, Mikael Morin wrote:
> as you seem to be very much into memory issues,

Well, it started as middle-end project by Steven Bosscher and others 
which looked at memory issues due to  http://gcc.gnu.org/PR54146 (a 
program using attribute((flatten)), which essentially causes the whole 
program to be inlined) - the PR is invalid, but it was seen as 
opportunity to fix memory issues. The other was the 4.8 regression 
http://gcc.gnu.org/PR54332 - where SPEC's 481.wrf needed >10GB to compile.

At that point Fortran came into the game as wrf (Weather Research and 
Forecasting) is written in Fortran (and C). [The issue turned out to be 
a lacking df_free_collection_rec call in df_bb_verify.] In course of 
looking at PR54332, Jakub asked me to reduce the leakage in Fortran. 
Besides increasing the memory use of GCC a bit (the FE memory is freed 
before the ME starts allocating a lot of memory), it also cluttered the 
valgrind output. As an exchange, he fixed the gfortran's issue with 
init_emit.

I think the most important FE leaks are now fixed. Though, we should try 
to fix the remaining as well and try to make sure that no new leaks pop 
up with new code. See PR54384 for some remaining issues. (I do not 
intent to work on them in the near future.)

> could you add comments
> in gfortran.h telling which pointers account for reference counting?
> As far as I remember for symbols, there are:
>    gfc_symtree::n::sym;
>    gfc_namespace::proc_name;

Well, I have still not gained a full overview about refs, but there are 
refs in gfc_namespace, gfc_symbol and (new) in gfc_common_head.


The latter is easy: For each symbol in the (named) common block, the 
refs is incremented, and in gfc_free_symbol it is decremented and 
deleted if refs == 1. [For blank commons, gfc_namespace contains a 
nonpointer gfc_common_head field.]


For namespaces, refs gets set to 1 at creation in gfc_get_namespace; it 
gets incremented if the ns is used by a sym->formal_ns (unless 
sym->formal_ns == sym->ns);  the formal_ns gets freed if 
sym->formal_ns->refs == 2. And gfc_free_namespace decrements its refs - 
and frees the namespace if refs == 1 (before the decrement). There is 
also in module.c a mio_namespace_ref which refs++. The latter and the 
similar code in resolve.c is needed for (quoting gfortran.h):

  /* Normally we don't need to refcount namespaces.  However when we read
     a module containing a function with multiple entry points, this
     will appear as several functions with the same formal namespace.  */


And for gfc_symbol->refs: That get's always incremented when the symbol 
is referenced, and decremented if the a symbol is released.


Actually, it is unclear to me what kind of comment you would like to see 
in gfortran.h.


> In the old patch athttp://gcc.gnu.org/bugzilla/attachment.cgi?id=21016
> I was using helper functions to do refcount updates, like below (for
> namespaces, there was the same for symbols). They handled correctly rhs
> == NULL and lhs == rhs, and took care of freeing the lhs (if necessary).
> You could use something alike.

As the bug fixes showed, most issues occurred because refs were 
incremented although they shouldn't. Either as copy'n'paste bug or 
because to paper over some logic bug. I don't think that any helper 
function would have helped.

 * * *

The attached patch fixes the gfc_ss leakage in Polyhedron's channel.f90 
and rnflow.f90. I have committed the patch after successful regtesting 
as Rev. 190713. That's hopefully the last gfc_ss leakage.

Tobias
Index: gcc/fortran/trans-expr.c
===================================================================
--- gcc/fortran/trans-expr.c	(revision 190712)
+++ gcc/fortran/trans-expr.c	(revision 190713)
@@ -6781,6 +6781,11 @@
   gfc_conv_function_expr (&se, expr2);
   gfc_add_block_to_block (&se.pre, &se.post);
 
+  if (ss)
+    gfc_cleanup_loop (&loop);
+  else
+    gfc_free_ss_chain (se.ss);
+
   return gfc_finish_block (&se.pre);
 }
 
Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(revision 190712)
+++ gcc/fortran/ChangeLog	(revision 190713)
@@ -1,5 +1,11 @@
 2012-08-27  Tobias Burnus  <burnus@net-b.de>
 
+	PR fortran/54384
+	* trans-expr.c (gfc_trans_arrayfunc_assign): Free se.ss
+	and loop.
+
+2012-08-27  Tobias Burnus  <burnus@net-b.de>
+
 	PR fortran/41093
 	* gfortran.h (gfc_common_head): Add "int refs".
 	* match.c (gfc_match_common): Increment refs.