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
