Hi Mikael, The comment is very welcome! Looks good to me. OK for mainline.
Thanks for the patch. Paul On Fri, 15 Sept 2023 at 08:19, Mikael Morin via Fortran <fort...@gcc.gnu.org> wrote: > > Hello, > > Harald reminded me recently that there was a working patch attached to the PR. > I added a documentation comment with the hope that it may help avoid > making the same mistake in the future. > Regression tested on x86_64-pc-linux-gnu. > OK for master? > > -- >8 -- > > Remove one reference count incrementation following the assignment of a > symbol pointer to a local variable. Most symbol pointers are "weak" pointer > and don't need any reference count update when they are assigned, and it is > especially the case of local variables. > > This fixes a memory leak with the testcase from the PR (not included). > > PR fortran/108957 > > gcc/fortran/ChangeLog: > > * gfortran.h (gfc_symbol): Add comment documenting reference counting. > * parse.cc (parse_interface): Remove reference count incrementation. > --- > gcc/fortran/gfortran.h | 20 ++++++++++++++++++++ > gcc/fortran/parse.cc | 3 --- > 2 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h > index f4a1c106cea..6caf7765ac6 100644 > --- a/gcc/fortran/gfortran.h > +++ b/gcc/fortran/gfortran.h > @@ -1944,7 +1944,27 @@ typedef struct gfc_symbol > according to the Fortran standard. */ > unsigned pass_as_value:1; > > + /* Reference counter, used for memory management. > + > + Some symbols may be present in more than one namespace, for example > + function and subroutine symbols are present both in the outer namespace > and > + the procedure body namespace. Freeing symbols with the namespaces they > are > + in would result in double free for those symbols. This field counts > + references and is used to delay the memory release until the last > reference > + to the symbol is removed. > + > + Not every symbol pointer is accounted for reference counting. Fields > + gfc_symtree::n::sym are, and gfc_finalizer::proc_sym as well. But most > of > + them (dummy arguments, generic list elements, etc) are "weak" pointers; > + the reference count isn't updated when they are assigned, and they are > + ignored when the surrounding structure memory is released. This is not > a > + problem because there is always a namespace as surrounding context and > + symbols have a name they can be referred with in that context, so the > + namespace keeps the symbol from being freed, keeping the pointer valid. > + When the namespace ceases to exist, and the symbols with it, the other > + structures referencing symbols cease to exist as well. */ > int refs; > + > struct gfc_namespace *ns; /* namespace containing this symbol */ > > tree backend_decl; > diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc > index 8f09ddf753c..58386805ffe 100644 > --- a/gcc/fortran/parse.cc > +++ b/gcc/fortran/parse.cc > @@ -4064,9 +4064,6 @@ loop: > accept_statement (st); > prog_unit = gfc_new_block; > prog_unit->formal_ns = gfc_current_ns; > - if (prog_unit == prog_unit->formal_ns->proc_name > - && prog_unit->ns != prog_unit->formal_ns) > - prog_unit->refs++; > > decl: > /* Read data declaration statements. */ > -- > 2.40.1 >
