[PATCH] Fortran: fix passing of optional dummy as actual to optional argument [PR55978]

[PATCH] Fortran: fix passing of optional dummy as actual to optional argument [PR55978]

[Harald Anlauf]

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

Dear all,

the attached patch fixes issues exhibited by the testcase in comment#19 of PR55978.

First, when passing an allocatable optional dummy array to an optional dummy,
we need to prevent accessing the data component of the array when the argument
is not present, and pass a null pointer instead.  This is straightforward.

Second, the case of a missing pointer optional dummy array should have worked,
but the presence check surprisingly did not work as expected at -O0 or -Og,
but at higher optimization levels.  Interestingly, the dump-tree looked right,
but running under gdb or investigating the assembler revealed that the order
of tests in a logical AND expression was opposed to what the tree-dump looked
like.  Replacing TRUTH_AND_EXPR by TRUTH_ANDIF_EXPR and checking the optimized
dump confirmed that this does fix the issue.

Note that the tree-dump is not changed by this replacement.  Does this mean
thar AND and ANDIF currently are not differentiated at this level?

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Would it be ok to backport this to 14-branch, too?

Thanks,
Harald


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr55978.diff --]
[-- Type: text/x-patch, Size: 4203 bytes --]

From 94e4c66d8374a12be38637620f362acf1fba5343 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Sun, 23 Jun 2024 22:36:43 +0200
Subject: [PATCH] Fortran: fix passing of optional dummy as actual to optional
 argument [PR55978]

gcc/fortran/ChangeLog:

	PR fortran/55978
	* trans-array.cc (gfc_conv_array_parameter): Do not dereference
	data component of a missing allocatable dummy array argument for
	passing as actual to optional dummy.  Harden logic of presence
	check for optional pointer dummy by using TRUTH_ANDIF_EXPR instead
	of TRUTH_AND_EXPR.

gcc/testsuite/ChangeLog:

	PR fortran/55978
	* gfortran.dg/optional_absent_12.f90: New test.
---
 gcc/fortran/trans-array.cc                    | 20 ++++++++++---
 .../gfortran.dg/optional_absent_12.f90        | 30 +++++++++++++++++++
 2 files changed, 46 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/optional_absent_12.f90

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 19d69aec9c0..26237f43bec 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -8703,6 +8703,10 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
 	&& (sym->backend_decl != parent))
     this_array_result = false;

+  /* Passing an optional dummy argument as actual to an optional dummy?  */
+  bool pass_optional;
+  pass_optional = fsym && fsym->attr.optional && sym && sym->attr.optional;
+
   /* Passing address of the array if it is not pointer or assumed-shape.  */
   if (full_array_var && g77 && !this_array_result
       && sym->ts.type != BT_DERIVED && sym->ts.type != BT_CLASS)
@@ -8740,6 +8744,14 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
 	  if (size)
 	    array_parameter_size (&se->pre, tmp, expr, size);
 	  se->expr = gfc_conv_array_data (tmp);
+	  if (pass_optional)
+	    {
+	      tree cond = gfc_conv_expr_present (sym);
+	      se->expr = build3_loc (input_location, COND_EXPR,
+				     TREE_TYPE (se->expr), cond, se->expr,
+				     fold_convert (TREE_TYPE (se->expr),
+						   null_pointer_node));
+	    }
           return;
         }
     }
@@ -8989,8 +9001,8 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
 	  tmp = fold_build2_loc (input_location, NE_EXPR, logical_type_node,
 				 fold_convert (TREE_TYPE (tmp), ptr), tmp);

-	  if (fsym && fsym->attr.optional && sym && sym->attr.optional)
-	    tmp = fold_build2_loc (input_location, TRUTH_AND_EXPR,
+	  if (pass_optional)
+	    tmp = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR,
 				   logical_type_node,
 				   gfc_conv_expr_present (sym), tmp);

@@ -9024,8 +9036,8 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
       tmp = fold_build2_loc (input_location, NE_EXPR, logical_type_node,
 			     fold_convert (TREE_TYPE (tmp), ptr), tmp);

-      if (fsym && fsym->attr.optional && sym && sym->attr.optional)
-	tmp = fold_build2_loc (input_location, TRUTH_AND_EXPR,
+      if (pass_optional)
+	tmp = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR,
 			       logical_type_node,
 			       gfc_conv_expr_present (sym), tmp);

diff --git a/gcc/testsuite/gfortran.dg/optional_absent_12.f90 b/gcc/testsuite/gfortran.dg/optional_absent_12.f90
new file mode 100644
index 00000000000..1e61d91fb6d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/optional_absent_12.f90
@@ -0,0 +1,30 @@
+! { dg-do run }
+! { dg-additional-options "-fcheck=array-temps" }
+!
+! PR fortran/55978 - comment#19
+!
+! Test passing of (missing) optional dummy to optional array argument
+
+program test
+  implicit none
+  integer, pointer :: p(:) => null()
+  call one (p)
+  call one (null())
+  call one ()
+  call three ()
+contains
+  subroutine one (y)
+    integer, pointer, optional, intent(in) :: y(:)
+    call two (y)
+  end subroutine one
+
+  subroutine three (z)
+    integer, allocatable, optional, intent(in) :: z(:)
+    call two (z)
+  end subroutine three
+
+  subroutine two (x)
+    integer, optional, intent(in) :: x(*)
+    if (present (x)) stop 1
+  end subroutine two
+end
--
2.35.3

Re: [PATCH] Fortran: fix passing of optional dummy as actual to optional argument [PR55978]

[Mikael Morin]

Le 23/06/2024 à 22:58, Harald Anlauf a écrit :
> Dear all,
> 
> the attached patch fixes issues exhibited by the testcase in comment#19 of PR55978.
> 
> First, when passing an allocatable optional dummy array to an optional dummy,
> we need to prevent accessing the data component of the array when the argument
> is not present, and pass a null pointer instead.  This is straightforward.
> 
> Second, the case of a missing pointer optional dummy array should have worked,
> but the presence check surprisingly did not work as expected at -O0 or -Og,
> but at higher optimization levels.  Interestingly, the dump-tree looked right,
> but running under gdb or investigating the assembler revealed that the order
> of tests in a logical AND expression was opposed to what the tree-dump looked
> like.  Replacing TRUTH_AND_EXPR by TRUTH_ANDIF_EXPR and checking the optimized
> dump confirmed that this does fix the issue.
> 
> Note that the tree-dump is not changed by this replacement.  Does this mean
> thar AND and ANDIF currently are not differentiated at this level?
> 
tree-pretty-print.cc's op_symbol_code handles them as:

     case TRUTH_AND_EXPR:
     case TRUTH_ANDIF_EXPR:
       return "&&";

so no, I don't think they are differentiated.

> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> 
> Would it be ok to backport this to 14-branch, too?
> 
Sure, OK for both.
Thanks.

Re: [PATCH] Fortran: fix passing of optional dummy as actual to optional argument [PR55978]

[Andreas Schwab]

On Jun 24 2024, Mikael Morin wrote:

> tree-pretty-print.cc's op_symbol_code handles them as:
>
>     case TRUTH_AND_EXPR:
>     case TRUTH_ANDIF_EXPR:
>       return "&&";
>
> so no, I don't think they are differentiated.

Only because C does not have a TRUTH_AND_EXPR operator.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."