[PATCH] [RFC] RAII auto_mpfr and autp_mpz

[PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Richard Biener]

The following adds two RAII classes, one for mpz_t and one for mpfr_t
making object lifetime management easier.  Both formerly require
explicit initialization with {mpz,mpfr}_init and release with
{mpz,mpfr}_clear.

I've converted two example places (where lifetime is trivial).

I've sofar only build cc1 with the change.  Any comments?

Thanks,
Richard.

	* system.h (class auto_mpz): New,
	* realmpfr.h (class auto_mpfr): Likewise.
	* fold-const-call.cc (do_mpfr_arg1): Use auto_mpfr.
	(do_mpfr_arg2): Likewise.
	* tree-ssa-loop-niter.cc (bound_difference): Use auto_mpz;
---
 gcc/fold-const-call.cc     |  8 ++------
 gcc/realmpfr.h             | 15 +++++++++++++++
 gcc/system.h               | 14 ++++++++++++++
 gcc/tree-ssa-loop-niter.cc | 10 +---------
 4 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/gcc/fold-const-call.cc b/gcc/fold-const-call.cc
index 43819c1f984..fa0b287cc8a 100644
--- a/gcc/fold-const-call.cc
+++ b/gcc/fold-const-call.cc
@@ -130,14 +130,12 @@ do_mpfr_arg1 (real_value *result,
 
   int prec = format->p;
   mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
-  mpfr_t m;
 
-  mpfr_init2 (m, prec);
+  auto_mpfr m (prec);
   mpfr_from_real (m, arg, MPFR_RNDN);
   mpfr_clear_flags ();
   bool inexact = func (m, m, rnd);
   bool ok = do_mpfr_ckconv (result, m, inexact, format);
-  mpfr_clear (m);
 
   return ok;
 }
@@ -224,14 +222,12 @@ do_mpfr_arg2 (real_value *result,
 
   int prec = format->p;
   mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
-  mpfr_t m;
 
-  mpfr_init2 (m, prec);
+  auto_mpfr m (prec);
   mpfr_from_real (m, arg1, MPFR_RNDN);
   mpfr_clear_flags ();
   bool inexact = func (m, arg0.to_shwi (), m, rnd);
   bool ok = do_mpfr_ckconv (result, m, inexact, format);
-  mpfr_clear (m);
 
   return ok;
 }
diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h
index 5e032c05f25..2db2ecc94d4 100644
--- a/gcc/realmpfr.h
+++ b/gcc/realmpfr.h
@@ -24,6 +24,21 @@
 #include <mpfr.h>
 #include <mpc.h>
 
+class auto_mpfr
+{
+public:
+  auto_mpfr () { mpfr_init (m_mpfr); }
+  explicit auto_mpfr (mpfr_prec_t prec) { mpfr_init2 (m_mpfr, prec); }
+  ~auto_mpfr () { mpfr_clear (m_mpfr); }
+
+  operator mpfr_t& () { return m_mpfr; }
+
+  auto_mpfr (const auto_mpfr &) = delete;
+
+private:
+  mpfr_t m_mpfr;
+};
+
 /* Convert between MPFR and REAL_VALUE_TYPE.  The caller is
    responsible for initializing and clearing the MPFR parameter.  */
 
diff --git a/gcc/system.h b/gcc/system.h
index 64cd5a49258..99f6c410481 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -701,6 +701,20 @@ extern int vsnprintf (char *, size_t, const char *, va_list);
 /* Do not introduce a gmp.h dependency on the build system.  */
 #ifndef GENERATOR_FILE
 #include <gmp.h>
+
+class auto_mpz
+{
+public:
+  auto_mpz () { mpz_init (m_mpz); }
+  ~auto_mpz () { mpz_clear (m_mpz); }
+
+  operator mpz_t& () { return m_mpz; }
+
+  auto_mpz (const auto_mpz &) = delete;
+
+private:
+  mpz_t m_mpz;
+};
 #endif
 
 /* Get libiberty declarations.  */
diff --git a/gcc/tree-ssa-loop-niter.cc b/gcc/tree-ssa-loop-niter.cc
index dc4c7a418f6..dcfba2fc7ae 100644
--- a/gcc/tree-ssa-loop-niter.cc
+++ b/gcc/tree-ssa-loop-niter.cc
@@ -722,7 +722,6 @@ bound_difference (class loop *loop, tree x, tree y, bounds *bnds)
   tree type = TREE_TYPE (x);
   tree varx, vary;
   mpz_t offx, offy;
-  mpz_t minx, maxx, miny, maxy;
   int cnt = 0;
   edge e;
   basic_block bb;
@@ -754,19 +753,12 @@ bound_difference (class loop *loop, tree x, tree y, bounds *bnds)
     {
       /* Otherwise, use the value ranges to determine the initial
 	 estimates on below and up.  */
-      mpz_init (minx);
-      mpz_init (maxx);
-      mpz_init (miny);
-      mpz_init (maxy);
+      auto_mpz minx, maxx, miny, maxy;
       determine_value_range (loop, type, varx, offx, minx, maxx);
       determine_value_range (loop, type, vary, offy, miny, maxy);
 
       mpz_sub (bnds->below, minx, maxy);
       mpz_sub (bnds->up, maxx, miny);
-      mpz_clear (minx);
-      mpz_clear (maxx);
-      mpz_clear (miny);
-      mpz_clear (maxy);
     }
 
   /* If both X and Y are constants, we cannot get any more precise.  */
-- 
2.35.3

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Jonathan Wakely]

On Mon, 6 Mar 2023 at 10:11, Richard Biener <rguenther@suse.de> wrote:
>
> The following adds two RAII classes, one for mpz_t and one for mpfr_t
> making object lifetime management easier.  Both formerly require
> explicit initialization with {mpz,mpfr}_init and release with
> {mpz,mpfr}_clear.
>
> I've converted two example places (where lifetime is trivial).
>
> I've sofar only build cc1 with the change.  Any comments?
>
> Thanks,
> Richard.
>
>         * system.h (class auto_mpz): New,
>         * realmpfr.h (class auto_mpfr): Likewise.
>         * fold-const-call.cc (do_mpfr_arg1): Use auto_mpfr.
>         (do_mpfr_arg2): Likewise.
>         * tree-ssa-loop-niter.cc (bound_difference): Use auto_mpz;
> ---
>  gcc/fold-const-call.cc     |  8 ++------
>  gcc/realmpfr.h             | 15 +++++++++++++++
>  gcc/system.h               | 14 ++++++++++++++
>  gcc/tree-ssa-loop-niter.cc | 10 +---------
>  4 files changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/gcc/fold-const-call.cc b/gcc/fold-const-call.cc
> index 43819c1f984..fa0b287cc8a 100644
> --- a/gcc/fold-const-call.cc
> +++ b/gcc/fold-const-call.cc
> @@ -130,14 +130,12 @@ do_mpfr_arg1 (real_value *result,
>
>    int prec = format->p;
>    mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
> -  mpfr_t m;
>
> -  mpfr_init2 (m, prec);
> +  auto_mpfr m (prec);
>    mpfr_from_real (m, arg, MPFR_RNDN);
>    mpfr_clear_flags ();
>    bool inexact = func (m, m, rnd);
>    bool ok = do_mpfr_ckconv (result, m, inexact, format);
> -  mpfr_clear (m);
>
>    return ok;
>  }
> @@ -224,14 +222,12 @@ do_mpfr_arg2 (real_value *result,
>
>    int prec = format->p;
>    mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
> -  mpfr_t m;
>
> -  mpfr_init2 (m, prec);
> +  auto_mpfr m (prec);
>    mpfr_from_real (m, arg1, MPFR_RNDN);
>    mpfr_clear_flags ();
>    bool inexact = func (m, arg0.to_shwi (), m, rnd);
>    bool ok = do_mpfr_ckconv (result, m, inexact, format);
> -  mpfr_clear (m);
>
>    return ok;
>  }
> diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h
> index 5e032c05f25..2db2ecc94d4 100644
> --- a/gcc/realmpfr.h
> +++ b/gcc/realmpfr.h
> @@ -24,6 +24,21 @@
>  #include <mpfr.h>
>  #include <mpc.h>
>
> +class auto_mpfr
> +{
> +public:
> +  auto_mpfr () { mpfr_init (m_mpfr); }
> +  explicit auto_mpfr (mpfr_prec_t prec) { mpfr_init2 (m_mpfr, prec); }
> +  ~auto_mpfr () { mpfr_clear (m_mpfr); }
> +
> +  operator mpfr_t& () { return m_mpfr; }


This implicit conversion makes the following mistake possible, if code
is incorrectly converted to use it:

auto_mpfr m (prec);
// ...
mpfr_clear (m);  // oops!

You could prevent that by adding this to the class body:

friend void mpfr_clear (auto_mpfr&) = delete;

This will be a better match for calls to mpfr_clear(m) than using the
implicit conversion then calling the real function, and will give an
error if used:
auto.cc:20:13: error: use of deleted function 'void mpfr_clear(auto_mpfr&)'

This deleted friend will not be a candidate for calls to mpfr_clear
with an argument of any other type, only for calls with an argument of
type auto_mpfr.

> +
> +  auto_mpfr (const auto_mpfr &) = delete;

This class has an implicit-defined assignment operator, which would
result in a leaks and double-frees.
You should add:
   auto_mpfr &operator=(const auto_mpfr &) = delete;
This ensures it can't becopied by construction or assignment.

The same two comments apply to auto_mpz.

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Richard Biener]

On Mon, 6 Mar 2023, Jonathan Wakely wrote:

> On Mon, 6 Mar 2023 at 10:11, Richard Biener <rguenther@suse.de> wrote:
> >
> > The following adds two RAII classes, one for mpz_t and one for mpfr_t
> > making object lifetime management easier.  Both formerly require
> > explicit initialization with {mpz,mpfr}_init and release with
> > {mpz,mpfr}_clear.
> >
> > I've converted two example places (where lifetime is trivial).
> >
> > I've sofar only build cc1 with the change.  Any comments?
> >
> > Thanks,
> > Richard.
> >
> >         * system.h (class auto_mpz): New,
> >         * realmpfr.h (class auto_mpfr): Likewise.
> >         * fold-const-call.cc (do_mpfr_arg1): Use auto_mpfr.
> >         (do_mpfr_arg2): Likewise.
> >         * tree-ssa-loop-niter.cc (bound_difference): Use auto_mpz;
> > ---
> >  gcc/fold-const-call.cc     |  8 ++------
> >  gcc/realmpfr.h             | 15 +++++++++++++++
> >  gcc/system.h               | 14 ++++++++++++++
> >  gcc/tree-ssa-loop-niter.cc | 10 +---------
> >  4 files changed, 32 insertions(+), 15 deletions(-)
> >
> > diff --git a/gcc/fold-const-call.cc b/gcc/fold-const-call.cc
> > index 43819c1f984..fa0b287cc8a 100644
> > --- a/gcc/fold-const-call.cc
> > +++ b/gcc/fold-const-call.cc
> > @@ -130,14 +130,12 @@ do_mpfr_arg1 (real_value *result,
> >
> >    int prec = format->p;
> >    mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
> > -  mpfr_t m;
> >
> > -  mpfr_init2 (m, prec);
> > +  auto_mpfr m (prec);
> >    mpfr_from_real (m, arg, MPFR_RNDN);
> >    mpfr_clear_flags ();
> >    bool inexact = func (m, m, rnd);
> >    bool ok = do_mpfr_ckconv (result, m, inexact, format);
> > -  mpfr_clear (m);
> >
> >    return ok;
> >  }
> > @@ -224,14 +222,12 @@ do_mpfr_arg2 (real_value *result,
> >
> >    int prec = format->p;
> >    mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
> > -  mpfr_t m;
> >
> > -  mpfr_init2 (m, prec);
> > +  auto_mpfr m (prec);
> >    mpfr_from_real (m, arg1, MPFR_RNDN);
> >    mpfr_clear_flags ();
> >    bool inexact = func (m, arg0.to_shwi (), m, rnd);
> >    bool ok = do_mpfr_ckconv (result, m, inexact, format);
> > -  mpfr_clear (m);
> >
> >    return ok;
> >  }
> > diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h
> > index 5e032c05f25..2db2ecc94d4 100644
> > --- a/gcc/realmpfr.h
> > +++ b/gcc/realmpfr.h
> > @@ -24,6 +24,21 @@
> >  #include <mpfr.h>
> >  #include <mpc.h>
> >
> > +class auto_mpfr
> > +{
> > +public:
> > +  auto_mpfr () { mpfr_init (m_mpfr); }
> > +  explicit auto_mpfr (mpfr_prec_t prec) { mpfr_init2 (m_mpfr, prec); }
> > +  ~auto_mpfr () { mpfr_clear (m_mpfr); }
> > +
> > +  operator mpfr_t& () { return m_mpfr; }
> 
> 
> This implicit conversion makes the following mistake possible, if code
> is incorrectly converted to use it:
> 
> auto_mpfr m (prec);
> // ...
> mpfr_clear (m);  // oops!
> 
> You could prevent that by adding this to the class body:
> 
> friend void mpfr_clear (auto_mpfr&) = delete;
> 
> This will be a better match for calls to mpfr_clear(m) than using the
> implicit conversion then calling the real function, and will give an
> error if used:
> auto.cc:20:13: error: use of deleted function 'void mpfr_clear(auto_mpfr&)'
>
> This deleted friend will not be a candidate for calls to mpfr_clear
> with an argument of any other type, only for calls with an argument of
> type auto_mpfr.

OK, it might be OK to mpfr_clear() twice and/or mpfr_clear/mpfr_init
again.  Quite possibly mpfr_init should get the same treatmen, mixing
auto_* with explicit lifetime management is bad.

> > +
> > +  auto_mpfr (const auto_mpfr &) = delete;
> 
> This class has an implicit-defined assignment operator, which would
> result in a leaks and double-frees.
> You should add:
>    auto_mpfr &operator=(const auto_mpfr &) = delete;
> This ensures it can't becopied by construction or assignment.
> 
> The same two comments apply to auto_mpz.

Thanks a lot, I've adjusted the patch to the one below.

Richard.

From c2736b929a3d0440432f31e65f5c89f4ec9dc21d Mon Sep 17 00:00:00 2001
From: Richard Biener <rguenther@suse.de>
Date: Mon, 6 Mar 2023 11:06:38 +0100
Subject: [PATCH] [RFC] RAII auto_mpfr and autp_mpz
To: gcc-patches@gcc.gnu.org

The following adds two RAII classes, one for mpz_t and one for mpfr_t
making object lifetime management easier.  Both formerly require
explicit initialization with {mpz,mpfr}_init and release with
{mpz,mpfr}_clear.

I've converted two example places (where lifetime is trivial).

	* system.h (class auto_mpz): New,
	* realmpfr.h (class auto_mpfr): Likewise.
	* fold-const-call.cc (do_mpfr_arg1): Use auto_mpfr.
	(do_mpfr_arg2): Likewise.
	* tree-ssa-loop-niter.cc (bound_difference): Use auto_mpz;
---
 gcc/fold-const-call.cc     |  8 ++------
 gcc/realmpfr.h             | 20 ++++++++++++++++++++
 gcc/system.h               | 18 ++++++++++++++++++
 gcc/tree-ssa-loop-niter.cc | 10 +---------
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/gcc/fold-const-call.cc b/gcc/fold-const-call.cc
index 43819c1f984..fa0b287cc8a 100644
--- a/gcc/fold-const-call.cc
+++ b/gcc/fold-const-call.cc
@@ -130,14 +130,12 @@ do_mpfr_arg1 (real_value *result,
 
   int prec = format->p;
   mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
-  mpfr_t m;
 
-  mpfr_init2 (m, prec);
+  auto_mpfr m (prec);
   mpfr_from_real (m, arg, MPFR_RNDN);
   mpfr_clear_flags ();
   bool inexact = func (m, m, rnd);
   bool ok = do_mpfr_ckconv (result, m, inexact, format);
-  mpfr_clear (m);
 
   return ok;
 }
@@ -224,14 +222,12 @@ do_mpfr_arg2 (real_value *result,
 
   int prec = format->p;
   mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
-  mpfr_t m;
 
-  mpfr_init2 (m, prec);
+  auto_mpfr m (prec);
   mpfr_from_real (m, arg1, MPFR_RNDN);
   mpfr_clear_flags ();
   bool inexact = func (m, arg0.to_shwi (), m, rnd);
   bool ok = do_mpfr_ckconv (result, m, inexact, format);
-  mpfr_clear (m);
 
   return ok;
 }
diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h
index 5e032c05f25..a91cd2b6321 100644
--- a/gcc/realmpfr.h
+++ b/gcc/realmpfr.h
@@ -24,6 +24,26 @@
 #include <mpfr.h>
 #include <mpc.h>
 
+class auto_mpfr
+{
+public:
+  auto_mpfr () { mpfr_init (m_mpfr); }
+  explicit auto_mpfr (mpfr_prec_t prec) { mpfr_init2 (m_mpfr, prec); }
+  ~auto_mpfr () { mpfr_clear (m_mpfr); }
+
+  operator mpfr_t& () { return m_mpfr; }
+
+  auto_mpfr (const auto_mpfr &) = delete;
+  auto_mpfr &operator=(const auto_mpfr &) = delete;
+
+  friend void mpfr_clear (auto_mpfr&) = delete;
+  friend void mpfr_init (auto_mpfr&) = delete;
+  friend void mpfr_init2 (auto_mpfr&, mpfr_prec_t) = delete;
+
+private:
+  mpfr_t m_mpfr;
+};
+
 /* Convert between MPFR and REAL_VALUE_TYPE.  The caller is
    responsible for initializing and clearing the MPFR parameter.  */
 
diff --git a/gcc/system.h b/gcc/system.h
index 64cd5a49258..b16c384df6d 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -701,6 +701,24 @@ extern int vsnprintf (char *, size_t, const char *, va_list);
 /* Do not introduce a gmp.h dependency on the build system.  */
 #ifndef GENERATOR_FILE
 #include <gmp.h>
+
+class auto_mpz
+{
+public:
+  auto_mpz () { mpz_init (m_mpz); }
+  ~auto_mpz () { mpz_clear (m_mpz); }
+
+  operator mpz_t& () { return m_mpz; }
+
+  auto_mpz (const auto_mpz &) = delete;
+  auto_mpz &operator=(const auto_mpz &) = delete;
+
+  friend void mpz_clear (auto_mpz&) = delete;
+  friend void mpz_init (auto_mpz&) = delete;
+
+private:
+  mpz_t m_mpz;
+};
 #endif
 
 /* Get libiberty declarations.  */
diff --git a/gcc/tree-ssa-loop-niter.cc b/gcc/tree-ssa-loop-niter.cc
index dc4c7a418f6..dcfba2fc7ae 100644
--- a/gcc/tree-ssa-loop-niter.cc
+++ b/gcc/tree-ssa-loop-niter.cc
@@ -722,7 +722,6 @@ bound_difference (class loop *loop, tree x, tree y, bounds *bnds)
   tree type = TREE_TYPE (x);
   tree varx, vary;
   mpz_t offx, offy;
-  mpz_t minx, maxx, miny, maxy;
   int cnt = 0;
   edge e;
   basic_block bb;
@@ -754,19 +753,12 @@ bound_difference (class loop *loop, tree x, tree y, bounds *bnds)
     {
       /* Otherwise, use the value ranges to determine the initial
 	 estimates on below and up.  */
-      mpz_init (minx);
-      mpz_init (maxx);
-      mpz_init (miny);
-      mpz_init (maxy);
+      auto_mpz minx, maxx, miny, maxy;
       determine_value_range (loop, type, varx, offx, minx, maxx);
       determine_value_range (loop, type, vary, offy, miny, maxy);
 
       mpz_sub (bnds->below, minx, maxy);
       mpz_sub (bnds->up, maxx, miny);
-      mpz_clear (minx);
-      mpz_clear (maxx);
-      mpz_clear (miny);
-      mpz_clear (maxy);
     }
 
   /* If both X and Y are constants, we cannot get any more precise.  */
-- 
2.35.3

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Jonathan Wakely]

On Mon, 6 Mar 2023 at 11:01, Richard Biener <rguenther@suse.de> wrote:
>
> On Mon, 6 Mar 2023, Jonathan Wakely wrote:
>
> > On Mon, 6 Mar 2023 at 10:11, Richard Biener <rguenther@suse.de> wrote:
> > >
> > > The following adds two RAII classes, one for mpz_t and one for mpfr_t
> > > making object lifetime management easier.  Both formerly require
> > > explicit initialization with {mpz,mpfr}_init and release with
> > > {mpz,mpfr}_clear.
> > >
> > > I've converted two example places (where lifetime is trivial).
> > >
> > > I've sofar only build cc1 with the change.  Any comments?
> > >
> > > Thanks,
> > > Richard.
> > >
> > >         * system.h (class auto_mpz): New,
> > >         * realmpfr.h (class auto_mpfr): Likewise.
> > >         * fold-const-call.cc (do_mpfr_arg1): Use auto_mpfr.
> > >         (do_mpfr_arg2): Likewise.
> > >         * tree-ssa-loop-niter.cc (bound_difference): Use auto_mpz;
> > > ---
> > >  gcc/fold-const-call.cc     |  8 ++------
> > >  gcc/realmpfr.h             | 15 +++++++++++++++
> > >  gcc/system.h               | 14 ++++++++++++++
> > >  gcc/tree-ssa-loop-niter.cc | 10 +---------
> > >  4 files changed, 32 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/gcc/fold-const-call.cc b/gcc/fold-const-call.cc
> > > index 43819c1f984..fa0b287cc8a 100644
> > > --- a/gcc/fold-const-call.cc
> > > +++ b/gcc/fold-const-call.cc
> > > @@ -130,14 +130,12 @@ do_mpfr_arg1 (real_value *result,
> > >
> > >    int prec = format->p;
> > >    mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
> > > -  mpfr_t m;
> > >
> > > -  mpfr_init2 (m, prec);
> > > +  auto_mpfr m (prec);
> > >    mpfr_from_real (m, arg, MPFR_RNDN);
> > >    mpfr_clear_flags ();
> > >    bool inexact = func (m, m, rnd);
> > >    bool ok = do_mpfr_ckconv (result, m, inexact, format);
> > > -  mpfr_clear (m);
> > >
> > >    return ok;
> > >  }
> > > @@ -224,14 +222,12 @@ do_mpfr_arg2 (real_value *result,
> > >
> > >    int prec = format->p;
> > >    mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
> > > -  mpfr_t m;
> > >
> > > -  mpfr_init2 (m, prec);
> > > +  auto_mpfr m (prec);
> > >    mpfr_from_real (m, arg1, MPFR_RNDN);
> > >    mpfr_clear_flags ();
> > >    bool inexact = func (m, arg0.to_shwi (), m, rnd);
> > >    bool ok = do_mpfr_ckconv (result, m, inexact, format);
> > > -  mpfr_clear (m);
> > >
> > >    return ok;
> > >  }
> > > diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h
> > > index 5e032c05f25..2db2ecc94d4 100644
> > > --- a/gcc/realmpfr.h
> > > +++ b/gcc/realmpfr.h
> > > @@ -24,6 +24,21 @@
> > >  #include <mpfr.h>
> > >  #include <mpc.h>
> > >
> > > +class auto_mpfr
> > > +{
> > > +public:
> > > +  auto_mpfr () { mpfr_init (m_mpfr); }
> > > +  explicit auto_mpfr (mpfr_prec_t prec) { mpfr_init2 (m_mpfr, prec); }
> > > +  ~auto_mpfr () { mpfr_clear (m_mpfr); }
> > > +
> > > +  operator mpfr_t& () { return m_mpfr; }
> >
> >
> > This implicit conversion makes the following mistake possible, if code
> > is incorrectly converted to use it:
> >
> > auto_mpfr m (prec);
> > // ...
> > mpfr_clear (m);  // oops!
> >
> > You could prevent that by adding this to the class body:
> >
> > friend void mpfr_clear (auto_mpfr&) = delete;
> >
> > This will be a better match for calls to mpfr_clear(m) than using the
> > implicit conversion then calling the real function, and will give an
> > error if used:
> > auto.cc:20:13: error: use of deleted function 'void mpfr_clear(auto_mpfr&)'
> >
> > This deleted friend will not be a candidate for calls to mpfr_clear
> > with an argument of any other type, only for calls with an argument of
> > type auto_mpfr.
>
> OK, it might be OK to mpfr_clear() twice and/or mpfr_clear/mpfr_init
> again.  Quite possibly mpfr_init should get the same treatmen, mixing
> auto_* with explicit lifetime management is bad.

Ah yes, good point.

> > > +
> > > +  auto_mpfr (const auto_mpfr &) = delete;
> >
> > This class has an implicit-defined assignment operator, which would
> > result in a leaks and double-frees.
> > You should add:
> >    auto_mpfr &operator=(const auto_mpfr &) = delete;
> > This ensures it can't becopied by construction or assignment.
> >
> > The same two comments apply to auto_mpz.
>
> Thanks a lot, I've adjusted the patch to the one below.

LGTM.

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Jakub Jelinek]

On Mon, Mar 06, 2023 at 11:01:18AM +0000, Richard Biener wrote:
> +  auto_mpfr &operator=(const auto_mpfr &) = delete;
> +  auto_mpz &operator=(const auto_mpz &) = delete;

Just formatting nit, space before (.

Looks like nice improvement and thanks Jonathan for the suggestions ;)

	Jakub

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Richard Biener]

On Mon, 6 Mar 2023, Jakub Jelinek wrote:

> On Mon, Mar 06, 2023 at 11:01:18AM +0000, Richard Biener wrote:
> > +  auto_mpfr &operator=(const auto_mpfr &) = delete;
> > +  auto_mpz &operator=(const auto_mpz &) = delete;
> 
> Just formatting nit, space before (.
> 
> Looks like nice improvement and thanks Jonathan for the suggestions ;)

Good, I've queued it for stage1 unless fortran folks want to pick it
up earlier for the purpose of fixing leaks.

Richard.

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Bernhard Reutner-Fischer]

[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]

On Mon, 6 Mar 2023 11:29:30 +0000 (UTC)
Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

> On Mon, 6 Mar 2023, Jakub Jelinek wrote:
> 
> > On Mon, Mar 06, 2023 at 11:01:18AM +0000, Richard Biener wrote:  
> > > +  auto_mpfr &operator=(const auto_mpfr &) = delete;
> > > +  auto_mpz &operator=(const auto_mpz &) = delete;  
> > 
> > Just formatting nit, space before (.
> > 
> > Looks like nice improvement and thanks Jonathan for the suggestions ;)  
> 
> Good, I've queued it for stage1 unless fortran folks want to pick it
> up earlier for the purpose of fixing leaks.

You did not Cc the fortran list though, not sure if Tobias is aware of
this possibility.

While it's a nice idea, there have been resentments towards (visible)
C++ in the fortran frontend and especially the library, i think.
I do not know if this has changed in the meantime.

PS: not sure about libgfortran/intrinsics/trigd.inc
FTYPE s
in #ifdef SIND_SMALL_LITERAL
when this is true:
if (mpfr_cmp_ld (ax, SIND_SMALL_LITERAL) < 0)
ISTM we leak s in the return x;

This seems to happen both in SIND and TAND, from the looks.

PPS: without handling (mpfr|mpz)_clears proper, hence missing quite
some potential spots, i guess one could potentially discuss at least
$ git diff | diffstat -ulp1
gcc/builtins.cc
gcc/fold-const-call.cc
gcc/fortran/arith.cc
gcc/fortran/array.cc
gcc/fortran/check.cc
gcc/fortran/data.cc
gcc/fortran/dependency.cc
gcc/fortran/expr.cc
gcc/fortran/resolve.cc
gcc/fortran/simplify.cc
gcc/fortran/target-memory.cc
gcc/fortran/trans-array.cc
gcc/fortran/trans-intrinsic.cc
gcc/real.cc
gcc/rust/backend/rust-compile-expr.cc
gcc/tree-ssa-loop-niter.cc
gcc/ubsan.cc

See the spots highlighted in the attached patch, which i did not try to
compile. I did not filter out the files you patched already, nor
externally maintained files like rust (which seems to leak fval and
ival not only on errors, from a quick look).

That was from
find ./ \( -name "*.c[cp]" -o -name "*.[ch]" -o -name "*.inc" \) -a \( ! -path "./gcc/testsuite/*" -a ! -path "./gcc/contrib/*" \) -exec spatch --sp-file ~/coccinelle/auto_mpfrz.2.cocci --in-place {} \;

thanks,

[-- Attachment #2: auto_mpfrz.2.cocci.txt --]
[-- Type: text/plain, Size: 969 bytes --]

// mpz and mpfr -> auto_\1
// Remember to s/= [[:space:]]*XXXZXXX//g
// to get rid of the disguise-in-c hack
// TODO: properly handle (mpfr|mpz)_clears
@ mpfr_1 @
typedef mpfr_t;
identifier i;
@@
- mpfr_t i;
+ auto_mpfr i;
...
- mpfr_init (i);
...
(
- mpfr_clear (i);
|
mpfr_clears (
 ...,
- i,
 ...
 );
)

@ mpfr_2 @
typedef mpfr_t;
identifier i;
expression prec;
@@
- mpfr_t i;
+ auto_mpfr i = XXXZXXX(prec);
...
- mpfr_init2 (i, prec);
...
(
- mpfr_clear (i);
|
mpfr_clears (
 ...,
- i,
 ...
 );
)

@ mpfr_3 @
@@
- mpfr_clears(NULL);

/// mpz ///////////////////////////////////////////////////

@ mpz_1 @
typedef mpz_t;
identifier i;
@@
- mpz_t i;
+ auto_mpz i;
...
- mpz_init (i);
...
(
- mpz_clear (i);
|
mpz_clears (
 ...,
- i,
 ...
 );
)

@ mpz_2 @
typedef mpz_t;
identifier i;
expression prec;
@@
- mpz_t i;
+ auto_mpz i = XXXZXXX(prec);
...
- mpz_init2 (i, prec);
...
(
- mpz_clear (i);
|
mpz_clears (
 ...,
- i,
 ...
 );
)

@ mpz_3 @
@@
- mpz_clears(NULL);


[-- Attachment #3: 00001-auto_mpfr_mpz.00.patch.txt --]
[-- Type: text/plain, Size: 28072 bytes --]

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 4d467c8c5c1..9be0949aa1d 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -11064,15 +11064,13 @@ do_mpfr_lgamma_r (tree arg, tree arg_sg, tree type)
 	  const int prec = fmt->p;
 	  const mpfr_rnd_t rnd = fmt->round_towards_zero? MPFR_RNDZ : MPFR_RNDN;
 	  int inexact, sg;
-	  mpfr_t m;
+	  auto_mpfr m (prec);
 	  tree result_lg;
 
-	  mpfr_init2 (m, prec);
 	  mpfr_from_real (m, ra, MPFR_RNDN);
 	  mpfr_clear_flags ();
 	  inexact = mpfr_lgamma (m, &sg, m, rnd);
 	  result_lg = do_mpfr_ckconv (m, type, inexact);
-	  mpfr_clear (m);
 	  if (result_lg)
 	    {
 	      tree result_sg;
diff --git a/gcc/fold-const-call.cc b/gcc/fold-const-call.cc
index 43819c1f984..900c4069ddb 100644
--- a/gcc/fold-const-call.cc
+++ b/gcc/fold-const-call.cc
@@ -130,14 +130,13 @@ do_mpfr_arg1 (real_value *result,
 
   int prec = format->p;
   mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
-  mpfr_t m;
+  auto_mpfr m (prec);
 
-  mpfr_init2 (m, prec);
+  
   mpfr_from_real (m, arg, MPFR_RNDN);
   mpfr_clear_flags ();
   bool inexact = func (m, m, rnd);
   bool ok = do_mpfr_ckconv (result, m, inexact, format);
-  mpfr_clear (m);
 
   return ok;
 }
@@ -224,14 +223,13 @@ do_mpfr_arg2 (real_value *result,
 
   int prec = format->p;
   mpfr_rnd_t rnd = format->round_towards_zero ? MPFR_RNDZ : MPFR_RNDN;
-  mpfr_t m;
+  auto_mpfr m (prec);
 
-  mpfr_init2 (m, prec);
+  
   mpfr_from_real (m, arg1, MPFR_RNDN);
   mpfr_clear_flags ();
   bool inexact = func (m, arg0.to_shwi (), m, rnd);
   bool ok = do_mpfr_ckconv (result, m, inexact, format);
-  mpfr_clear (m);
 
   return ok;
 }
diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index d0d1c0b03d2..5b14a833d9a 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -329,13 +329,12 @@ static arith
 gfc_check_real_range (mpfr_t p, int kind)
 {
   arith retval;
-  mpfr_t q;
+  auto_mpfr q;
   int i;
 
   i = gfc_validate_kind (BT_REAL, kind, false);
 
   gfc_set_model (p);
-  mpfr_init (q);
   mpfr_abs (q, p, GFC_RND_MODE);
 
   retval = ARITH_OK;
@@ -352,7 +351,6 @@ gfc_check_real_range (mpfr_t p, int kind)
     }
   else if (mpfr_sgn (q) == 0)
     {
-      mpfr_clear (q);
       return retval;
     }
   else if (mpfr_cmp (q, gfc_real_kinds[i].huge) > 0)
@@ -405,8 +403,6 @@ gfc_check_real_range (mpfr_t p, int kind)
 	mpfr_set (p, q, MPFR_RNDN);
     }
 
-  mpfr_clear (q);
-
   return retval;
 }
 
@@ -769,8 +765,8 @@ gfc_arith_divide (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
 
       if (warn_integer_division)
 	{
-	  mpz_t r;
-	  mpz_init (r);
+	  auto_mpz r;
+	  
 	  mpz_tdiv_qr (result->value.integer, r, op1->value.integer,
 		       op2->value.integer);
 
@@ -783,7 +779,6 @@ gfc_arith_divide (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
 			       &op1->where);
 	      free (p);
 	    }
-	  mpz_clear (r);
 	}
       else
 	mpz_tdiv_q (result->value.integer, op1->value.integer,
@@ -2093,12 +2088,11 @@ static bool
 wprecision_int_real (mpz_t n, mpfr_t r)
 {
   bool ret;
-  mpz_t i;
-  mpz_init (i);
+  auto_mpz i;
+  
   mpfr_get_z (i, r, GFC_RND_MODE);
   mpz_sub (i, i, n);
   ret = mpz_cmp_si (i, 0) != 0;
-  mpz_clear (i);
   return ret;
 }
 
@@ -2425,9 +2419,9 @@ gfc_complex2int (gfc_expr *src, int kind)
 	}
 
       else {
-	mpfr_t f;
+	auto_mpfr f;
 
-	mpfr_init (f);
+	
 	mpfr_frac (f, src->value.real, GFC_RND_MODE);
 	if (mpfr_cmp_si (f, 0) != 0)
 	  {
@@ -2436,7 +2430,6 @@ gfc_complex2int (gfc_expr *src, int kind)
 			     gfc_typename (&result->ts), &src->where);
 	    did_warn = true;
 	  }
-	mpfr_clear (f);
       }
 
       if (!did_warn && warn_conversion_extra)
diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index be5eb8b6a0f..49bd296a6ac 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -1722,14 +1722,13 @@ expand_iterator (gfc_constructor *c)
 {
   gfc_expr *start, *end, *step;
   iterator_stack frame;
-  mpz_t trip;
+  auto_mpz trip;
   bool t;
 
   end = step = NULL;
 
   t = false;
 
-  mpz_init (trip);
   mpz_init (frame.value);
   frame.prev = NULL;
 
@@ -1787,7 +1786,6 @@ cleanup:
   gfc_free_expr (end);
   gfc_free_expr (step);
 
-  mpz_clear (trip);
   mpz_clear (frame.value);
 
   iter_stack = frame.prev;
diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
index 8c1ae8c2f00..f07dfadbc36 100644
--- a/gcc/fortran/check.cc
+++ b/gcc/fortran/check.cc
@@ -211,7 +211,7 @@ bin2real (gfc_expr *x, int kind)
   char buf[114], *sp;
   int b, i, ie, t, w;
   bool sgn;
-  mpz_t em;
+  auto_mpz em;
 
   i = gfc_validate_kind (BT_REAL, kind, false);
   t = gfc_real_kinds[i].digits - 1;
@@ -237,7 +237,6 @@ bin2real (gfc_expr *x, int kind)
   /* Extract biased exponent. */
   memset (buf, 0, 114);
   strncpy (buf, ++sp, w);
-  mpz_init (em);
   mpz_set_str (em, buf, 2);
   ie = mpz_get_si (em);
 
@@ -284,8 +283,6 @@ bin2real (gfc_expr *x, int kind)
     }
 
    if (sgn) mpfr_neg (x->value.real, x->value.real, GFC_RND_MODE);
-
-   mpz_clear (em);
 }
 
 
@@ -388,7 +385,7 @@ gfc_boz2int (gfc_expr *x, int kind)
 {
   int i, len;
   char *buf, *str;
-  mpz_t tmp1;
+  auto_mpz tmp1;
 
   if (!is_boz_constant (x))
     return false;
@@ -437,18 +434,16 @@ gfc_boz2int (gfc_expr *x, int kind)
     }
 
   /* Convert as-if unsigned integer.  */
-  mpz_init (tmp1);
   mpz_set_str (tmp1, buf, x->boz.rdx);
 
   /* Check for wrap-around.  */
   if (mpz_cmp (tmp1, gfc_integer_kinds[i].huge) > 0)
     {
-      mpz_t tmp2;
-      mpz_init (tmp2);
+      auto_mpz tmp2;
+      
       mpz_add_ui (tmp2, gfc_integer_kinds[i].huge, 1);
       mpz_mod (tmp1, tmp1, tmp2);
       mpz_sub (tmp1, tmp1, tmp2);
-      mpz_clear (tmp2);
     }
 
   /* Clear boz info.  */
@@ -460,7 +455,6 @@ gfc_boz2int (gfc_expr *x, int kind)
   mpz_set (x->value.integer, tmp1);
   x->ts.type = BT_INTEGER;
   x->ts.kind = kind;
-  mpz_clear (tmp1);
 
   return true;
 }
diff --git a/gcc/fortran/data.cc b/gcc/fortran/data.cc
index d29eb12c1b1..5c88e34e3ff 100644
--- a/gcc/fortran/data.cc
+++ b/gcc/fortran/data.cc
@@ -49,9 +49,9 @@ get_array_index (gfc_array_ref *ar, mpz_t *offset)
   gfc_expr *e;
   int i;
   mpz_t delta;
-  mpz_t tmp;
+  auto_mpz tmp;
 
-  mpz_init (tmp);
+  
   mpz_set_si (*offset, 0);
   mpz_init_set_si (delta, 1);
   for (i = 0; i < ar->dimen; i++)
@@ -76,7 +76,6 @@ get_array_index (gfc_array_ref *ar, mpz_t *offset)
       mpz_mul (delta, tmp, delta);
     }
   mpz_clear (delta);
-  mpz_clear (tmp);
 }
 
 /* Find if there is a constructor which component is equal to COM.
@@ -638,7 +637,7 @@ gfc_advance_section (mpz_t *section_index, gfc_array_ref *ar,
 {
   int i;
   mpz_t delta;
-  mpz_t tmp;
+  auto_mpz tmp;
   bool forwards;
   int cmp;
   gfc_expr *start, *end, *stride;
@@ -698,7 +697,6 @@ gfc_advance_section (mpz_t *section_index, gfc_array_ref *ar,
 
   mpz_set_si (*offset_ret, 0);
   mpz_init_set_si (delta, 1);
-  mpz_init (tmp);
   for (i = 0; i < ar->dimen; i++)
     {
       mpz_sub (tmp, section_index[i], ar->as->lower[i]->value.integer);
@@ -710,7 +708,6 @@ gfc_advance_section (mpz_t *section_index, gfc_array_ref *ar,
       mpz_add_ui (tmp, tmp, 1);
       mpz_mul (delta, tmp, delta);
     }
-  mpz_clear (tmp);
   mpz_clear (delta);
 }
 
@@ -797,11 +794,10 @@ gfc_get_section_index (gfc_array_ref *ar, mpz_t *section_index, mpz_t *offset)
 {
   int i;
   mpz_t delta;
-  mpz_t tmp;
+  auto_mpz tmp;
   gfc_expr *start;
 
   mpz_set_si (*offset, 0);
-  mpz_init (tmp);
   mpz_init_set_si (delta, 1);
   for (i = 0; i < ar->dimen; i++)
     {
@@ -839,7 +835,6 @@ gfc_get_section_index (gfc_array_ref *ar, mpz_t *section_index, mpz_t *offset)
       mpz_mul (delta, tmp, delta);
     }
 
-  mpz_clear (tmp);
   mpz_clear (delta);
 }
 
diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
index 9117825ee6e..0d0d782f67a 100644
--- a/gcc/fortran/dependency.cc
+++ b/gcc/fortran/dependency.cc
@@ -1576,16 +1576,14 @@ check_section_vs_section (gfc_array_ref *l_ar, gfc_array_ref *r_ar, int n)
   if (IS_CONSTANT_INTEGER (l_stride) && IS_CONSTANT_INTEGER (r_stride)
       && gfc_dep_difference (l_start, r_start, &tmp))
     {
-      mpz_t gcd;
+      auto_mpz gcd;
       int result;
 
-      mpz_init (gcd);
       mpz_gcd (gcd, l_stride->value.integer, r_stride->value.integer);
 
       mpz_fdiv_r (tmp, tmp, gcd);
       result = mpz_cmp_si (tmp, 0L);
 
-      mpz_clear (gcd);
       mpz_clear (tmp);
 
       if (result != 0)
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index c295721b9d6..7695710cf6b 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -1354,10 +1354,10 @@ find_array_element (gfc_constructor_base base, gfc_array_ref *ar,
 {
   unsigned long nelemen;
   int i;
-  mpz_t delta;
+  auto_mpz delta;
   mpz_t offset;
   mpz_t span;
-  mpz_t tmp;
+  auto_mpz tmp;
   gfc_constructor *cons;
   gfc_expr *e;
   bool t;
@@ -1366,8 +1366,6 @@ find_array_element (gfc_constructor_base base, gfc_array_ref *ar,
   e = NULL;
 
   mpz_init_set_ui (offset, 0);
-  mpz_init (delta);
-  mpz_init (tmp);
   mpz_init_set_ui (span, 1);
   for (i = 0; i < ar->dimen; i++)
     {
@@ -1423,10 +1421,8 @@ find_array_element (gfc_constructor_base base, gfc_array_ref *ar,
     }
 
 depart:
-  mpz_clear (delta);
   mpz_clear (offset);
   mpz_clear (span);
-  mpz_clear (tmp);
   *rval = cons;
   return t;
 }
@@ -1503,9 +1499,9 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
   mpz_t delta[GFC_MAX_DIMENSIONS];
   mpz_t ctr[GFC_MAX_DIMENSIONS];
   mpz_t delta_mpz;
-  mpz_t tmp_mpz;
+  auto_mpz tmp_mpz;
   mpz_t nelts;
-  mpz_t ptr;
+  auto_mpz ptr;
   gfc_constructor_base base;
   gfc_constructor *cons, *vecsub[GFC_MAX_DIMENSIONS];
   gfc_expr *begin;
@@ -1527,7 +1523,6 @@ 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);
 
   /* Do the initialization now, so that we can cleanup without
      keeping track of where we were.  */
@@ -1671,7 +1666,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
@@ -1739,12 +1733,9 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
 				   gfc_copy_expr (cons->expr), NULL);
     }
 
-  mpz_clear (ptr);
-
 cleanup:
 
   mpz_clear (delta_mpz);
-  mpz_clear (tmp_mpz);
   mpz_clear (nelts);
   for (d = 0; d < rank; d++)
     {
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index fb0745927ac..f18e8058b6e 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1480,8 +1480,8 @@ resolve_structure_cons (gfc_expr *expr, int init)
 	  && comp->as && !comp->attr.allocatable && !comp->attr.pointer
 	  && !comp->attr.pdt_array)
 	{
-	  mpz_t len;
-	  mpz_init (len);
+	  auto_mpz len;
+	  
 	  for (int n = 0; n < rank; n++)
 	    {
 	      if (comp->as->upper[n]->expr_type != EXPR_CONSTANT
@@ -1509,7 +1509,6 @@ resolve_structure_cons (gfc_expr *expr, int init)
 		  t = false;
 		}
 	    }
-	  mpz_clear (len);
 	}
 
       if (!comp->attr.pointer || comp->attr.proc_pointer
@@ -4628,7 +4627,7 @@ static int
 compute_last_value_for_triplet (gfc_expr *start, gfc_expr *end,
 				gfc_expr *stride, mpz_t last)
 {
-  mpz_t rem;
+  auto_mpz rem;
 
   if (start == NULL || start->expr_type != EXPR_CONSTANT
       || end == NULL || end->expr_type != EXPR_CONSTANT
@@ -4660,11 +4659,9 @@ compute_last_value_for_triplet (gfc_expr *start, gfc_expr *end,
 	return 0;
     }
 
-  mpz_init (rem);
   mpz_sub (rem, end->value.integer, start->value.integer);
   mpz_tdiv_r (rem, rem, stride->value.integer);
   mpz_sub (last, end->value.integer, rem);
-  mpz_clear (rem);
 
   return 1;
 }
@@ -7754,9 +7751,7 @@ conformable_arrays (gfc_expr *e1, gfc_expr *e2)
   if (e1->shape)
     {
       int i;
-      mpz_t s;
-
-      mpz_init (s);
+      auto_mpz s;
 
       for (i = 0; i < e1->rank; i++)
 	{
@@ -7778,12 +7773,9 @@ conformable_arrays (gfc_expr *e1, gfc_expr *e2)
 	    {
 	      gfc_error ("Source-expr at %L and allocate-object at %L must "
 			 "have the same shape", &e1->where, &e2->where);
-	      mpz_clear (s);
    	      return false;
 	    }
 	}
-
-      mpz_clear (s);
     }
 
   return true;
@@ -16601,13 +16593,12 @@ static bool traverse_data_var (gfc_data_variable *, locus *);
 static bool
 traverse_data_list (gfc_data_variable *var, locus *where)
 {
-  mpz_t trip;
+  auto_mpz trip;
   iterator_stack frame;
   gfc_expr *e, *start, *end, *step;
   bool retval = true;
 
   mpz_init (frame.value);
-  mpz_init (trip);
 
   start = gfc_copy_expr (var->iter.start);
   end = gfc_copy_expr (var->iter.end);
@@ -16680,7 +16671,6 @@ traverse_data_list (gfc_data_variable *var, locus *where)
 
 cleanup:
   mpz_clear (frame.value);
-  mpz_clear (trip);
 
   gfc_free_expr (start);
   gfc_free_expr (end);
diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
index 20ea38e0007..3a6316c00a9 100644
--- a/gcc/fortran/simplify.cc
+++ b/gcc/fortran/simplify.cc
@@ -1157,13 +1157,12 @@ gfc_simplify_asin (gfc_expr *x)
 static void
 rad2deg (mpfr_t x)
 {
-  mpfr_t tmp;
+  auto_mpfr tmp;
 
-  mpfr_init (tmp);
+  
   mpfr_const_pi (tmp, GFC_RND_MODE);
   mpfr_mul_ui (x, x, 180, GFC_RND_MODE);
   mpfr_div (x, x, tmp, GFC_RND_MODE);
-  mpfr_clear (tmp);
 }
 
 
@@ -1927,13 +1926,12 @@ gfc_simplify_cos (gfc_expr *x)
 static void
 deg2rad (mpfr_t x)
 {
-  mpfr_t d2r;
+  auto_mpfr d2r;
 
-  mpfr_init (d2r);
+  
   mpfr_const_pi (d2r, GFC_RND_MODE);
   mpfr_div_ui (d2r, d2r, 180, GFC_RND_MODE);
   mpfr_mul (x, x, d2r, GFC_RND_MODE);
-  mpfr_clear (d2r);
 }
 
 
@@ -2835,7 +2833,7 @@ static void
 asympt_erfc_scaled (mpfr_t res, mpfr_t arg)
 {
   mpfr_t sum, x, u, v, w, oldsum, sumtrunc;
-  mpz_t num;
+  auto_mpz num;
   mpfr_prec_t prec;
   unsigned i;
 
@@ -2847,7 +2845,6 @@ asympt_erfc_scaled (mpfr_t res, mpfr_t arg)
   mpfr_init (u);
   mpfr_init (v);
   mpfr_init (w);
-  mpz_init (num);
 
   mpfr_init (oldsum);
   mpfr_init (sumtrunc);
@@ -2897,7 +2894,6 @@ asympt_erfc_scaled (mpfr_t res, mpfr_t arg)
   mpfr_set_default_prec (prec);
 
   mpfr_clears (sum, x, u, v, w, oldsum, sumtrunc, NULL);
-  mpz_clear (num);
 }
 
 
@@ -3172,7 +3168,7 @@ gfc_expr *
 gfc_simplify_floor (gfc_expr *e, gfc_expr *k)
 {
   gfc_expr *result;
-  mpfr_t floor;
+  auto_mpfr floor (mpfr_get_prec(e->value.real));
   int kind;
 
   kind = get_kind (BT_INTEGER, k, "FLOOR", gfc_default_integer_kind);
@@ -3182,14 +3178,11 @@ gfc_simplify_floor (gfc_expr *e, gfc_expr *k)
   if (e->expr_type != EXPR_CONSTANT)
     return NULL;
 
-  mpfr_init2 (floor, mpfr_get_prec (e->value.real));
   mpfr_floor (floor, e->value.real);
 
   result = gfc_get_constant_expr (BT_INTEGER, kind, &e->where);
   gfc_mpfr_to_mpz (result->value.integer, floor, &e->where);
 
-  mpfr_clear (floor);
-
   return range_check (result, "FLOOR");
 }
 
@@ -6199,7 +6192,7 @@ static int norm2_scale;
 static gfc_expr *
 norm2_add_squared (gfc_expr *result, gfc_expr *e)
 {
-  mpfr_t tmp;
+  auto_mpfr tmp;
 
   gcc_assert (e->ts.type == BT_REAL && e->expr_type == EXPR_CONSTANT);
   gcc_assert (result->ts.type == BT_REAL
@@ -6221,7 +6214,6 @@ norm2_add_squared (gfc_expr *result, gfc_expr *e)
 	}
     }
 
-  mpfr_init (tmp);
   if (mpfr_regular_p (e->value.real))
     {
       exp = mpfr_get_exp (e->value.real);
@@ -6247,7 +6239,6 @@ norm2_add_squared (gfc_expr *result, gfc_expr *e)
   mpfr_pow_ui (tmp, tmp, 2, GFC_RND_MODE);
   mpfr_add (result->value.real, result->value.real, tmp,
 	    GFC_RND_MODE);
-  mpfr_clear (tmp);
 
   return result;
 }
@@ -6265,12 +6256,11 @@ norm2_do_sqrt (gfc_expr *result, gfc_expr *e)
   mpfr_sqrt (result->value.real, result->value.real, GFC_RND_MODE);
   if (norm2_scale && mpfr_regular_p (result->value.real))
     {
-      mpfr_t tmp;
-      mpfr_init (tmp);
+      auto_mpfr tmp;
+      
       mpfr_set_ui (tmp, 1, GFC_RND_MODE);
       mpfr_set_exp (tmp, norm2_scale);
       mpfr_mul (result->value.real, result->value.real, tmp, GFC_RND_MODE);
-      mpfr_clear (tmp);
     }
   norm2_scale = 0;
 
@@ -6304,12 +6294,11 @@ gfc_simplify_norm2 (gfc_expr *e, gfc_expr *dim)
       mpfr_sqrt (result->value.real, result->value.real, GFC_RND_MODE);
       if (norm2_scale && mpfr_regular_p (result->value.real))
 	{
-	  mpfr_t tmp;
-	  mpfr_init (tmp);
+	  auto_mpfr tmp;
+	  
 	  mpfr_set_ui (tmp, 1, GFC_RND_MODE);
 	  mpfr_set_exp (tmp, norm2_scale);
 	  mpfr_mul (result->value.real, result->value.real, tmp, GFC_RND_MODE);
-	  mpfr_clear (tmp);
 	}
       norm2_scale = 0;
     }
diff --git a/gcc/fortran/target-memory.cc b/gcc/fortran/target-memory.cc
index 05b82558672..400bef20225 100644
--- a/gcc/fortran/target-memory.cc
+++ b/gcc/fortran/target-memory.cc
@@ -467,9 +467,8 @@ gfc_interpret_character (unsigned char *buffer, size_t buffer_size,
       result->value.character.string[i] = (gfc_char_t) buffer[i];
   else
     {
-      mpz_t integer;
+      auto_mpz integer;
       size_t bytes = size_character (1, result->ts.kind);
-      mpz_init (integer);
       gcc_assert (bytes <= sizeof (unsigned long));
 
       for (size_t i = 0; i < (size_t) result->value.character.length; i++)
@@ -480,8 +479,6 @@ gfc_interpret_character (unsigned char *buffer, size_t buffer_size,
 	  result->value.character.string[i]
 	    = (gfc_char_t) mpz_get_ui (integer);
 	}
-
-      mpz_clear (integer);
     }
 
   result->value.character.string[result->value.character.length] = '\0';
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 63bd1ac573a..3aa5d155601 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -1796,13 +1796,11 @@ gfc_get_array_constructor_size (mpz_t * size, gfc_constructor_base base)
 {
   gfc_constructor *c;
   gfc_iterator *i;
-  mpz_t val;
-  mpz_t len;
+  auto_mpz val;
+  auto_mpz len;
   bool dynamic;
 
   mpz_set_ui (*size, 0);
-  mpz_init (len);
-  mpz_init (val);
 
   dynamic = false;
   for (c = gfc_constructor_first (base); c; c = gfc_constructor_next (c))
@@ -1828,8 +1826,6 @@ gfc_get_array_constructor_size (mpz_t * size, gfc_constructor_base base)
 	  mpz_add (*size, *size, len);
 	}
     }
-  mpz_clear (len);
-  mpz_clear (val);
   return dynamic;
 }
 
@@ -2037,13 +2033,12 @@ gfc_trans_array_constructor_value (stmtblock_t * pblock, tree type,
   tree step = NULL_TREE;
   stmtblock_t body;
   gfc_se se;
-  mpz_t size;
+  auto_mpz size;
   gfc_constructor *c;
 
   tree shadow_loopvar = NULL_TREE;
   gfc_saved_var saved_loopvar;
 
-  mpz_init (size);
   for (c = gfc_constructor_first (base); c; c = gfc_constructor_next (c))
     {
       /* If this is an iterator or an array, the offset must be a variable.  */
@@ -2292,7 +2287,6 @@ gfc_trans_array_constructor_value (stmtblock_t * pblock, tree type,
 	  gfc_restore_sym (c->iterator->var->symtree->n.sym, &saved_loopvar);
 	}
     }
-  mpz_clear (size);
 }
 
 
@@ -5229,14 +5223,13 @@ set_loop_bounds (gfc_loopinfo *loop)
   gfc_ss **loopspec;
   bool dynamic[GFC_MAX_DIMENSIONS];
   mpz_t *cshape;
-  mpz_t i;
+  auto_mpz i;
   bool nonoptional_arr;
 
   gfc_loopinfo * const outer_loop = outermost_loop (loop);
 
   loopspec = loop->specloop;
 
-  mpz_init (i);
   for (n = 0; n < loop->dimen; n++)
     {
       loopspec[n] = NULL;
@@ -5452,7 +5445,6 @@ set_loop_bounds (gfc_loopinfo *loop)
 	  loop->from[n] = gfc_index_zero_node;
 	}
     }
-  mpz_clear (i);
 
   for (loop = loop->nested; loop; loop = loop->next)
     set_loop_bounds (loop);
diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index 21eeb12ca89..2d275b7fd0a 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -457,7 +457,7 @@ gfc_conv_intrinsic_aint (gfc_se * se, gfc_expr * expr, enum rounding_mode op)
   tree tmp;
   tree cond;
   tree decl;
-  mpfr_t huge;
+  auto_mpfr huge;
   int n, nargs;
   int kind;
 
@@ -498,7 +498,6 @@ gfc_conv_intrinsic_aint (gfc_se * se, gfc_expr * expr, enum rounding_mode op)
 
   /* Test if the value is too large to handle sensibly.  */
   gfc_set_model_kind (kind);
-  mpfr_init (huge);
   n = gfc_validate_kind (BT_INTEGER, kind, false);
   mpfr_set_z (huge, gfc_integer_kinds[n].huge, GFC_RND_MODE);
   tmp = gfc_conv_mpfr_to_tree (huge, kind, 0);
@@ -517,7 +516,6 @@ gfc_conv_intrinsic_aint (gfc_se * se, gfc_expr * expr, enum rounding_mode op)
   tmp = convert (type, tmp);
   se->expr = fold_build3_loc (input_location, COND_EXPR, type, cond, tmp,
 			      arg[0]);
-  mpfr_clear (huge);
 }
 
 
@@ -4632,15 +4630,13 @@ gfc_conv_intrinsic_cotan (gfc_se *se, gfc_expr *expr)
     {
       tree tan;
       tree tmp;
-      mpfr_t pio2;
+      auto_mpfr pio2;
 
       /* Create pi/2.  */
       gfc_set_model_kind (expr->ts.kind);
-      mpfr_init (pio2);
       mpfr_const_pi (pio2, GFC_RND_MODE);
       mpfr_div_ui (pio2, pio2, 2, GFC_RND_MODE);
       tmp = gfc_conv_mpfr_to_tree (pio2, expr->ts.kind, 0);
-      mpfr_clear (pio2);
 
       /* Find tan builtin function.  */
       m = gfc_lookup_intrinsic (GFC_ISYM_TAN);
diff --git a/gcc/real.cc b/gcc/real.cc
index 126695bf2e2..99a216051f1 100644
--- a/gcc/real.cc
+++ b/gcc/real.cc
@@ -2131,7 +2131,7 @@ real_from_string (REAL_VALUE_TYPE *r, const char *str)
     {
       /* Decimal floating point.  */
       const char *cstr = str;
-      mpfr_t m;
+      auto_mpfr m (SIGNIFICAND_BITS);
       bool inexact;
 
       while (*cstr == '0')
@@ -2148,19 +2148,16 @@ real_from_string (REAL_VALUE_TYPE *r, const char *str)
 	goto is_a_zero;
 
       /* Nonzero value, possibly overflowing or underflowing.  */
-      mpfr_init2 (m, SIGNIFICAND_BITS);
       inexact = mpfr_strtofr (m, str, NULL, 10, MPFR_RNDZ);
       /* The result should never be a NaN, and because the rounding is
 	 toward zero should never be an infinity.  */
       gcc_assert (!mpfr_nan_p (m) && !mpfr_inf_p (m));
       if (mpfr_zero_p (m) || mpfr_get_exp (m) < -MAX_EXP + 4)
 	{
-	  mpfr_clear (m);
 	  goto underflow;
 	}
       else if (mpfr_get_exp (m) > MAX_EXP - 4)
 	{
-	  mpfr_clear (m);
 	  goto overflow;
 	}
       else
@@ -2173,7 +2170,6 @@ real_from_string (REAL_VALUE_TYPE *r, const char *str)
 	  gcc_assert (r->cl == rvc_normal);
 	  /* Set a sticky bit if mpfr_strtofr was inexact.  */
 	  r->sig[0] |= inexact;
-	  mpfr_clear (m);
 	}
     }
 
@@ -2474,12 +2470,11 @@ dconst_e_ptr (void)
      These constants need to be given to at least 160 bits precision.  */
   if (value.cl == rvc_zero)
     {
-      mpfr_t m;
-      mpfr_init2 (m, SIGNIFICAND_BITS);
+      auto_mpfr m (SIGNIFICAND_BITS);
+      
       mpfr_set_ui (m, 1, MPFR_RNDN);
       mpfr_exp (m, m, MPFR_RNDN);
       real_from_mpfr (&value, m, NULL_TREE, MPFR_RNDN);
-      mpfr_clear (m);
 
     }
   return &value;
@@ -2517,11 +2512,10 @@ dconst_sqrt2_ptr (void)
      These constants need to be given to at least 160 bits precision.  */
   if (value.cl == rvc_zero)
     {
-      mpfr_t m;
-      mpfr_init2 (m, SIGNIFICAND_BITS);
+      auto_mpfr m (SIGNIFICAND_BITS);
+      
       mpfr_sqrt_ui (m, 2, MPFR_RNDN);
       real_from_mpfr (&value, m, NULL_TREE, MPFR_RNDN);
-      mpfr_clear (m);
     }
   return &value;
 }
diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc
index d58e2258947..a3b56fb6252 100644
--- a/gcc/rust/backend/rust-compile-expr.cc
+++ b/gcc/rust/backend/rust-compile-expr.cc
@@ -2117,10 +2117,9 @@ CompileExpr::compile_integer_literal (const HIR::LiteralExpr &expr,
       return error_mark_node;
     }
 
-  mpz_t type_min;
-  mpz_t type_max;
-  mpz_init (type_min);
-  mpz_init (type_max);
+  auto_mpz type_min;
+  auto_mpz type_max;
+  
   get_type_static_bounds (type, type_min, type_max);
 
   if (mpz_cmp (ival, type_min) < 0 || mpz_cmp (ival, type_max) > 0)
@@ -2133,8 +2132,6 @@ CompileExpr::compile_integer_literal (const HIR::LiteralExpr &expr,
 
   tree result = wide_int_to_tree (type, wi::from_mpz (type, ival, true));
 
-  mpz_clear (type_min);
-  mpz_clear (type_max);
   mpz_clear (ival);
 
   return result;
diff --git a/gcc/tree-ssa-loop-niter.cc b/gcc/tree-ssa-loop-niter.cc
index 581bf5d067b..f1515a62494 100644
--- a/gcc/tree-ssa-loop-niter.cc
+++ b/gcc/tree-ssa-loop-niter.cc
@@ -159,17 +159,15 @@ refine_value_range_using_guard (tree type, tree var,
 
       if (operand_equal_p (var, c0, 0))
 	{
-	  mpz_t valc1;
+	  auto_mpz valc1;
 
 	  /* Case of comparing VAR with its below/up bounds.  */
-	  mpz_init (valc1);
+	  
 	  wi::to_mpz (wi::to_wide (c1), valc1, TYPE_SIGN (type));
 	  if (mpz_cmp (valc1, below) == 0)
 	    cmp = GT_EXPR;
 	  if (mpz_cmp (valc1, up) == 0)
 	    cmp = LT_EXPR;
-
-	  mpz_clear (valc1);
 	}
       else
 	{
@@ -506,7 +504,7 @@ bound_difference_of_offsetted_base (tree type, mpz_t x, mpz_t y,
 {
   int rel = mpz_cmp (x, y);
   bool may_wrap = !nowrap_type_p (type);
-  mpz_t m;
+  auto_mpz m;
 
   /* If X == Y, then the expressions are always equal.
      If X > Y, there are the following possibilities:
@@ -529,7 +527,6 @@ bound_difference_of_offsetted_base (tree type, mpz_t x, mpz_t y,
       return;
     }
 
-  mpz_init (m);
   wi::to_mpz (wi::minus_one (TYPE_PRECISION (type)), m, UNSIGNED);
   mpz_add_ui (m, m, 1);
   mpz_sub (bnds->up, x, y);
@@ -542,8 +539,6 @@ bound_difference_of_offsetted_base (tree type, mpz_t x, mpz_t y,
       else
 	mpz_add (bnds->up, bnds->up, m);
     }
-
-  mpz_clear (m);
 }
 
 /* From condition C0 CMP C1 derives information regarding the
@@ -983,7 +978,7 @@ number_of_iterations_ne (class loop *loop, tree type, affine_iv *iv,
 {
   tree niter_type = unsigned_type_for (type);
   tree s, c, d, bits, assumption, tmp, bound;
-  mpz_t max;
+  auto_mpz max;
 
   niter->control = *iv;
   niter->bound = final;
@@ -1011,12 +1006,10 @@ number_of_iterations_ne (class loop *loop, tree type, affine_iv *iv,
 		       fold_convert (niter_type, iv->base));
     }
 
-  mpz_init (max);
   number_of_iterations_ne_max (max, iv->no_overflow, c, s, bnds,
 			       exit_must_be_taken);
   niter->max = widest_int::from (wi::from_mpz (niter_type, max, false),
 				 TYPE_SIGN (niter_type));
-  mpz_clear (max);
 
   /* Compute no-overflow information for the control iv.  This can be
      proven when below two conditions are satisfied:
@@ -1163,7 +1156,7 @@ number_of_iterations_lt_to_ne (tree type, affine_iv *iv0, affine_iv *iv1,
   tree niter_type = TREE_TYPE (step);
   tree mod = fold_build2 (FLOOR_MOD_EXPR, niter_type, *delta, step);
   tree tmod;
-  mpz_t mmod;
+  auto_mpz mmod;
   tree assumption = boolean_true_node, bound, noloop;
   bool ret = false, fv_comp_no_overflow;
   tree type1 = type;
@@ -1176,7 +1169,6 @@ number_of_iterations_lt_to_ne (tree type, affine_iv *iv0, affine_iv *iv1,
     mod = fold_build2 (MINUS_EXPR, niter_type, step, mod);
   tmod = fold_convert (type1, mod);
 
-  mpz_init (mmod);
   wi::to_mpz (wi::to_wide (mod), mmod, UNSIGNED);
   mpz_neg (mmod, mmod);
 
@@ -1264,7 +1256,6 @@ number_of_iterations_lt_to_ne (tree type, affine_iv *iv0, affine_iv *iv1,
 
   ret = true;
 end:
-  mpz_clear (mmod);
   return ret;
 }
 
diff --git a/gcc/ubsan.cc b/gcc/ubsan.cc
index c2f2e75b300..0256d4eb8ad 100644
--- a/gcc/ubsan.cc
+++ b/gcc/ubsan.cc
@@ -1877,14 +1877,13 @@ ubsan_instrument_float_cast (location_t loc, tree type, tree expr)
       /* For _Decimal128 up to 34 decimal digits, - sign,
 	 dot, e, exponent.  */
       char buf[64];
-      mpfr_t m;
+      auto_mpfr m (prec + 2);
       int p = REAL_MODE_FORMAT (mode)->p;
       REAL_VALUE_TYPE maxval, minval;
 
       /* Use mpfr_snprintf rounding to compute the smallest
 	 representable decimal number greater or equal than
 	 1 << (prec - !uns_p).  */
-      mpfr_init2 (m, prec + 2);
       mpfr_set_ui_2exp (m, 1, prec - !uns_p, MPFR_RNDN);
       mpfr_snprintf (buf, sizeof buf, "%.*RUe", p - 1, m);
       decimal_real_from_string (&maxval, buf);
@@ -1904,7 +1903,6 @@ ubsan_instrument_float_cast (location_t loc, tree type, tree expr)
 	  decimal_real_from_string (&minval, buf);
 	  min = build_real (expr_type, minval);
 	}
-      mpfr_clear (m);
     }
   else
     return NULL_TREE;

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Jakub Jelinek]

On Tue, Mar 07, 2023 at 07:51:03PM +0100, Bernhard Reutner-Fischer wrote:
> While it's a nice idea, there have been resentments towards (visible)
> C++ in the fortran frontend and especially the library, i think.

I thought libgfortran is written in C and Fortran and doesn't use gmp/mpfr,
so this doesn't apply to it (ok, intrinsics/trigd.inc uses mpfr_*
names macros if in library which do something different).

As for the FE, we don't need to change all places with manual
allocation/deallocation for those, though changing most of them
will help with maintainability as one doesn't have to care about leaks.
I only see 2 mpfr_init2 calls in Fortran FE though, so pressumably
everything else could use visually C like auto_mpfr var;
instead of mpfr_t var + mpfr_init + mpfr_clear.

	Jakub

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Alexander Monakov]

Hi,

On Mon, 6 Mar 2023, Richard Biener via Gcc-patches wrote:

> --- a/gcc/realmpfr.h
> +++ b/gcc/realmpfr.h
> @@ -24,6 +24,26 @@
>  #include <mpfr.h>
>  #include <mpc.h>
>  
> +class auto_mpfr
> +{
> +public:
> +  auto_mpfr () { mpfr_init (m_mpfr); }
> +  explicit auto_mpfr (mpfr_prec_t prec) { mpfr_init2 (m_mpfr, prec); }
> +  ~auto_mpfr () { mpfr_clear (m_mpfr); }
> +
> +  operator mpfr_t& () { return m_mpfr; }
> +
> +  auto_mpfr (const auto_mpfr &) = delete;
> +  auto_mpfr &operator=(const auto_mpfr &) = delete;

Shouldn't this use the idiom suggested in ansidecl.h, i.e.

  private:
    DISABLE_COPY_AND_ASSIGN (auto_mpfr);

Alexander

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Jonathan Wakely]

On Tue, 7 Mar 2023 at 19:15, Alexander Monakov wrote:
>
> Hi,
>
> On Mon, 6 Mar 2023, Richard Biener via Gcc-patches wrote:
>
> > --- a/gcc/realmpfr.h
> > +++ b/gcc/realmpfr.h
> > @@ -24,6 +24,26 @@
> >  #include <mpfr.h>
> >  #include <mpc.h>
> >
> > +class auto_mpfr
> > +{
> > +public:
> > +  auto_mpfr () { mpfr_init (m_mpfr); }
> > +  explicit auto_mpfr (mpfr_prec_t prec) { mpfr_init2 (m_mpfr, prec); }
> > +  ~auto_mpfr () { mpfr_clear (m_mpfr); }
> > +
> > +  operator mpfr_t& () { return m_mpfr; }
> > +
> > +  auto_mpfr (const auto_mpfr &) = delete;
> > +  auto_mpfr &operator=(const auto_mpfr &) = delete;
>
> Shouldn't this use the idiom suggested in ansidecl.h, i.e.
>
>   private:
>     DISABLE_COPY_AND_ASSIGN (auto_mpfr);


Why? A macro like that (or a base class like boost::noncopyable) has
some value in a code base that wants to work for both C++03 and C++11
(or later). But in GCC we know we have C++11 now, so we can just
delete members. I don't see what the macro adds.

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Alexander Monakov]

On Tue, 7 Mar 2023, Jonathan Wakely wrote:

> > Shouldn't this use the idiom suggested in ansidecl.h, i.e.
> >
> >   private:
> >     DISABLE_COPY_AND_ASSIGN (auto_mpfr);
> 
> 
> Why? A macro like that (or a base class like boost::noncopyable) has
> some value in a code base that wants to work for both C++03 and C++11
> (or later). But in GCC we know we have C++11 now, so we can just
> delete members. I don't see what the macro adds.

Evidently it's possible to forget to delete one of the members, as
showcased in this very thread.

The idiom is also slightly easier to read.

Alexander

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Jonathan Wakely]

On Tue, 7 Mar 2023 at 21:52, Alexander Monakov wrote:
>
>
> On Tue, 7 Mar 2023, Jonathan Wakely wrote:
>
> > > Shouldn't this use the idiom suggested in ansidecl.h, i.e.
> > >
> > >   private:
> > >     DISABLE_COPY_AND_ASSIGN (auto_mpfr);
> >
> >
> > Why? A macro like that (or a base class like boost::noncopyable) has
> > some value in a code base that wants to work for both C++03 and C++11
> > (or later). But in GCC we know we have C++11 now, so we can just
> > delete members. I don't see what the macro adds.
>
> Evidently it's possible to forget to delete one of the members, as
> showcased in this very thread.

But easily caught by review.

> The idiom is also slightly easier to read.

That's a matter of opinion, I prefer the idiomatic C++ code to a SHOUTY MACRO.

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Marek Polacek]

On Tue, Mar 07, 2023 at 09:54:08PM +0000, Jonathan Wakely via Gcc-patches wrote:
> On Tue, 7 Mar 2023 at 21:52, Alexander Monakov wrote:
> >
> >
> > On Tue, 7 Mar 2023, Jonathan Wakely wrote:
> >
> > > > Shouldn't this use the idiom suggested in ansidecl.h, i.e.
> > > >
> > > >   private:
> > > >     DISABLE_COPY_AND_ASSIGN (auto_mpfr);
> > >
> > >
> > > Why? A macro like that (or a base class like boost::noncopyable) has
> > > some value in a code base that wants to work for both C++03 and C++11
> > > (or later). But in GCC we know we have C++11 now, so we can just
> > > delete members. I don't see what the macro adds.
> >
> > Evidently it's possible to forget to delete one of the members, as
> > showcased in this very thread.
> 
> But easily caught by review.
> 
> > The idiom is also slightly easier to read.
> 
> That's a matter of opinion, I prefer the idiomatic C++ code to a SHOUTY MACRO.

FWIW, I'd also prefer to see the explicit =deletes rather than having to
go look up what exactly the macro does.

Marek

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Richard Biener]

On Wed, 8 Mar 2023, Alexander Monakov wrote:

> 
> On Tue, 7 Mar 2023, Jonathan Wakely wrote:
> 
> > > Shouldn't this use the idiom suggested in ansidecl.h, i.e.
> > >
> > >   private:
> > >     DISABLE_COPY_AND_ASSIGN (auto_mpfr);
> > 
> > 
> > Why? A macro like that (or a base class like boost::noncopyable) has
> > some value in a code base that wants to work for both C++03 and C++11
> > (or later). But in GCC we know we have C++11 now, so we can just
> > delete members. I don't see what the macro adds.
> 
> Evidently it's possible to forget to delete one of the members, as
> showcased in this very thread.

Yes.  And I copy&pasted from somewhere I forgot which also forgot it ...

> The idiom is also slightly easier to read.

Of course inconsistency in the code-base isn't helping that.
auto_bitmap seems to declare but not define things (including
move assign/CTOR?)

Richard.

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Jonathan Wakely]

On Wed, 8 Mar 2023 at 07:25, Richard Biener wrote:
>
> On Wed, 8 Mar 2023, Alexander Monakov wrote:
>
> >
> > On Tue, 7 Mar 2023, Jonathan Wakely wrote:
> >
> > > > Shouldn't this use the idiom suggested in ansidecl.h, i.e.
> > > >
> > > >   private:
> > > >     DISABLE_COPY_AND_ASSIGN (auto_mpfr);
> > >
> > >
> > > Why? A macro like that (or a base class like boost::noncopyable) has
> > > some value in a code base that wants to work for both C++03 and C++11
> > > (or later). But in GCC we know we have C++11 now, so we can just
> > > delete members. I don't see what the macro adds.
> >
> > Evidently it's possible to forget to delete one of the members, as
> > showcased in this very thread.
>
> Yes.  And I copy&pasted from somewhere I forgot which also forgot it ...

Looks like gcc/gimple-predicate-analysis.h and gcc/timevar.h each have
two cases of a deleted copy constructor but no deleted assignment.
I'll send a patch for those.


> > The idiom is also slightly easier to read.
>
> Of course inconsistency in the code-base isn't helping that.
> auto_bitmap seems to declare but not define things (including
> move assign/CTOR?)

I'll change those to deleted too.

Re: [PATCH] [RFC] RAII auto_mpfr and autp_mpz

[Jakub Jelinek]

On Wed, Mar 08, 2023 at 10:13:39AM +0000, Jonathan Wakely wrote:
> On Wed, 8 Mar 2023 at 07:25, Richard Biener wrote:
> >
> > On Wed, 8 Mar 2023, Alexander Monakov wrote:
> >
> > >
> > > On Tue, 7 Mar 2023, Jonathan Wakely wrote:
> > >
> > > > > Shouldn't this use the idiom suggested in ansidecl.h, i.e.
> > > > >
> > > > >   private:
> > > > >     DISABLE_COPY_AND_ASSIGN (auto_mpfr);
> > > >
> > > >
> > > > Why? A macro like that (or a base class like boost::noncopyable) has
> > > > some value in a code base that wants to work for both C++03 and C++11
> > > > (or later). But in GCC we know we have C++11 now, so we can just
> > > > delete members. I don't see what the macro adds.
> > >
> > > Evidently it's possible to forget to delete one of the members, as
> > > showcased in this very thread.
> >
> > Yes.  And I copy&pasted from somewhere I forgot which also forgot it ...
> 
> Looks like gcc/gimple-predicate-analysis.h and gcc/timevar.h each have
> two cases of a deleted copy constructor but no deleted assignment.
> I'll send a patch for those.

So perhaps use the = delete decls and a standardized comment (same wording
for all the auto_* types) above those which says what the above macro says?
Then one can grep that comment etc.

	Jakub

[PATCH 0/3] Fix mpfr and mpz memory leaks

[Bernhard Reutner-Fischer]

From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org>

Hi!

These patches for the go, rust and fortran frontends fix the mpfr and
mpz memory leaks my coccinelle script found.

Bootstrapped and regtested without regressions on x86_64-unknown-linux,
including go,rust,fortran.

Ok for trunk for the fortran bits?

I'd hope that go and rust can apply the respective patches upstream.

Bernhard Reutner-Fischer (3):
  go: Fix memory leak in Integer_expression
  rust: Fix memory leak in compile_{integer,float}_literal
  Fortran: Fix mpz and mpfr memory leaks

 gcc/fortran/array.cc                  | 3 +++
 gcc/fortran/expr.cc                   | 8 ++++----
 gcc/fortran/simplify.cc               | 7 ++++++-
 gcc/go/gofrontend/expressions.cc      | 5 +++++
 gcc/rust/backend/rust-compile-expr.cc | 7 +++++++
 5 files changed, 25 insertions(+), 5 deletions(-)

-- 
2.30.2

[PATCH 1/3] go: Fix memory leak in Integer_expression

[Bernhard Reutner-Fischer]

From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org>

Cc: Ian Lance Taylor <ian@airs.com>

gcc/go/ChangeLog:

	* gofrontend/expressions.cc (Integer_expression::do_import): Fix
	memory leak.
---
 gcc/go/gofrontend/expressions.cc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc
index 4ac55af7433..93f5d5dc52b 100644
--- a/gcc/go/gofrontend/expressions.cc
+++ b/gcc/go/gofrontend/expressions.cc
@@ -2728,6 +2728,7 @@ Integer_expression::do_import(Import_expression* imp, Location loc)
 	    {
 	      go_error_at(imp->location(), "bad number in import data: %qs",
 			  real_str.c_str());
+	      mpfr_clear (real);
 	      return Expression::make_error(loc);
 	    }
 	}
@@ -2743,6 +2744,8 @@ Integer_expression::do_import(Import_expression* imp, Location loc)
 	{
 	  go_error_at(imp->location(), "bad number in import data: %qs",
 		      imag_str.c_str());
+	  mpfr_clear (imag);
+	  mpfr_clear (real);
 	  return Expression::make_error(loc);
 	}
       mpc_t cval;
@@ -2766,6 +2769,7 @@ Integer_expression::do_import(Import_expression* imp, Location loc)
 	{
 	  go_error_at(imp->location(), "bad number in import data: %qs",
 		      num.c_str());
+	  mpz_clear (val);
 	  return Expression::make_error(loc);
 	}
       Expression* ret;
@@ -2783,6 +2787,7 @@ Integer_expression::do_import(Import_expression* imp, Location loc)
 	{
 	  go_error_at(imp->location(), "bad number in import data: %qs",
 		      num.c_str());
+	  mpfr_clear (val);
 	  return Expression::make_error(loc);
 	}
       Expression* ret = Expression::make_float(&val, NULL, loc);
-- 
2.30.2

[PATCH 2/3] rust: Fix memory leak in compile_{integer,float}_literal

[Bernhard Reutner-Fischer]

From: Bernhard Reutner-Fischer <aldot@gcc.gnu.org>

Cc: Arthur Cohen <arthur.cohen@embecosm.com>
Cc: Philip Herron <herron.philip@googlemail.com>

gcc/rust/ChangeLog:

	* backend/rust-compile-expr.cc (CompileExpr::compile_integer_literal):
	(CompileExpr::compile_float_literal): Fix memory leak.
---
 gcc/rust/backend/rust-compile-expr.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc
index 436fc924a13..82078b81953 100644
--- a/gcc/rust/backend/rust-compile-expr.cc
+++ b/gcc/rust/backend/rust-compile-expr.cc
@@ -2119,6 +2119,7 @@ CompileExpr::compile_integer_literal (const HIR::LiteralExpr &expr,
   if (mpz_init_set_str (ival, literal_value.as_string ().c_str (), 10) != 0)
     {
       rust_error_at (expr.get_locus (), "bad number in literal");
+      mpz_clear (ival);
       return error_mark_node;
     }
 
@@ -2133,6 +2134,9 @@ CompileExpr::compile_integer_literal (const HIR::LiteralExpr &expr,
       rust_error_at (expr.get_locus (),
 		     "integer overflows the respective type %<%s%>",
 		     tyty->get_name ().c_str ());
+      mpz_clear (type_min);
+      mpz_clear (type_max);
+      mpz_clear (ival);
       return error_mark_node;
     }
 
@@ -2158,6 +2162,7 @@ CompileExpr::compile_float_literal (const HIR::LiteralExpr &expr,
       != 0)
     {
       rust_error_at (expr.get_locus (), "bad number in literal");
+      mpfr_clear (fval);
       return error_mark_node;
     }
 
@@ -2179,9 +2184,11 @@ CompileExpr::compile_float_literal (const HIR::LiteralExpr &expr,
       rust_error_at (expr.get_locus (),
 		     "decimal overflows the respective type %<%s%>",
 		     tyty->get_name ().c_str ());
+      mpfr_clear (fval);
       return error_mark_node;
     }
 
+  mpfr_clear (fval);
   return real_value;
 }
 
-- 
2.30.2

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

[Bernhard Reutner-Fischer]

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

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

[Harald Anlauf]

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");
>   }

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

[Harald Anlauf]

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");
>   }

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

[Bernhard Reutner-Fischer]

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");
>>   }
>

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

[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");
> >>   }  
> >  
> 

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

[Steve Kargl]

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

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

[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,