Le 27/07/2024 à 19:23, rep.dot....@gmail.com a écrit :
On 22 July 2024 20:53:18 CEST, Mikael Morin <morin-mik...@orange.fr> wrote:
From: Mikael Morin <mik...@gcc.gnu.org>
Hello,
this fixes a null pointer dereference with absent optional dummy passed
as BACK argument of MINLOC/MAXLOC.
Tested for regression on x86_64-linux.
OK for master?
-- >8 --
Protect the evaluation of BACK with a check that the reference is non-null
in case the expression is an optional dummy, in the inline code generated
for MINLOC and MAXLOC.
This change contains a revert of the non-testsuite part of commit
r15-1994-ga55d24b3cf7f4d07492bb8e6fcee557175b47ea3, which factored the
evaluation of BACK out of the loop using the scalarizer. It was a bad idea,
because delegating the argument evaluation to the scalarizer makes it
cumbersome to add a null pointer check next to the evaluation.
Instead, evaluate BACK at the beginning, before scalarization, add a check
that the argument is present if necessary, and evaluate the resulting
expression to a variable, before using the variable in the inline code.
gcc/fortran/ChangeLog:
* trans-intrinsic.cc (maybe_absent_optional_variable): New function.
(gfc_conv_intrinsic_minmaxloc): Remove BACK from scalarization and
evaluate it before. Add a check that BACK is not null if the
expression is an optional dummy. Save the resulting expression to a
variable. Use the variable in the generated inline code.
gcc/testsuite/ChangeLog:
* gfortran.dg/maxloc_6.f90: New test.
* gfortran.dg/minloc_7.f90: New test.
---
gcc/fortran/trans-intrinsic.cc | 81 +++++-
gcc/testsuite/gfortran.dg/maxloc_6.f90 | 366 +++++++++++++++++++++++++
gcc/testsuite/gfortran.dg/minloc_7.f90 | 366 +++++++++++++++++++++++++
3 files changed, 799 insertions(+), 14 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/maxloc_6.f90
create mode 100644 gcc/testsuite/gfortran.dg/minloc_7.f90
diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index 180d0d7a88c..9f3c3ce47bc 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -5209,6 +5209,50 @@ gfc_conv_intrinsic_dot_product (gfc_se * se, gfc_expr *
expr)
}
+/* Tells whether the expression E is a reference to an optional variable whose
+ presence is not known at compile time. Those are variable references
without
+ subreference; if there is a subreference, we can assume the variable is
+ present. We have to special case full arrays, which we represent with a
fake
+ "full" reference, and class descriptors for which a reference to data is not
+ really a subreference. */
+
+bool
+maybe_absent_optional_variable (gfc_expr *e)
+{
+ if (!(e && e->expr_type == EXPR_VARIABLE))
+ return false;
[]
+}
+
+
/* Remove unneeded kind= argument from actual argument list when the
result conversion is dealt with in a different place. */
@@ -5321,11 +5365,11 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
tree nonempty;
tree lab1, lab2;
tree b_if, b_else;
+ tree back;
gfc_loopinfo loop;
gfc_actual_arglist *actual;
gfc_ss *arrayss;
gfc_ss *maskss;
- gfc_ss *backss;
gfc_se arrayse;
gfc_se maskse;
gfc_expr *arrayexpr;
@@ -5391,10 +5435,27 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr *
expr, enum tree_code op)
&& maskexpr->symtree->n.sym->attr.dummy
&& maskexpr->symtree->n.sym->attr.optional;
backexpr = actual->next->next->expr;
- if (backexpr)
- backss = gfc_get_scalar_ss (gfc_ss_terminator, backexpr);
+
+ gfc_init_se (&backse, NULL);
+ if (backexpr == nullptr)
+ back = logical_false_node;
+ else if (maybe_absent_optional_variable (backexpr))
+ {
/* if this fires, some wonky race is going on.. */
+ gcc_assert (backexpr->expr_type == EXPR_VARIABLE);
drop it, downgrade to checking, or is it worth?
Whether it is worth it, I don't know; it's protecting the access to
backexpr->symtree a few lines down, idependently of the implementation
of maybe_absent_optional_variable.
I expect the compiler to optimize it away, so I can surely make it a
checking-only assert.
+
+ gfc_conv_expr (&backse, backexpr);
+ tree present = gfc_conv_expr_present (backexpr->symtree->n.sym, false);
+ back = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR,
+ logical_type_node, present, backse.expr);
+ }
else
- backss = nullptr;
+ {
+ gfc_conv_expr (&backse, backexpr);
+ back = backse.expr;
+ }
+ gfc_add_block_to_block (&se->pre, &backse.pre);
+ back = gfc_evaluate_now_loc (input_location, back, &se->pre);
+ gfc_add_block_to_block (&se->pre, &backse.post);
nonempty = NULL
[]
thanks
