Re: [Patch, fortran] PR60898 premature release of entry symbols

[Paul Richard Thomas <paul dot richard dot thomas at gmail dot com>]

Dear Mikael,

That looks to be OK for 4.8, 4.9 and 5.0.

Strange testcase, though... :-)

Thanks for the patch

Paul

On 15 February 2015 at 18:48, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Hello,
>
> I propose a fix for PR60898, where a symbol is freed despite remaining
> reachable in the symbol tree.
> The problem comes from this code in resolve_symbol:
>>
>>     /* If we find that a flavorless symbol is an interface in one of the
>>        parent namespaces, find its symtree in this namespace, free the
>>        symbol and set the symtree to point to the interface symbol.  */
>>       for (ns = gfc_current_ns->parent; ns; ns = ns->parent)
>>       {
>>         symtree = gfc_find_symtree (ns->sym_root, sym->name);
>>         if (symtree && [...])
>>           {
>>             this_symtree = gfc_find_symtree (gfc_current_ns->sym_root,
>>                                              sym->name);
>>             gfc_release_symbol (sym);
>>             symtree->n.sym->refs++;
>>             this_symtree->n.sym = symtree->n.sym;
>>             return;
>>           }
>>       }
>>
>
> Here, the target of an element of the current namespace's name tree is
> changed to point to the outer symbol.  And the current symbol is freed,
> without checking that it really was what was in the name tree before.
>
> In the testcase https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60898#c7 ,
> the problematic symbol is an entry, which is available in the name tree
> only through a mangled name (created by gfc_get_unique_symtree in
> get_proc_name), so gfc_find_symtree won't find it by name lookup.
> In this case, what gfc_find_symtree finds is a symbol that is already
> the outer interface symbol, so reassigning this_symtree.n.sym would be a
> no-op.
>
> The patch proposed checks that sym == this_symtree->n.sym, so that the
> symbol reassignment is only made in that case.  Otherwise, the regular
> symbol resolution happens normally.
>
> This patch is a stripped down version of what I posted before in the PR,
> which contained a symbol.c part which was increasing the reference count
> locally in do_traverse_symtree, to delay symbol release after all of
> them have been processed.  That part was useless because if a symbol had
> to be processed more than once (meaning it was available under different
> names), it will have the corresponding reference count set so that it
> won't be freed too early in any case.
> Worse, that part was interacting badly with the hack used to break
> circular references in gfc_release_symbol, so it was better left out.
>
> Anyway, this is regression tested[*] on x86_64-unknown-linux-gnu. OK for
> trunk/4.9/4.8 ?
>
> Mikael
>
> [*] I have a few failing testcases (also without the patch), namely the
> following; does this ring a bell ?
> FAIL: gfortran.dg/erf_3.F90
> FAIL: gfortran.dg/fmt_g0_7.f08
> FAIL: gfortran.dg/fmt_en.f90
> FAIL: gfortran.dg/nan_7.f90
> FAIL: gfortran.dg/quad_2.f90
> FAIL: gfortran.dg/quad_3.f90
> FAIL: gfortran.dg/round_4.f90



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx