| Message ID | 20181021140434.30743-1-rep.dot.nop@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FORTRAN] Fix memory leak of gsymbol | expand |
ping [I'll rebase and retest this too since it's been a while. Ok if it passes?] On Sun, 21 Oct 2018 16:04:34 +0200 Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > Hi! > > Regtested on x86_64-unknown-linux, installing on > aldot/fortran-fe-stringpool. > > We did not free global symbols. For a simplified abstract_type_3.f03 > valgrind reports: > > 96 bytes in 1 blocks are still reachable in loss record 461 of 602 > at 0x48377D5: calloc (vg_replace_malloc.c:711) > by 0x21257C3: xcalloc (xmalloc.c:162) > by 0x98611B: gfc_get_gsymbol(char const*) (symbol.c:4341) > by 0x932C58: parse_module() (parse.c:5912) > by 0x9336F8: gfc_parse_file() (parse.c:6236) > by 0x991449: gfc_be_parse_file() (f95-lang.c:204) > by 0x11D8EDE: compile_file() (toplev.c:455) > by 0x11DB9C3: do_compile() (toplev.c:2170) > by 0x11DBCAF: toplev::main(int, char**) (toplev.c:2305) > by 0x2045D37: main (main.c:39) > > This patch reduces leaks to > > LEAK SUMMARY: > definitely lost: 344 bytes in 1 blocks > indirectly lost: 3,024 bytes in 4 blocks > possibly lost: 0 bytes in 0 blocks > - still reachable: 1,576,174 bytes in 2,277 blocks > + still reachable: 1,576,078 bytes in 2,276 blocks > suppressed: 0 bytes in 0 blocks > > gcc/fortran/ChangeLog: > > 2018-10-21 Bernhard Reutner-Fischer <aldot@gcc.gnu.org> > > * parse.c (clean_up_modules): Free gsym. > --- > gcc/fortran/parse.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c > index b7265c42f58..f7c369a17ac 100644 > --- a/gcc/fortran/parse.c > +++ b/gcc/fortran/parse.c > @@ -6066,7 +6066,7 @@ resolve_all_program_units (gfc_namespace *gfc_global_ns_list) > > > static void > -clean_up_modules (gfc_gsymbol *gsym) > +clean_up_modules (gfc_gsymbol *&gsym) > { > if (gsym == NULL) > return; > @@ -6074,14 +6074,18 @@ clean_up_modules (gfc_gsymbol *gsym) > clean_up_modules (gsym->left); > clean_up_modules (gsym->right); > > - if (gsym->type != GSYM_MODULE || !gsym->ns) > + if (gsym->type != GSYM_MODULE) > return; > > - gfc_current_ns = gsym->ns; > - gfc_derived_types = gfc_current_ns->derived_types; > - gfc_done_2 (); > - gsym->ns = NULL; > - return; > + if (gsym->ns) > + { > + gfc_current_ns = gsym->ns; > + gfc_derived_types = gfc_current_ns->derived_types; > + gfc_done_2 (); > + gsym->ns = NULL; > + } > + free (gsym); > + gsym = NULL; > } > >
Hi Bernhard, Am 27.10.21 um 23:43 schrieb Bernhard Reutner-Fischer via Gcc-patches: > ping > [I'll rebase and retest this too since it's been a while. > Ok if it passes?] > > On Sun, 21 Oct 2018 16:04:34 +0200 > Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > >> Hi! >> >> Regtested on x86_64-unknown-linux, installing on >> aldot/fortran-fe-stringpool. >> >> We did not free global symbols. For a simplified abstract_type_3.f03 >> valgrind reports: >> >> 96 bytes in 1 blocks are still reachable in loss record 461 of 602 >> at 0x48377D5: calloc (vg_replace_malloc.c:711) >> by 0x21257C3: xcalloc (xmalloc.c:162) >> by 0x98611B: gfc_get_gsymbol(char const*) (symbol.c:4341) >> by 0x932C58: parse_module() (parse.c:5912) >> by 0x9336F8: gfc_parse_file() (parse.c:6236) >> by 0x991449: gfc_be_parse_file() (f95-lang.c:204) >> by 0x11D8EDE: compile_file() (toplev.c:455) >> by 0x11DB9C3: do_compile() (toplev.c:2170) >> by 0x11DBCAF: toplev::main(int, char**) (toplev.c:2305) >> by 0x2045D37: main (main.c:39) >> >> This patch reduces leaks to >> >> LEAK SUMMARY: >> definitely lost: 344 bytes in 1 blocks >> indirectly lost: 3,024 bytes in 4 blocks >> possibly lost: 0 bytes in 0 blocks >> - still reachable: 1,576,174 bytes in 2,277 blocks >> + still reachable: 1,576,078 bytes in 2,276 blocks >> suppressed: 0 bytes in 0 blocks >> >> gcc/fortran/ChangeLog: >> >> 2018-10-21 Bernhard Reutner-Fischer <aldot@gcc.gnu.org> >> >> * parse.c (clean_up_modules): Free gsym. >> --- >> gcc/fortran/parse.c | 18 +++++++++++------- >> 1 file changed, 11 insertions(+), 7 deletions(-) >> >> diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c >> index b7265c42f58..f7c369a17ac 100644 >> --- a/gcc/fortran/parse.c >> +++ b/gcc/fortran/parse.c >> @@ -6066,7 +6066,7 @@ resolve_all_program_units (gfc_namespace *gfc_global_ns_list) >> >> >> static void >> -clean_up_modules (gfc_gsymbol *gsym) >> +clean_up_modules (gfc_gsymbol *&gsym) >> { >> if (gsym == NULL) >> return; >> @@ -6074,14 +6074,18 @@ clean_up_modules (gfc_gsymbol *gsym) >> clean_up_modules (gsym->left); >> clean_up_modules (gsym->right); >> >> - if (gsym->type != GSYM_MODULE || !gsym->ns) >> + if (gsym->type != GSYM_MODULE) >> return; >> >> - gfc_current_ns = gsym->ns; >> - gfc_derived_types = gfc_current_ns->derived_types; >> - gfc_done_2 (); >> - gsym->ns = NULL; >> - return; >> + if (gsym->ns) >> + { >> + gfc_current_ns = gsym->ns; >> + gfc_derived_types = gfc_current_ns->derived_types; >> + gfc_done_2 (); >> + gsym->ns = NULL; >> + } >> + free (gsym); >> + gsym = NULL; this essentially looks fine, but did you inspect the callers? With the change to the interface (*gsym -> *&gsym), it could have effects not visible here due to the explicit gsym = NULL. Assuming you checked that, and if it regtests fine, then it is OK for mainline. Thanks for the patch! Harald >> } >> >> > >
On Thu, 28 Oct 2021 23:37:59 +0200 Harald Anlauf <anlauf@gmx.de> wrote: > Hi Bernhard, > > Am 27.10.21 um 23:43 schrieb Bernhard Reutner-Fischer via Gcc-patches: > > ping > > [I'll rebase and retest this too since it's been a while. > > Ok if it passes?] > > > > On Sun, 21 Oct 2018 16:04:34 +0200 > > Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > >> gcc/fortran/ChangeLog: > >> > >> 2018-10-21 Bernhard Reutner-Fischer <aldot@gcc.gnu.org> > >> > >> * parse.c (clean_up_modules): Free gsym. > this essentially looks fine, but did you inspect the callers? > > With the change to the interface (*gsym -> *&gsym), it could have > effects not visible here due to the explicit gsym = NULL. > > Assuming you checked that, and if it regtests fine, then it is > OK for mainline. The only caller is translate_all_program_units. Since we free only module gsyms, even -fdump-fortran-global is unaffected by this, fwiw. It regtests cleanly and i will push it when the rest is approved. Thanks!
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c index b7265c42f58..f7c369a17ac 100644 --- a/gcc/fortran/parse.c +++ b/gcc/fortran/parse.c @@ -6066,7 +6066,7 @@ resolve_all_program_units (gfc_namespace *gfc_global_ns_list) static void -clean_up_modules (gfc_gsymbol *gsym) +clean_up_modules (gfc_gsymbol *&gsym) { if (gsym == NULL) return; @@ -6074,14 +6074,18 @@ clean_up_modules (gfc_gsymbol *gsym) clean_up_modules (gsym->left); clean_up_modules (gsym->right); - if (gsym->type != GSYM_MODULE || !gsym->ns) + if (gsym->type != GSYM_MODULE) return; - gfc_current_ns = gsym->ns; - gfc_derived_types = gfc_current_ns->derived_types; - gfc_done_2 (); - gsym->ns = NULL; - return; + if (gsym->ns) + { + gfc_current_ns = gsym->ns; + gfc_derived_types = gfc_current_ns->derived_types; + gfc_done_2 (); + gsym->ns = NULL; + } + free (gsym); + gsym = NULL; }
Hi! Regtested on x86_64-unknown-linux, installing on aldot/fortran-fe-stringpool. We did not free global symbols. For a simplified abstract_type_3.f03 valgrind reports: 96 bytes in 1 blocks are still reachable in loss record 461 of 602 at 0x48377D5: calloc (vg_replace_malloc.c:711) by 0x21257C3: xcalloc (xmalloc.c:162) by 0x98611B: gfc_get_gsymbol(char const*) (symbol.c:4341) by 0x932C58: parse_module() (parse.c:5912) by 0x9336F8: gfc_parse_file() (parse.c:6236) by 0x991449: gfc_be_parse_file() (f95-lang.c:204) by 0x11D8EDE: compile_file() (toplev.c:455) by 0x11DB9C3: do_compile() (toplev.c:2170) by 0x11DBCAF: toplev::main(int, char**) (toplev.c:2305) by 0x2045D37: main (main.c:39) This patch reduces leaks to LEAK SUMMARY: definitely lost: 344 bytes in 1 blocks indirectly lost: 3,024 bytes in 4 blocks possibly lost: 0 bytes in 0 blocks - still reachable: 1,576,174 bytes in 2,277 blocks + still reachable: 1,576,078 bytes in 2,276 blocks suppressed: 0 bytes in 0 blocks gcc/fortran/ChangeLog: 2018-10-21 Bernhard Reutner-Fischer <aldot@gcc.gnu.org> * parse.c (clean_up_modules): Free gsym. --- gcc/fortran/parse.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)