[PATCH] PR/101135 - Load of null pointer when passing absent assumed-shape array argument for an optional dummy argument

2022-01-29 Thread Harald Anlauf via Fortran
Dear Fortranners,

compiling with -fsanitize=undefined shows that we did mishandle the
case where a missing optional argument is passed to another procedure.

Besides the example given in the PR, the existing testcase
fortran.dg/missing_optional_dummy_6a.f90 fails with:

gcc/testsuite/gfortran.dg/missing_optional_dummy_6a.f90:21:29: runtime error: 
load of null pointer of type 'integer(kind=4)'
gcc/testsuite/gfortran.dg/missing_optional_dummy_6a.f90:22:30: runtime error: 
load of null pointer of type 'integer(kind=4)'
gcc/testsuite/gfortran.dg/missing_optional_dummy_6a.f90:27:29: runtime error: 
load of null pointer of type 'integer(kind=4)'

The least invasive change - already pointed out by the reporter - is
to check the presence of the argument before dereferencing the data
pointer after the offset calculation.  This requires adjusting the
checking pattern for gfortran.dg/missing_optional_dummy_6a.f90.

Regtesting reminded me that procedures with bind(c) attribute are doing
their own stuff, which is why they need to be excluded here, otherwise
testcase bind-c-contiguous-4.f90 would regress on the expected output.

I've created a testcase that uses this PR's input as well as the lesson
learned from studying the bind(c) testcase and placed this in the asan
subdirectory.

There is a potential alternative solution which I did not pursue, as I
think it is more invasive, but also that I didn't succeed to implement:
A non-present dummy array argument should not need to get its descriptor
set up.  Pursuing this is probably not the right thing to do during the
current stage of development and could be implemented later.  If somebody
believes this is important, feel free to open a PR for this.

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

Thanks,
Harald

From 69ca8f83149107f48b86360eb878d9d746b99234 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Sat, 29 Jan 2022 22:18:30 +0100
Subject: [PATCH] Fortran: fix handling of absent array argument passed to
 optional dummy

gcc/fortran/ChangeLog:

	PR fortran/101135
	* trans-array.cc (gfc_get_dataptr_offset): Check for optional
	arguments being present before dereferencing data pointer.

gcc/testsuite/ChangeLog:

	PR fortran/101135
	* gfortran.dg/missing_optional_dummy_6a.f90: Adjust diagnostic patterns.
	* gfortran.dg/asan/missing_optional_dummy_7.f90: New test.
---
 gcc/fortran/trans-array.cc| 11 
 .../asan/missing_optional_dummy_7.f90 | 64 +++
 .../gfortran.dg/missing_optional_dummy_6a.f90 |  4 +-
 3 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/asan/missing_optional_dummy_7.f90

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index cfb6eac11c7..9eaa99c5550 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -7207,6 +7207,17 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset,

   /* Set the target data pointer.  */
   offset = gfc_build_addr_expr (gfc_array_dataptr_type (desc), tmp);
+
+  /* Check for optional dummy argument being present.  BIND(C) procedure
+ arguments are excepted here since they are handled differently.  */
+  if (expr->expr_type == EXPR_VARIABLE
+  && expr->symtree->n.sym->attr.dummy
+  && expr->symtree->n.sym->attr.optional
+  && !expr->symtree->n.sym->ns->proc_name->attr.is_bind_c)
+offset = build3_loc (input_location, COND_EXPR, TREE_TYPE (offset),
+			 gfc_conv_expr_present (expr->symtree->n.sym), offset,
+			 fold_convert (TREE_TYPE (offset), gfc_index_zero_node));
+
   gfc_conv_descriptor_data_set (block, parm, offset);
 }

diff --git a/gcc/testsuite/gfortran.dg/asan/missing_optional_dummy_7.f90 b/gcc/testsuite/gfortran.dg/asan/missing_optional_dummy_7.f90
new file mode 100644
index 000..bdd7006170d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/asan/missing_optional_dummy_7.f90
@@ -0,0 +1,64 @@
+! { dg-do run }
+! { dg-additional-options "-fdump-tree-original -fsanitize=undefined" }
+!
+! PR fortran/101135 - Load of null pointer when passing absent
+! assumed-shape array argument for an optional dummy argument
+!
+! Based on testcase by Marcel Jacobse
+
+program main
+  implicit none
+  character(len=3) :: a(6) = ['abc', 'def', 'ghi', 'jlm', 'nop', 'qrs']
+  call as ()
+  call as (a(::2))
+  call as_c ()
+  call as_c (a(2::2))
+  call test_wrapper
+  call test_wrapper_c
+  call test2_wrapper
+contains
+  subroutine as (xx)
+character(len=*), optional, intent(in) :: xx(*)
+if (.not. present (xx)) return
+print *, xx(1:3)
+  end subroutine as
+  subroutine as_c (zz) bind(c)
+character(len=*), optional, intent(in) :: zz(*)
+if (.not. present (zz)) return
+print *, zz(1:3)
+  end subroutine as_c
+
+  subroutine test_wrapper (x)
+real, dimension(1), intent(out), optional :: x
+call test (x) !
+  end subroutine test_wrapper
+  subroutine test (y)
+real, dimension(:), intent(out), optional :: y
+if 

[PATCH] fortran: Unshare associate var charlen [PR104228]

2022-01-29 Thread Mikael Morin

Hello,

the attached patch is a fix for PR104228.
Even if simple, I wouldn’t call it obvious, as it’s involving character 
length and associate, so I don’t mind some extra review eyes.


Tested on x86_64-pc-linux-gnu. Ok for master/11/10/9?From 0819226560387b2953622ee3d5d051a35606d504 Mon Sep 17 00:00:00 2001
From: Mikael Morin 
Date: Fri, 28 Jan 2022 22:00:57 +0100
Subject: [PATCH] fortran: Unshare associate var charlen [PR104228]

PR104228 showed that character lengths were shared between associate
variable and associate targets.  This is problematic when the associate
target is itself a variable and gets a variable to hold the length, as
the length variable is added (and all the variables following it in the chain)
to both the associate variable scope and the target variable scope.
This caused an ICE when compiling with -O0 -fsanitize=address.

This change forces the creation of a separate character length for the
associate variable.  It also forces the initialization of the character
length variable to avoid regressing associate_32 and associate_47 tests.

gcc/fortran/ChangeLog:

	* resolve.cc (resolve_assoc_var): Also create a new character
	length for non-dummy associate targets.
	* trans-stmt.cc (trans_associate_var): Initialize character length
	even if no temporary is used for the associate variable.

gcc/testsuite/ChangeLog:

	* gfortran.dg/asan/associate_1.f90: New test.
	* gfortran.dg/asan/associate_2.f90: New test.
---
 gcc/fortran/resolve.cc|  1 -
 gcc/fortran/trans-stmt.cc |  2 +-
 .../gfortran.dg/asan/associate_1.f90  | 19 +++
 .../gfortran.dg/asan/associate_2.f90  | 19 +++
 4 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/asan/associate_1.f90
 create mode 100644 gcc/testsuite/gfortran.dg/asan/associate_2.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 835a4783718..266e41e25b1 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -9227,7 +9227,6 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_target)
 	sym->ts.u.cl = target->ts.u.cl;
 
   if (sym->ts.deferred && target->expr_type == EXPR_VARIABLE
-	  && target->symtree->n.sym->attr.dummy
 	  && sym->ts.u.cl == target->ts.u.cl)
 	{
 	  sym->ts.u.cl = gfc_new_charlen (sym->ns, NULL);
diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc
index 04f8147d23b..30b6bd5dd2a 100644
--- a/gcc/fortran/trans-stmt.cc
+++ b/gcc/fortran/trans-stmt.cc
@@ -1918,7 +1918,7 @@ trans_associate_var (gfc_symbol *sym, gfc_wrapped_block *block)
   gfc_conv_expr_descriptor (, e);
 
   if (sym->ts.type == BT_CHARACTER
-	  && !se.direct_byref && sym->ts.deferred
+	  && sym->ts.deferred
 	  && !sym->attr.select_type_temporary
 	  && VAR_P (sym->ts.u.cl->backend_decl)
 	  && se.string_length != sym->ts.u.cl->backend_decl)
diff --git a/gcc/testsuite/gfortran.dg/asan/associate_1.f90 b/gcc/testsuite/gfortran.dg/asan/associate_1.f90
new file mode 100644
index 000..b5ea75498b7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/asan/associate_1.f90
@@ -0,0 +1,19 @@
+! { dg-do compile }
+! { dg-additional-options "-O0" }
+!
+! PR fortran/104228
+! The code generated code for the program below wrongly pushed the Y character
+! length variable to both P and S scope, which was leading to an ICE when
+! address sanitizer was in effect
+
+program p
+   character(:), save, allocatable :: x(:)
+   call s
+contains
+   subroutine s
+  associate (y => x)
+ y = [x]
+  end associate
+   end
+end
+
diff --git a/gcc/testsuite/gfortran.dg/asan/associate_2.f90 b/gcc/testsuite/gfortran.dg/asan/associate_2.f90
new file mode 100644
index 000..9bfb2bfbafb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/asan/associate_2.f90
@@ -0,0 +1,19 @@
+! { dg-do compile }
+! { dg-additional-options "-O0" }
+!
+! PR fortran/104228
+! The code generated code for the program below wrongly pushed the Y character
+! length variable to both P and S scope, which was leading to an ICE when
+! address sanitizer was in effect
+
+program p
+   character(:), allocatable :: x(:)
+   call s
+contains
+   subroutine s
+  associate (y => x)
+ y = [x]
+  end associate
+   end
+end
+
-- 
2.34.1



Re: gfortran run on 64bit Mac ?

2022-01-29 Thread Iain Sandoe via Fortran
Hello David,

> On 29 Jan 2022, at 11:14, dmainprice via Fortran  wrote:

> gfortran  run on 64bit Mac ?

Yes, gfortran has been running on (x86_64) 64bit Mac since macOS 10.6.

> Any suggestions will F77 code run ?

if it will compile with gfortran, then you could expect it to run,

cheers
Iain

> 
> all the best and happy year
> 
> David Mainprice 
> 
> 



gfortran run on 64bit Mac ?

2022-01-29 Thread dmainprice via Fortran
Hello every one,

gfortran  run on 64bit Mac ?

Any suggestions will F77 code run ?


all the best and happy year

David Mainprice