[PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks

[Bernhard Reutner-Fischer via Fortran]

From: Bernhard Reutner-Fischer 

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

Re: [PATCH 2/2] Fortran: Fix memory leak in gfc_add_include_path [PR68800]

[Bernhard Reutner-Fischer via Fortran]

ping?

libcpp maintainers, is the helper in incpath.* ok?

fortraners,
Do you prefer a rogue, local forward declaration, or is the
introduction of that trivial wrapper ok? I don't think pulling in cpp.h
from f95-lang.cc is desirable, but i can do that if you all think
that's preferred.

cover-letter:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583522.html

gcc/incpath.* bits (i guess that'd be for the libcpp maintainers):
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583520.html

fortran bits:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583521.html

thanks,

On Sun, 7 Nov 2021 02:38:21 +0100
Bernhard Reutner-Fischer  wrote:

> On Sat, 6 Nov 2021 20:22:53 +0100
> Harald Anlauf  wrote:
> 
> > Hi Bernhard,
> > 
> > I cannot comment on the gcc/ parts, but
> >   
> 
> > > diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
> > > index e86386c8b17..04fe8fe460b 100644
> > > --- a/gcc/fortran/cpp.c
> > > +++ b/gcc/fortran/cpp.c
> > > @@ -728,12 +728,20 @@ gfc_cpp_done (void)
> > > cpp_clear_file_cache (cpp_in);
> > >   }
> > 
> > why do you introduce a wrapper for something outside of fortran
> > that is used only once,
> >   
> > > -/* PATH must be malloc-ed and NULL-terminated.  */
> > > +/* Free all cpp include dirs.  */
> > > +void
> > > +gfc_cpp_free_cpp_dirs (void)
> > > +{
> > > +  free_cpp_dirs ();
> > > +}  
> 
> > > diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
> > > index 44644a2a333..963b9a9c89e 100644
> > > --- a/gcc/fortran/cpp.h
> > > +++ b/gcc/fortran/cpp.h
> > > @@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool);
> > >   bool gfc_cpp_preprocess (const char *source_file);
> > >
> > >   void gfc_cpp_done (void);
> > > +void gfc_cpp_free_cpp_dirs (void);
> > >
> > >   void gfc_cpp_add_include_path (char *path, bool user_supplied);
> > >   void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
> > > diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
> > > index 58dcaf01d75..ec4c2cf01d9 100644
> > > --- a/gcc/fortran/f95-lang.c
> > > +++ b/gcc/fortran/f95-lang.c
> > > @@ -275,7 +275,7 @@ gfc_finish (void)
> > > gfc_cpp_done ();
> > > gfc_done_1 ();
> > > gfc_release_include_path ();
> > > -  return;
> > 
> > namely here?
> >   
> > > +  gfc_cpp_free_cpp_dirs ();
> > >   }
> > 
> > Why not call free_cpp_dirs () here directly, omit all unnecessary
> > stuff, and maybe only add a brief comment here?  
> 
> cpp.c includes incpath.h, f95-lang.c does not and should not.
> So the cleanest thing is to keep the cpp handling in cpp.[ch] and have
> the language frontend call into it's cpp bits.
> 
> It would be rather rogue to
> extern void free_cpp_dirs (void);
> in f95-lang.c and directly call it in gfc_finish, i'd say?
> 
> thanks,