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.mo...@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
