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 <anl...@gmx.de>
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 00000000000..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 (present (y)) y=0
+ end subroutine test
+
+ subroutine test_wrapper_c (w) bind(c)
+ real, dimension(1), intent(out), optional :: w
+ call test_c (w)
+ end subroutine test_wrapper_c
+ subroutine test_c (y) bind(c)
+ real, dimension(:), intent(out), optional :: y
+ if (present (y)) y=0
+ end subroutine test_c
+
+ subroutine test2_wrapper (u, v)
+ real, intent(out), optional :: u
+ real, dimension(1), intent(out), optional :: v
+ call test2 (u)
+ call test2 (v) !
+ end subroutine test2_wrapper
+ subroutine test2 (z)
+ real, dimension(..), intent(out), optional :: z
+ end subroutine test2
+end program
+
+! { dg-final { scan-tree-dump-times "data = v != 0B " 1 "original" } }
+! { dg-final { scan-tree-dump-times "data = x != 0B " 1 "original" } }
+! { dg-final { scan-tree-dump-times "data = xx.0 != 0B " 1 "original" } }
+! { dg-output " abcghinop(\n|\r\n|\r)" }"
+! { dg-output " defjlmqrs(\n|\r\n|\r)" }"
diff --git a/gcc/testsuite/gfortran.dg/missing_optional_dummy_6a.f90 b/gcc/testsuite/gfortran.dg/missing_optional_dummy_6a.f90
index c08c97a2c7e..bd34613c143 100644
--- a/gcc/testsuite/gfortran.dg/missing_optional_dummy_6a.f90
+++ b/gcc/testsuite/gfortran.dg/missing_optional_dummy_6a.f90
@@ -49,10 +49,10 @@ end program test
! { dg-final { scan-tree-dump-times "scalar2 \\(slr1" 1 "original" } }
-! { dg-final { scan-tree-dump-times "= es1 != 0B" 1 "original" } }
+! { dg-final { scan-tree-dump-times "= es1 != 0B" 2 "original" } }
! { dg-final { scan-tree-dump-times "assumed_shape2 \\(es1" 0 "original" } }
! { dg-final { scan-tree-dump-times "explicit_shape2 \\(es1" 1 "original" } }
-! { dg-final { scan-tree-dump-times "= as1.0 != 0B" 2 "original" } }
+! { dg-final { scan-tree-dump-times "= as1.0 != 0B" 4 "original" } }
! { dg-final { scan-tree-dump-times "assumed_shape2 \\(as1" 0 "original" } }
! { dg-final { scan-tree-dump-times "explicit_shape2 \\(as1" 0 "original" } }
--
2.31.1
