* [Patch] Fortran: Avoid double-free with parse error (PR96041, PR93423)
@ 2020-09-12 21:00 Tobias Burnus
2020-09-15 6:11 ` Early ping — " Tobias Burnus
2020-09-16 7:58 ` Andre Vehreschild
0 siblings, 2 replies; 5+ messages in thread
From: Tobias Burnus @ 2020-09-12 21:00 UTC (permalink / raw)
To: fortran, gcc-patches; +Cc: Harald Anlauf
[-- Attachment #1: Type: text/plain, Size: 787 bytes --]
The testcase for PR93423 did a double free, which caused
an ICE. That's reported in PR96041.
Slightly frustrated by the FAIL in the testsuite,
I decided to debug and, hopefully, fix this.
The problem is related to putting the symtree
into a sub namespace of the symbol's ns. That's fixed up
by copying things around – except in the error case where
all those fixups are undone. Thus, when the symbol tree
is deleted, the parent's sym->formal->sym is also deleted,
causing an ICE in resolve_formal_arguments.
Hopefully, I got this all right...
I see still one memory leak for a symbol in module.c
according to valgrind, but I don't know whether it is
related to those symbols. (There are a lot of other leaks,
mostly related to polymorphism (vtab etc.).)
OK for the trunk?
Tobias
[-- Attachment #2: f-memleak.diff --]
[-- Type: text/x-patch, Size: 1052 bytes --]
Fortran: Avoid double-free with parse error (PR96041, PR93423)
gcc/fortran/
PR fortran/96041
PR fortran/93423
* decl.c (gfc_match_submod_proc): Avoid later double-free
in the error case.
gcc/fortran/decl.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index c612b492f3e..326e6f5db7a 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -9819,6 +9819,15 @@ gfc_match_submod_proc (void)
if (gfc_match_eos () != MATCH_YES)
{
+ /* Unset st->n.sym. Note: in reject_statement (), the symbol changes are
+ undone, such that the st->n.sym->formal points to the original symbol;
+ if now this namespace is finalized, the formal namespace is freed,
+ but it might be still needed in the parent namespace. */
+ gfc_symtree *st = gfc_find_symtree (gfc_current_ns->sym_root, sym->name);
+ st->n.sym = NULL;
+ gfc_free_symbol (sym->tlink);
+ sym->tlink = NULL;
+ sym->refs--;
gfc_syntax_error (ST_MODULE_PROC);
return MATCH_ERROR;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Early ping — [Patch] Fortran: Avoid double-free with parse error (PR96041, PR93423)
2020-09-12 21:00 [Patch] Fortran: Avoid double-free with parse error (PR96041, PR93423) Tobias Burnus
@ 2020-09-15 6:11 ` Tobias Burnus
2020-09-16 7:58 ` Andre Vehreschild
1 sibling, 0 replies; 5+ messages in thread
From: Tobias Burnus @ 2020-09-15 6:11 UTC (permalink / raw)
To: fortran, gcc-patches; +Cc: Harald Anlauf
Early Fortran-review ping.
Solves:
PR 96041 - [11 regression] ICE in gfortran.dg/pr93423.f90 after r11-1792
(opened 2020-07-03)
The other PR revealed/caused the issue:
PR 93423 - [8/9/10/11 Regression] ICE on invalid with argument list for module procedure
"Backports will have to wait until PR96041 is resolved." (Comment 5)
On 9/12/20 11:00 PM, Tobias Burnus wrote:
> The testcase for PR93423 did a double free, which caused
> an ICE. That's reported in PR96041.
>
> Slightly frustrated by the FAIL in the testsuite,
> I decided to debug and, hopefully, fix this.
>
> The problem is related to putting the symtree
> into a sub namespace of the symbol's ns. That's fixed up
> by copying things around – except in the error case where
> all those fixups are undone. Thus, when the symbol tree
> is deleted, the parent's sym->formal->sym is also deleted,
> causing an ICE in resolve_formal_arguments.
>
> Hopefully, I got this all right...
> I see still one memory leak for a symbol in module.c
> according to valgrind, but I don't know whether it is
> related to those symbols. (There are a lot of other leaks,
> mostly related to polymorphism (vtab etc.).)
>
> OK for the trunk?
>
> Tobias
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch] Fortran: Avoid double-free with parse error (PR96041, PR93423)
2020-09-12 21:00 [Patch] Fortran: Avoid double-free with parse error (PR96041, PR93423) Tobias Burnus
2020-09-15 6:11 ` Early ping — " Tobias Burnus
@ 2020-09-16 7:58 ` Andre Vehreschild
2020-09-16 8:35 ` Tobias Burnus
1 sibling, 1 reply; 5+ messages in thread
From: Andre Vehreschild @ 2020-09-16 7:58 UTC (permalink / raw)
To: Tobias Burnus; +Cc: fortran, gcc-patches, Harald Anlauf
Hi Tobias,
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index c612b492f3e..326e6f5db7a 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -9819,6 +9819,15 @@ gfc_match_submod_proc (void)
if (gfc_match_eos () != MATCH_YES)
{
+ /* Unset st->n.sym. Note: in reject_statement (), the symbol
changes are
+ undone, such that the st->n.sym->formal points to the
original symbol;
+ if now this namespace is finalized, the formal namespace is
freed,
+ but it might be still needed in the parent namespace. */
+ gfc_symtree *st = gfc_find_symtree (gfc_current_ns->sym_root,
sym->name);
+ st->n.sym = NULL;
Don't we need free or unlink the st node from the symtree, too?
+ gfc_free_symbol (sym->tlink);
+ sym->tlink = NULL;
+ sym->refs--;
gfc_syntax_error (ST_MODULE_PROC);
return MATCH_ERROR;
}
Regards,
Andre
On Sat, 12 Sep 2020 23:00:12 +0200
Tobias Burnus <burnus@net-b.de> wrote:
> The testcase for PR93423 did a double free, which caused
> an ICE. That's reported in PR96041.
>
> Slightly frustrated by the FAIL in the testsuite,
> I decided to debug and, hopefully, fix this.
>
> The problem is related to putting the symtree
> into a sub namespace of the symbol's ns. That's fixed up
> by copying things around – except in the error case where
> all those fixups are undone. Thus, when the symbol tree
> is deleted, the parent's sym->formal->sym is also deleted,
> causing an ICE in resolve_formal_arguments.
>
> Hopefully, I got this all right...
> I see still one memory leak for a symbol in module.c
> according to valgrind, but I don't know whether it is
> related to those symbols. (There are a lot of other leaks,
> mostly related to polymorphism (vtab etc.).)
>
> OK for the trunk?
>
> Tobias
>
--
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 178 3837536 * vehre@gmx.de
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch] Fortran: Avoid double-free with parse error (PR96041, PR93423)
2020-09-16 7:58 ` Andre Vehreschild
@ 2020-09-16 8:35 ` Tobias Burnus
2020-09-17 11:45 ` Andre Vehreschild
0 siblings, 1 reply; 5+ messages in thread
From: Tobias Burnus @ 2020-09-16 8:35 UTC (permalink / raw)
To: Andre Vehreschild, Tobias Burnus; +Cc: gcc-patches, Harald Anlauf, fortran
Hi Andre,
On 9/16/20 9:58 AM, Andre Vehreschild wrote:
> + st->n.sym = NULL;
>
> Don't we need free or unlink the st node from the symtree, too?
I did not see a way to simply remove a single symtree; as this
is the error case, I left the item in the symtree.
The symtree itself is removed when the 'contains' is
processed by calling:
gfc_free_namespace (...)
which calls
free_sym_tree (ns->sym_root)
and that function is:
if (sym_tree == NULL)
return;
free_sym_tree (sym_tree->left);
free_sym_tree (sym_tree->right);
gfc_release_symbol (sym_tree->n.sym);
free (sym_tree);
This "gfc_release_symbol" was actually the call which
handled our "bp" symbol – deleting n.sym->formal.
The parent namespace was still present and called
resolve_formal – which den caused the ICE as its
n.sym->formal was before deleted in "our" namespace.
Tobias
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch] Fortran: Avoid double-free with parse error (PR96041, PR93423)
2020-09-16 8:35 ` Tobias Burnus
@ 2020-09-17 11:45 ` Andre Vehreschild
0 siblings, 0 replies; 5+ messages in thread
From: Andre Vehreschild @ 2020-09-17 11:45 UTC (permalink / raw)
To: Tobias Burnus; +Cc: Tobias Burnus, gcc-patches, Harald Anlauf, fortran
Hi Tobias,
I see. Then OK for trunk by me.
- Andre
On Wed, 16 Sep 2020 10:35:53 +0200
Tobias Burnus <tobias@codesourcery.com> wrote:
> Hi Andre,
>
> On 9/16/20 9:58 AM, Andre Vehreschild wrote:
> > + st->n.sym = NULL;
> >
> > Don't we need free or unlink the st node from the symtree, too?
>
> I did not see a way to simply remove a single symtree; as this
> is the error case, I left the item in the symtree.
>
> The symtree itself is removed when the 'contains' is
> processed by calling:
> gfc_free_namespace (...)
> which calls
> free_sym_tree (ns->sym_root)
> and that function is:
> if (sym_tree == NULL)
> return;
> free_sym_tree (sym_tree->left);
> free_sym_tree (sym_tree->right);
> gfc_release_symbol (sym_tree->n.sym);
> free (sym_tree);
>
> This "gfc_release_symbol" was actually the call which
> handled our "bp" symbol – deleting n.sym->formal.
>
> The parent namespace was still present and called
> resolve_formal – which den caused the ICE as its
> n.sym->formal was before deleted in "our" namespace.
>
> Tobias
>
> -----------------
> Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung,
> Alexander Walter
--
Andre Vehreschild * Email: vehre ad gmx dot de
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-17 11:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-12 21:00 [Patch] Fortran: Avoid double-free with parse error (PR96041, PR93423) Tobias Burnus
2020-09-15 6:11 ` Early ping — " Tobias Burnus
2020-09-16 7:58 ` Andre Vehreschild
2020-09-16 8:35 ` Tobias Burnus
2020-09-17 11:45 ` Andre Vehreschild
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).