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

[Mikael Morin <mikael dot morin at sfr dot fr>]

On 27/08/2012 14:14, Tobias Burnus wrote:
> On 08/26/2012 08:12 PM, Tobias Burnus wrote:
>> This patch fixes one ICE and several memory leaks. But there are more.
>>
> 
> I have now committed the patch with the following additional patch
>         * module.c (mio_symbol): Don't increase sym->refs for its
>         use in sym->formal_ns->proc_name.
> 
> The patch for the ICE (PR54370) was committed as Rev. 190709, the rest
> (memory leakage, including PR41093)  as Rev. 190710.
> 
> The additional patch is required as otherwise sym->refs is too high for
> gfc_free_symbol - and gfc_free_namespace doesn't free (and thus
> decrement) ns->proc_name.
> 
> 
> I have regtested and tested the patch with a couple of programs, I also
> checked some with valgrind; the leakage decreased quit a bit but there
> are still known leaks, cf.
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54384
> 
> I do hope that the patch didn't cause any regression, but I do not rule
> out problems in some special situations as the memory handling is not
> always that transparent.
> 
> Tobias
> 
> 
> --- a/gcc/fortran/module.c
> +++ b/gcc/fortran/module.c
> @@ -3807,10 +3807,7 @@ mio_symbol (gfc_symbol *sym)
>      {
>        mio_namespace_ref (&sym->formal_ns);
>        if (sym->formal_ns)
> -       {
> -         sym->formal_ns->proc_name = sym;
> -         sym->refs++;
> -       }
> +       sym->formal_ns->proc_name = sym;
>      }
> 
>    /* Save/restore common block links.  */

Hello,

as you seem to be very much into memory issues, 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;

and for namespaces, there is:
  gfc_symbol::formal_ns;
what else? does it make sense at all to have it reference counted?

***

In the old patch at http://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.

void
gfc_set_ns_ptr (gfc_namespace **dest, gfc_namespace *src)
{
  gfc_namespace *ns;

  if (src)
    src->refs++;

  ns = *dest;
  *dest = src;
  gfc_free_namespace (ns);
}

Mikael