[PATCH] Fortran: check type of operands of logical operations, comparisons [PR107272]
Dear all,
this PR is actually very related to PR107217 that addressed ICEs
with bad array constructors with typespec when used in arithmetic
expressions. The present patch extends the checking to logical
operations and to comparisons and catches several ICE-on-invalid
as well as a few cases of accepts-invalid.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Thanks,
Harald
From 779baf06888f3adef13c12c468c0a5ef0a45f93e Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Sun, 16 Oct 2022 20:32:27 +0200
Subject: [PATCH] Fortran: check type of operands of logical operations,
comparisons [PR107272]
gcc/fortran/ChangeLog:
PR fortran/107272
* arith.cc (gfc_arith_not): Operand must be of type BT_LOGICAL.
(gfc_arith_and): Likewise.
(gfc_arith_or): Likewise.
(gfc_arith_eqv): Likewise.
(gfc_arith_neqv): Likewise.
(gfc_arith_eq): Compare consistency of types of operands.
(gfc_arith_ne): Likewise.
(gfc_arith_gt): Likewise.
(gfc_arith_ge): Likewise.
(gfc_arith_lt): Likewise.
(gfc_arith_le): Likewise.
gcc/testsuite/ChangeLog:
PR fortran/107272
* gfortran.dg/pr107272.f90: New test.
---
gcc/fortran/arith.cc | 33 ++
gcc/testsuite/gfortran.dg/pr107272.f90 | 21
2 files changed, 54 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/pr107272.f90
diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index c8e882badab..fc9224ebc5c 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -422,6 +422,9 @@ gfc_arith_not (gfc_expr *op1, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != BT_LOGICAL)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, op1->ts.kind, &op1->where);
result->value.logical = !op1->value.logical;
*resultp = result;
@@ -435,6 +438,9 @@ gfc_arith_and (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != BT_LOGICAL || op2->ts.type != BT_LOGICAL)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_kind_max (op1, op2),
&op1->where);
result->value.logical = op1->value.logical && op2->value.logical;
@@ -449,6 +455,9 @@ gfc_arith_or (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != BT_LOGICAL || op2->ts.type != BT_LOGICAL)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_kind_max (op1, op2),
&op1->where);
result->value.logical = op1->value.logical || op2->value.logical;
@@ -463,6 +472,9 @@ gfc_arith_eqv (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != BT_LOGICAL || op2->ts.type != BT_LOGICAL)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_kind_max (op1, op2),
&op1->where);
result->value.logical = op1->value.logical == op2->value.logical;
@@ -477,6 +489,9 @@ gfc_arith_neqv (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != BT_LOGICAL || op2->ts.type != BT_LOGICAL)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_kind_max (op1, op2),
&op1->where);
result->value.logical = op1->value.logical != op2->value.logical;
@@ -1187,6 +1202,9 @@ gfc_arith_eq (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != op2->ts.type)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_default_logical_kind,
&op1->where);
result->value.logical = (op1->ts.type == BT_COMPLEX)
@@ -1203,6 +1221,9 @@ gfc_arith_ne (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != op2->ts.type)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_default_logical_kind,
&op1->where);
result->value.logical = (op1->ts.type == BT_COMPLEX)
@@ -1219,6 +1240,9 @@ gfc_arith_gt (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != op2->ts.type)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_default_logical_kind,
&op1->where);
result->value.logical = (gfc_compare_expr (op1, op2, INTRINSIC_GT) > 0);
@@ -1233,6 +1257,9 @@ gfc_arith_ge (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != op2->ts.type)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_default_logical_kind,
&op1->where);
result->value.logical = (gfc_compare_expr (op1, op2, INTRINSIC_GE) >= 0);
@@ -1247,6 +1274,9 @@ gfc_arith_lt (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp)
{
gfc_expr *result;
+ if (op1->ts.type != op2->ts.type)
+return ARITH_INVALID_TYPE;
+
result = gfc_get_constant_expr (BT_LOGICAL, gfc_default_logical_kind,
&op1->where);
result->value.logical = (gfc_compare_expr (op1, op2, INTRINSIC_LT)
Re: [Patch] Fortran: Fixes for kind=4 characters strings [PR107266]
Hi Tobias,
the patch LGTM.
Regarding testcase char4_decl-2.f90, I played a little and found that
one could in addition check the storage_size of aa, pp in the main and
compare with storage_size (4_'foo') etc. Without your patch the
storage sizes look odd. (Strictly speaking, a comparison like
if (aa .ne. 4_'foo') stop 123
is not fully sufficient to catch such oddities.)
Thanks,
Harald
Am 14.10.22 um 23:18 schrieb Tobias Burnus:
Long introduction - but the patch is rather simple: Don't use kind=1
as type where kind=4 should be used.
Long introduction + background, feel free to skip.
This popped up for libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90
which uses kind=4 characters – if Sandra's "Fortran: delinearize
multi-dimensional
array accesses" patch is applied.
Patch: https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562230.html
Used for OG11:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584716.html
On the OG12 alias devel/omp/gcc-12 vendor branch, it is used:
https://gcc.gnu.org/g:39a8c371fda6136cf77c74895a00b136409e0ba3
* * *
For mainline, I did not observe a wrong-code issue at runtime, still:
void frobc (character(kind=4)[1:*_a] * & restrict a, ...
...
static void frobc (character(kind=1) * & restrict, ...
feels odd, i.e. having the definition as kind=4 and the declaration as
kind=1.
With the patch, it becomes:
static void frobc (character(kind=4) * & restrict, character(kind=4) *
&, ...
* * *
For the following, questionable code (→ PR107266), it is even worse:
character(kind=4) function f(x) bind(C)
character(kind=4), value :: x
end
this gives the following, which has the wrong ABI:
character(kind=1) f (character(kind=1) x)
{
(void) 0;
}
With the patch, it becomes:
character(kind=4) f (character(kind=4) x)
* * *
I think that all only exercises the trans-type.cc patch;
the trans-expr.cc code gets called – as an assert shows,
but I fail to get a dump where this goes wrong.
However, for struct-elem-map-1.f90 with mainline or with
OG12 and the patch:
#pragma omp target map(tofrom:var.uni2[40 / 20] [len: 20])
while on OG12 without the attached patch:
#pragma omp target map(tofrom:var.uni2[40 / 5] [len: 5])
where the problem is that TYPE_SIZE_UNIT is wrong. Whether
this only affects OG12 due to the delinearizer patch or
some code on mainline as well, I don't know.
Still, I think it should be fixed ...
OK for mainline?
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201,
80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer:
Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München;
Registergericht München, HRB 106955
Re: [PATCH] Fortran: check type of operands of logical operations, comparisons [PR107272]
Le 16/10/2022 à 20:46, Harald Anlauf via Fortran a écrit : Dear all, this PR is actually very related to PR107217 that addressed ICEs with bad array constructors with typespec when used in arithmetic expressions. The present patch extends the checking to logical operations and to comparisons and catches several ICE-on-invalid as well as a few cases of accepts-invalid. Regtested on x86_64-pc-linux-gnu. OK for mainline? Yes, thanks.
Re: [PATCH, v2] Fortran: handle bad array ctors with typespec [PR93483, , PR107216, PR107219]
Le 15/10/2022 à 22:15, Harald Anlauf via Fortran a écrit : Dear all, here is an updated version of the patch that includes suggestions and comments by Mikael in PR93483. Basic new features are: - a new enum value ARITH_NOT_REDUCED to keep track if we encountered an expression that was not reduced via reduce_unary/reduce_binary - a cleanup of the related checking, resulting in more readable code. - a new testcase by Mikael that exhibited a flaw in the first patch due to a false resolution of a symbol by premature simplification. Regtested again. OK for mainline? (...) diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 10bb098d136..7b8f0b148bd 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -222,11 +222,12 @@ enum gfc_intrinsic_op Assumptions are made about the numbering of the interface_op enums. */ #define GFC_INTRINSIC_OPS GFC_INTRINSIC_END -/* Arithmetic results. */ +/* Arithmetic results. ARITH_NOT_REDUCED is used to keep track of failed + reductions because an erroneous expression was encountered. */ The expressions are not always erroneous. They can be, but in the testcase for example, all the expressions are valid. They are just unsupported by the arithmetic evaluation code which works only with literal constants and arrays of literal constants (and arrays of arrays etc). OK with that comment fixed. Thanks.
