* [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks [not found] <nycvar.YFH.7.77.849.2303061129030.18795@jbgna.fhfr.qr> @ 2023-04-02 15:05 ` Bernhard Reutner-Fischer 2023-04-03 19:50 ` Harald Anlauf 0 siblings, 1 reply; 6+ messages in thread From: Bernhard Reutner-Fischer @ 2023-04-02 15:05 UTC (permalink / raw) To: gcc-patches; +Cc: Bernhard Reutner-Fischer, fortran From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org> Cc: fortran@gcc.gnu.org gcc/fortran/ChangeLog: * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing. * expr.cc (find_array_section): Fix mpz memory leak. * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in error paths. (gfc_simplify_set_exponent): Fix mpfr memory leak. --- gcc/fortran/array.cc | 3 +++ gcc/fortran/expr.cc | 8 ++++---- gcc/fortran/simplify.cc | 7 ++++++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc index be5eb8b6a0f..8b1e816a859 100644 --- a/gcc/fortran/array.cc +++ b/gcc/fortran/array.cc @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result, mpz_t *end) return t; default: + mpz_clear (lower); + mpz_clear (stride); + mpz_clear (upper); gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type"); } diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 7fb33f81788..b4736804eda 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) mpz_init_set_ui (delta_mpz, one); mpz_init_set_ui (nelts, one); mpz_init (tmp_mpz); + mpz_init (ptr); /* Do the initialization now, so that we can cleanup without keeping track of where we were. */ @@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) mpz_mul (delta_mpz, delta_mpz, tmp_mpz); } - mpz_init (ptr); cons = gfc_constructor_first (base); /* Now clock through the array reference, calculating the index in @@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) "at %L requires an increase of the allowed %d " "upper limit. See %<-fmax-array-constructor%> " "option", &expr->where, flag_max_array_constructor); - return false; + t = false; + goto cleanup; } cons = gfc_constructor_lookup (base, limit); @@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) gfc_copy_expr (cons->expr), NULL); } - mpz_clear (ptr); - cleanup: mpz_clear (delta_mpz); @@ -1765,6 +1764,7 @@ cleanup: mpz_clear (ctr[d]); mpz_clear (stride[d]); } + mpz_clear (ptr); gfc_constructor_free (base); return t; } diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index ecf0e3558df..d1f06335e79 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a " "negative value %d for dimension %d", &shape_exp->where, shape[rank], rank+1); + mpz_clear (index); return &gfc_bad_expr; } @@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, { gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different", &order_exp->where, &shape_exp->where); + mpz_clear (index); return &gfc_bad_expr; } @@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, { gfc_error ("Sizes of ORDER at %L and SHAPE at %L are different", &order_exp->where, &shape_exp->where); + mpz_clear (index); return &gfc_bad_expr; } @@ -6918,6 +6921,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, "in the range [1, ..., %d] for the RESHAPE intrinsic " "near %L", order[i], &order_exp->where, rank, &shape_exp->where); + mpz_clear (index); return &gfc_bad_expr; } @@ -6926,6 +6930,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, { gfc_error ("ORDER at %L is not a permutation of the size of " "SHAPE at %L", &order_exp->where, &shape_exp->where); + mpz_clear (index); return &gfc_bad_expr; } x[order[i]] = 1; @@ -7408,7 +7413,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) exp2 = (unsigned long) mpz_get_d (i->value.integer); mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE); - mpfr_clears (absv, log2, pow2, frac, NULL); + mpfr_clears (exp, absv, log2, pow2, frac, NULL); return range_check (result, "SET_EXPONENT"); } -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks 2023-04-02 15:05 ` [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks Bernhard Reutner-Fischer @ 2023-04-03 19:50 ` Harald Anlauf 2023-04-03 21:42 ` Bernhard Reutner-Fischer 0 siblings, 1 reply; 6+ messages in thread From: Harald Anlauf @ 2023-04-03 19:50 UTC (permalink / raw) To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Bernhard Reutner-Fischer, fortran Hi Bernhard, there is neither context nor a related PR with a testcase showing that this patch fixes issues seen there. On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote: > From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org> > > Cc: fortran@gcc.gnu.org > > gcc/fortran/ChangeLog: > > * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing. > * expr.cc (find_array_section): Fix mpz memory leak. > * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in > error paths. > (gfc_simplify_set_exponent): Fix mpfr memory leak. > --- > gcc/fortran/array.cc | 3 +++ > gcc/fortran/expr.cc | 8 ++++---- > gcc/fortran/simplify.cc | 7 ++++++- > 3 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc > index be5eb8b6a0f..8b1e816a859 100644 > --- a/gcc/fortran/array.cc > +++ b/gcc/fortran/array.cc > @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result, mpz_t *end) > return t; > > default: > + mpz_clear (lower); > + mpz_clear (stride); > + mpz_clear (upper); > gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type"); > } What is the point of clearing variables before issuing a gfc_internal_error? > diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc > index 7fb33f81788..b4736804eda 100644 > --- a/gcc/fortran/expr.cc > +++ b/gcc/fortran/expr.cc > @@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) > mpz_init_set_ui (delta_mpz, one); > mpz_init_set_ui (nelts, one); > mpz_init (tmp_mpz); > + mpz_init (ptr); > > /* Do the initialization now, so that we can cleanup without > keeping track of where we were. */ > @@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) > mpz_mul (delta_mpz, delta_mpz, tmp_mpz); > } > > - mpz_init (ptr); > cons = gfc_constructor_first (base); > > /* Now clock through the array reference, calculating the index in > @@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) > "at %L requires an increase of the allowed %d " > "upper limit. See %<-fmax-array-constructor%> " > "option", &expr->where, flag_max_array_constructor); > - return false; > + t = false; > + goto cleanup; > } > > cons = gfc_constructor_lookup (base, limit); > @@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) > gfc_copy_expr (cons->expr), NULL); > } > > - mpz_clear (ptr); > - > cleanup: > > mpz_clear (delta_mpz); > @@ -1765,6 +1764,7 @@ cleanup: > mpz_clear (ctr[d]); > mpz_clear (stride[d]); > } > + mpz_clear (ptr); > gfc_constructor_free (base); > return t; > } > diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc > index ecf0e3558df..d1f06335e79 100644 > --- a/gcc/fortran/simplify.cc > +++ b/gcc/fortran/simplify.cc > @@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a " > "negative value %d for dimension %d", > &shape_exp->where, shape[rank], rank+1); > + mpz_clear (index); > return &gfc_bad_expr; > } > > @@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > { > gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different", > &order_exp->where, &shape_exp->where); > + mpz_clear (index); > return &gfc_bad_expr; > } > > @@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > { > gfc_error ("Sizes of ORDER at %L and SHAPE at %L are different", > &order_exp->where, &shape_exp->where); > + mpz_clear (index); > return &gfc_bad_expr; > } > > @@ -6918,6 +6921,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > "in the range [1, ..., %d] for the RESHAPE intrinsic " > "near %L", order[i], &order_exp->where, rank, > &shape_exp->where); > + mpz_clear (index); > return &gfc_bad_expr; > } > > @@ -6926,6 +6930,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > { > gfc_error ("ORDER at %L is not a permutation of the size of " > "SHAPE at %L", &order_exp->where, &shape_exp->where); > + mpz_clear (index); > return &gfc_bad_expr; > } > x[order[i]] = 1; > @@ -7408,7 +7413,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) > exp2 = (unsigned long) mpz_get_d (i->value.integer); > mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE); > > - mpfr_clears (absv, log2, pow2, frac, NULL); > + mpfr_clears (exp, absv, log2, pow2, frac, NULL); > > return range_check (result, "SET_EXPONENT"); > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks 2023-04-03 19:50 ` Harald Anlauf @ 2023-04-03 21:42 ` Bernhard Reutner-Fischer 2023-04-17 19:47 ` ping " Bernhard Reutner-Fischer 0 siblings, 1 reply; 6+ messages in thread From: Bernhard Reutner-Fischer @ 2023-04-03 21:42 UTC (permalink / raw) To: Harald Anlauf, gcc-patches; +Cc: Bernhard Reutner-Fischer, fortran On 3 April 2023 21:50:49 CEST, Harald Anlauf <anlauf@gmx.de> wrote: >Hi Bernhard, > >there is neither context nor a related PR with a testcase showing >that this patch fixes issues seen there. Yes, i forgot to mention the PR: PR fortran/68800 I did not construct individual test cases but it should be obvious that we should not leak these. > >On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote: >> From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org> >> >> Cc: fortran@gcc.gnu.org >> >> gcc/fortran/ChangeLog: >> >> * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing. >> * expr.cc (find_array_section): Fix mpz memory leak. >> * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in >> error paths. >> (gfc_simplify_set_exponent): Fix mpfr memory leak. >> --- >> gcc/fortran/array.cc | 3 +++ >> gcc/fortran/expr.cc | 8 ++++---- >> gcc/fortran/simplify.cc | 7 ++++++- >> 3 files changed, 13 insertions(+), 5 deletions(-) >> >> diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc >> index be5eb8b6a0f..8b1e816a859 100644 >> --- a/gcc/fortran/array.cc >> +++ b/gcc/fortran/array.cc >> @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result, mpz_t *end) >> return t; >> >> default: >> + mpz_clear (lower); >> + mpz_clear (stride); >> + mpz_clear (upper); >> gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type"); >> } > >What is the point of clearing variables before issuing a gfc_internal_error? To make it obvious that we are aware that we allocated these. thanks, > >> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc >> index 7fb33f81788..b4736804eda 100644 >> --- a/gcc/fortran/expr.cc >> +++ b/gcc/fortran/expr.cc >> @@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) >> mpz_init_set_ui (delta_mpz, one); >> mpz_init_set_ui (nelts, one); >> mpz_init (tmp_mpz); >> + mpz_init (ptr); >> >> /* Do the initialization now, so that we can cleanup without >> keeping track of where we were. */ >> @@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) >> mpz_mul (delta_mpz, delta_mpz, tmp_mpz); >> } >> >> - mpz_init (ptr); >> cons = gfc_constructor_first (base); >> >> /* Now clock through the array reference, calculating the index in >> @@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) >> "at %L requires an increase of the allowed %d " >> "upper limit. See %<-fmax-array-constructor%> " >> "option", &expr->where, flag_max_array_constructor); >> - return false; >> + t = false; >> + goto cleanup; >> } >> >> cons = gfc_constructor_lookup (base, limit); >> @@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) >> gfc_copy_expr (cons->expr), NULL); >> } >> >> - mpz_clear (ptr); >> - >> cleanup: >> >> mpz_clear (delta_mpz); >> @@ -1765,6 +1764,7 @@ cleanup: >> mpz_clear (ctr[d]); >> mpz_clear (stride[d]); >> } >> + mpz_clear (ptr); >> gfc_constructor_free (base); >> return t; >> } >> diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc >> index ecf0e3558df..d1f06335e79 100644 >> --- a/gcc/fortran/simplify.cc >> +++ b/gcc/fortran/simplify.cc >> @@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, >> gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a " >> "negative value %d for dimension %d", >> &shape_exp->where, shape[rank], rank+1); >> + mpz_clear (index); >> return &gfc_bad_expr; >> } >> >> @@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, >> { >> gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different", >> &order_exp->where, &shape_exp->where); >> + mpz_clear (index); >> return &gfc_bad_expr; >> } >> >> @@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, >> { >> gfc_error ("Sizes of ORDER at %L and SHAPE at %L are different", >> &order_exp->where, &shape_exp->where); >> + mpz_clear (index); >> return &gfc_bad_expr; >> } >> >> @@ -6918,6 +6921,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, >> "in the range [1, ..., %d] for the RESHAPE intrinsic " >> "near %L", order[i], &order_exp->where, rank, >> &shape_exp->where); >> + mpz_clear (index); >> return &gfc_bad_expr; >> } >> >> @@ -6926,6 +6930,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, >> { >> gfc_error ("ORDER at %L is not a permutation of the size of " >> "SHAPE at %L", &order_exp->where, &shape_exp->where); >> + mpz_clear (index); >> return &gfc_bad_expr; >> } >> x[order[i]] = 1; >> @@ -7408,7 +7413,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) >> exp2 = (unsigned long) mpz_get_d (i->value.integer); >> mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE); >> >> - mpfr_clears (absv, log2, pow2, frac, NULL); >> + mpfr_clears (exp, absv, log2, pow2, frac, NULL); >> >> return range_check (result, "SET_EXPONENT"); >> } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* ping Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks 2023-04-03 21:42 ` Bernhard Reutner-Fischer @ 2023-04-17 19:47 ` Bernhard Reutner-Fischer 2023-04-17 22:18 ` Steve Kargl 0 siblings, 1 reply; 6+ messages in thread From: Bernhard Reutner-Fischer @ 2023-04-17 19:47 UTC (permalink / raw) To: gcc-patches, fortran; +Cc: rep.dot.nop, Harald Anlauf, Bernhard Reutner-Fischer Ping! Harald fixed the leak in set_exponent in the meantime. As stated in the cover-letter, it was bootstrapped and regtested without regression on x86_64-foo-linux. I consider it obvious, but never the less, OK for trunk (as in gcc-14) so far? thanks, On Mon, 03 Apr 2023 23:42:06 +0200 Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > On 3 April 2023 21:50:49 CEST, Harald Anlauf <anlauf@gmx.de> wrote: > >Hi Bernhard, > > > >there is neither context nor a related PR with a testcase showing > >that this patch fixes issues seen there. > > Yes, i forgot to mention the PR: > > PR fortran/68800 > > I did not construct individual test cases but it should be obvious that we should not leak these. > > > > >On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote: > >> From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org> > >> > >> Cc: fortran@gcc.gnu.org > >> > >> gcc/fortran/ChangeLog: > >> > >> * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing. > >> * expr.cc (find_array_section): Fix mpz memory leak. > >> * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in > >> error paths. > >> (gfc_simplify_set_exponent): Fix mpfr memory leak. > >> --- > >> gcc/fortran/array.cc | 3 +++ > >> gcc/fortran/expr.cc | 8 ++++---- > >> gcc/fortran/simplify.cc | 7 ++++++- > >> 3 files changed, 13 insertions(+), 5 deletions(-) > >> > >> diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc > >> index be5eb8b6a0f..8b1e816a859 100644 > >> --- a/gcc/fortran/array.cc > >> +++ b/gcc/fortran/array.cc > >> @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result, mpz_t *end) > >> return t; > >> > >> default: > >> + mpz_clear (lower); > >> + mpz_clear (stride); > >> + mpz_clear (upper); > >> gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type"); > >> } > > > >What is the point of clearing variables before issuing a gfc_internal_error? > > To make it obvious that we are aware that we allocated these. > > thanks, > > > >> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc > >> index 7fb33f81788..b4736804eda 100644 > >> --- a/gcc/fortran/expr.cc > >> +++ b/gcc/fortran/expr.cc > >> @@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) > >> mpz_init_set_ui (delta_mpz, one); > >> mpz_init_set_ui (nelts, one); > >> mpz_init (tmp_mpz); > >> + mpz_init (ptr); > >> > >> /* Do the initialization now, so that we can cleanup without > >> keeping track of where we were. */ > >> @@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) > >> mpz_mul (delta_mpz, delta_mpz, tmp_mpz); > >> } > >> > >> - mpz_init (ptr); > >> cons = gfc_constructor_first (base); > >> > >> /* Now clock through the array reference, calculating the index in > >> @@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) > >> "at %L requires an increase of the allowed %d " > >> "upper limit. See %<-fmax-array-constructor%> " > >> "option", &expr->where, flag_max_array_constructor); > >> - return false; > >> + t = false; > >> + goto cleanup; > >> } > >> > >> cons = gfc_constructor_lookup (base, limit); > >> @@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref) > >> gfc_copy_expr (cons->expr), NULL); > >> } > >> > >> - mpz_clear (ptr); > >> - > >> cleanup: > >> > >> mpz_clear (delta_mpz); > >> @@ -1765,6 +1764,7 @@ cleanup: > >> mpz_clear (ctr[d]); > >> mpz_clear (stride[d]); > >> } > >> + mpz_clear (ptr); > >> gfc_constructor_free (base); > >> return t; > >> } > >> diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc > >> index ecf0e3558df..d1f06335e79 100644 > >> --- a/gcc/fortran/simplify.cc > >> +++ b/gcc/fortran/simplify.cc > >> @@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > >> gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a " > >> "negative value %d for dimension %d", > >> &shape_exp->where, shape[rank], rank+1); > >> + mpz_clear (index); > >> return &gfc_bad_expr; > >> } > >> > >> @@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > >> { > >> gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different", > >> &order_exp->where, &shape_exp->where); > >> + mpz_clear (index); > >> return &gfc_bad_expr; > >> } > >> > >> @@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > >> { > >> gfc_error ("Sizes of ORDER at %L and SHAPE at %L are different", > >> &order_exp->where, &shape_exp->where); > >> + mpz_clear (index); > >> return &gfc_bad_expr; > >> } > >> > >> @@ -6918,6 +6921,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > >> "in the range [1, ..., %d] for the RESHAPE intrinsic " > >> "near %L", order[i], &order_exp->where, rank, > >> &shape_exp->where); > >> + mpz_clear (index); > >> return &gfc_bad_expr; > >> } > >> > >> @@ -6926,6 +6930,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr *shape_exp, > >> { > >> gfc_error ("ORDER at %L is not a permutation of the size of " > >> "SHAPE at %L", &order_exp->where, &shape_exp->where); > >> + mpz_clear (index); > >> return &gfc_bad_expr; > >> } > >> x[order[i]] = 1; > >> @@ -7408,7 +7413,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i) > >> exp2 = (unsigned long) mpz_get_d (i->value.integer); > >> mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE); > >> > >> - mpfr_clears (absv, log2, pow2, frac, NULL); > >> + mpfr_clears (exp, absv, log2, pow2, frac, NULL); > >> > >> return range_check (result, "SET_EXPONENT"); > >> } > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ping Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks 2023-04-17 19:47 ` ping " Bernhard Reutner-Fischer @ 2023-04-17 22:18 ` Steve Kargl 2023-05-08 6:00 ` Bernhard Reutner-Fischer 0 siblings, 1 reply; 6+ messages in thread From: Steve Kargl @ 2023-04-17 22:18 UTC (permalink / raw) To: Bernhard Reutner-Fischer via Fortran Cc: gcc-patches, rep.dot.nop, Harald Anlauf, Bernhard Reutner-Fischer On Mon, Apr 17, 2023 at 09:47:50PM +0200, Bernhard Reutner-Fischer via Fortran wrote: > Ping! > > Harald fixed the leak in set_exponent in the meantime. > As stated in the cover-letter, it was bootstrapped and regtested > without regression on x86_64-foo-linux. > > I consider it obvious, but never the less, OK for trunk (as in gcc-14) > so far? See below. > > On Mon, 03 Apr 2023 23:42:06 +0200 > Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > > > On 3 April 2023 21:50:49 CEST, Harald Anlauf <anlauf@gmx.de> wrote: > > >Hi Bernhard, > > > > > >there is neither context nor a related PR with a testcase showing > > >that this patch fixes issues seen there. > > > > Yes, i forgot to mention the PR: > > > > PR fortran/68800 > > > > I did not construct individual test cases but it should be obvious that we should not leak these. > > > > > > > >On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote: > > >> From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org> > > >> > > >> Cc: fortran@gcc.gnu.org > > >> > > >> gcc/fortran/ChangeLog: > > >> > > >> * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing. > > >> * expr.cc (find_array_section): Fix mpz memory leak. > > >> * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in > > >> error paths. > > >> (gfc_simplify_set_exponent): Fix mpfr memory leak. > > >> --- > > >> gcc/fortran/array.cc | 3 +++ > > >> gcc/fortran/expr.cc | 8 ++++---- > > >> gcc/fortran/simplify.cc | 7 ++++++- > > >> 3 files changed, 13 insertions(+), 5 deletions(-) > > >> > > >> diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc > > >> index be5eb8b6a0f..8b1e816a859 100644 > > >> --- a/gcc/fortran/array.cc > > >> +++ b/gcc/fortran/array.cc > > >> @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result, mpz_t *end) > > >> return t; > > >> > > >> default: > > >> + mpz_clear (lower); > > >> + mpz_clear (stride); > > >> + mpz_clear (upper); > > >> gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type"); > > >> } > > > > > > What is the point of clearing variables before issuing > > > a gfc_internal_error? > > > > To make it obvious that we are aware that we allocated these. I must admit I agree with Harald on this portion of the diff. What's the point? There is alot more allocated than just those 3 mzp_t variables when the internal error occurs. For example, gfortran does not walk the namespaces and free those before the ICE. I suppose silencing valgrind might be sufficient reason for the clutter. So, ok. > > >> &shape_exp->where, shape[rank], rank+1); > > >> + mpz_clear (index); > > >> return &gfc_bad_expr; > > >> } These types of changes are 'ok'. IIRC, gfortran will queue an error, and then forge on trying to match the code with other matchers. If successful, the error is tossed, so this would be a memory leak. -- Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ping Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks 2023-04-17 22:18 ` Steve Kargl @ 2023-05-08 6:00 ` Bernhard Reutner-Fischer 0 siblings, 0 replies; 6+ messages in thread From: Bernhard Reutner-Fischer @ 2023-05-08 6:00 UTC (permalink / raw) To: Steve Kargl Cc: Bernhard Reutner-Fischer via Fortran, gcc-patches, Harald Anlauf, Bernhard Reutner-Fischer On Mon, 17 Apr 2023 15:18:27 -0700 Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: > On Mon, Apr 17, 2023 at 09:47:50PM +0200, Bernhard Reutner-Fischer via Fortran wrote: > > On Mon, 03 Apr 2023 23:42:06 +0200 > > Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > > > > > On 3 April 2023 21:50:49 CEST, Harald Anlauf <anlauf@gmx.de> wrote: > > > >Hi Bernhard, > > > > > > > >there is neither context nor a related PR with a testcase showing > > > >that this patch fixes issues seen there. > > > > > > Yes, i forgot to mention the PR: > > > > > > PR fortran/68800 > > > > > > I did not construct individual test cases but it should be obvious that we should not leak these. > > > > > > > > > > >On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote: > > > >> From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org> > > > >> > > > >> Cc: fortran@gcc.gnu.org > > > >> > > > >> gcc/fortran/ChangeLog: > > > >> > > > >> * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing. > > > >> * expr.cc (find_array_section): Fix mpz memory leak. > > > >> * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in > > > >> error paths. > > > >> (gfc_simplify_set_exponent): Fix mpfr memory leak. > > > >> --- > > > >> gcc/fortran/array.cc | 3 +++ > > > >> gcc/fortran/expr.cc | 8 ++++---- > > > >> gcc/fortran/simplify.cc | 7 ++++++- > > > >> 3 files changed, 13 insertions(+), 5 deletions(-) > > > >> > > > >> diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc > > > >> index be5eb8b6a0f..8b1e816a859 100644 > > > >> --- a/gcc/fortran/array.cc > > > >> +++ b/gcc/fortran/array.cc > > > >> @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result, mpz_t *end) > > > >> return t; > > > >> > > > >> default: > > > >> + mpz_clear (lower); > > > >> + mpz_clear (stride); > > > >> + mpz_clear (upper); > > > >> gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type"); > > > >> } > > > > > > > > What is the point of clearing variables before issuing > > > > a gfc_internal_error? > > > > > > To make it obvious that we are aware that we allocated these. > > I must admit I agree with Harald on this portion > of the diff. What's the point? There is alot more > allocated than just those 3 mzp_t variables when the > internal error occurs. For example, gfortran does not > walk the namespaces and free those before the ICE. > I suppose silencing valgrind might be sufficient > reason for the clutter. So, ok. I've dropped this hunk and pushed the rest as r14-567-g2521390dd2f8e5 Harald fixed the leak of expr in gfc_simplify_set_exponent in the meantime. thanks, ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-08 6:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <nycvar.YFH.7.77.849.2303061129030.18795@jbgna.fhfr.qr>
2023-04-02 15:05 ` [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks Bernhard Reutner-Fischer
2023-04-03 19:50 ` Harald Anlauf
2023-04-03 21:42 ` Bernhard Reutner-Fischer
2023-04-17 19:47 ` ping " Bernhard Reutner-Fischer
2023-04-17 22:18 ` Steve Kargl
2023-05-08 6:00 ` Bernhard Reutner-Fischer
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).