[PATCH] Fortran: reject module variable as character length in PARAMETER [PR104349]
Dear all,
the attached patch fixes an ICE-on-invalid for a PARAMETER expression
where the character length was a MODULE variable. The ICE seemed
strange, as we were catching related erroneous code for declarations in
programs or subroutines. Removing a seemingly bogus check of restricted
expressions is the simplest way to fix this. (We could also catch this
differently in decl.cc).
Besides, this also fixes an accepts-invalid, see testcase. :-)
Regtested on x86_64-pc-linux-gnu. OK for mainline (13) or rather wait?
Thanks,
Harald
From 37136ce94b44149dd013b3d7fed7adba769241e6 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Mon, 3 Apr 2023 21:34:01 +0200
Subject: [PATCH] Fortran: reject module variable as character length in
PARAMETER [PR104349]
gcc/fortran/ChangeLog:
PR fortran/104349
* expr.cc (check_restricted): Adjust check for valid variables in
restricted expressions: make no exception for module variables.
gcc/testsuite/ChangeLog:
PR fortran/104349
* gfortran.dg/der_charlen_1.f90: Adjust dg-patterns.
* gfortran.dg/pr104349.f90: New test.
---
gcc/fortran/expr.cc | 2 --
gcc/testsuite/gfortran.dg/der_charlen_1.f90 | 2 ++
gcc/testsuite/gfortran.dg/pr104349.f90 | 8
3 files changed, 10 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/pr104349.f90
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 7fb33f81788..02028f993fd 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -3504,8 +3504,6 @@ check_restricted (gfc_expr *e)
|| sym->attr.implied_index
|| sym->attr.flavor == FL_PARAMETER
|| is_parent_of_current_ns (sym->ns)
- || (sym->ns->proc_name != NULL
- && sym->ns->proc_name->attr.flavor == FL_MODULE)
|| (gfc_is_formal_arg () && (sym->ns == gfc_current_ns)))
{
t = true;
diff --git a/gcc/testsuite/gfortran.dg/der_charlen_1.f90 b/gcc/testsuite/gfortran.dg/der_charlen_1.f90
index 9f394c73f25..1246522d516 100644
--- a/gcc/testsuite/gfortran.dg/der_charlen_1.f90
+++ b/gcc/testsuite/gfortran.dg/der_charlen_1.f90
@@ -22,3 +22,5 @@ CONTAINS
type(T), intent(in) :: X
end subroutine
end module another_core
+
+! { dg-prune-output "cannot appear in the expression" }
diff --git a/gcc/testsuite/gfortran.dg/pr104349.f90 b/gcc/testsuite/gfortran.dg/pr104349.f90
new file mode 100644
index 000..2bea4a37214
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr104349.f90
@@ -0,0 +1,8 @@
+! { dg-do compile }
+! PR fortran/104349 - reject module variable as character length in PARAMETER
+! Contributed by G.Steinmetz
+
+module m
+ character(n), parameter :: a(1) = 'b' ! { dg-error "cannot appear" }
+ character(n), parameter :: c= 'b' ! { dg-error "cannot appear" }
+end
--
2.35.3
Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks
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
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
On 3 April 2023 21:50:49 CEST, Harald Anlauf 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
>>
>> 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_
Re: [PATCH] Fortran: reject module variable as character length in PARAMETER [PR104349]
Hi Harald, OK for mainline. It is sufficiently small that, if there is any fallout in the next weeks, it can easily be reverted without great impact. Thanks for the patch. Paul On Mon, 3 Apr 2023 at 20:46, Harald Anlauf via Fortran wrote: > Dear all, > > the attached patch fixes an ICE-on-invalid for a PARAMETER expression > where the character length was a MODULE variable. The ICE seemed > strange, as we were catching related erroneous code for declarations in > programs or subroutines. Removing a seemingly bogus check of restricted > expressions is the simplest way to fix this. (We could also catch this > differently in decl.cc). > > Besides, this also fixes an accepts-invalid, see testcase. :-) > > Regtested on x86_64-pc-linux-gnu. OK for mainline (13) or rather wait? > > Thanks, > Harald > > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
