Compiling and running the minimal example program main implicit none call test_wrapper contains subroutine test_wrapper(y) real, dimension(1), intent(out), optional :: y call test(y) end subroutine test_wrapper subroutine test(y) real, dimension(:), intent(out), optional :: y if (present(y)) y=0 end subroutine test end program with -fsanitize=undefined on any gfortran version since 8.1.0 produces this false positive: /app/example.f90:8:20: runtime error: load of null pointer of type 'real(kind=4)' See for example: https://godbolt.org/z/aqGE18EGG The issue disappears on gfortran version 7.3.0 and earlier, and also if 'y' in 'test' is not an assumed-shape array, for example if replacing 'dimension(:)' by 'dimension(1)'. See also https://stackoverflow.com/questions/68046152/is-passing-an-absent-assumed-shape-array-for-an-optional-argument-of-another-pro Unfortunately I am not sure if this is an issue with gfortran itself or moreso a false positive by UndefinedBehaviourSanitizer. So perhaps this report should rather be for the "sanitizer" component, sorry if that is the case.
With some bisecting I managed to track this down to commit https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=243c288370fe51ba55c3a9ee61eb2a1a62cb1279 being the first "faulty" one. From what I can tell the commit aimed to catch null pointer dereferences in more cases. Not sure if this means that the commit made the check to broad so that it generates false positives for this Fortran case. Or if gfortran perhaps generates intermediate representation that is not quite valid and is correctly flagged by UBSan.
I think I found the issue within gfortran by looking at the output of -fdump-tree-all. For the example, the file a-example.f90.005t.original lists this intermediate representation for test_wrapper: __attribute__((fn spec (". w "))) void test_wrapper (real(kind=4)[1] * restrict y) { { struct array01_real(kind=4) parm.5; struct array01_real(kind=4) * D.3980; parm.5.span = 4; parm.5.dtype = {.elem_len=4, .rank=1, .type=3}; parm.5.dim[0].lbound = 1; parm.5.dim[0].ubound = 1; parm.5.dim[0].stride = 1; parm.5.data = (void *) &(*y)[0]; parm.5.offset = -1; D.3980 = y != 0B ? &parm.5 : 0B; test (D.3980); } } While there is a check for parameter y being present in the assignment for D.3980, it kinda seems too late. For setting the data member of the parm.5 array descriptor, parameter y was already dereferenced unconditionally before. So if y is absent, there is a null pointer dereference that UBSan seems to complain about, even if its result is never used. As a rookie attempt to fix this, I added a conditional to the data member assignment: diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c index a6bcd2b5ab7..503ceba82ee 100644 --- a/gcc/fortran/trans-array.c +++ b/gcc/fortran/trans-array.c @@ -7068,6 +7068,13 @@ 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); + if (expr->expr_type == EXPR_VARIABLE && expr->symtree->n.sym->attr.optional) + { + tree present = gfc_conv_expr_present (expr->symtree->n.sym); + offset = build3_loc (input_location, COND_EXPR, TREE_TYPE(offset), + present, offset, build_zero_cst(TREE_TYPE(offset))); + offset = gfc_evaluate_now (offset, block); + } gfc_conv_descriptor_data_set (block, parm, offset); } With that, test_wrapper now looks like this: __attribute__((fn spec (". w "))) void test_wrapper (real(kind=4)[1] * restrict y) { { struct array01_real(kind=4) parm.5; real(kind=4)[0:] * restrict D.3980; struct array01_real(kind=4) * D.3981; parm.5.span = 4; parm.5.dtype = {.elem_len=4, .rank=1, .type=3}; parm.5.dim[0].lbound = 1; parm.5.dim[0].ubound = 1; parm.5.dim[0].stride = 1; D.3980 = y != 0B ? (real(kind=4)[0:] * restrict) &(*y)[0] : 0B; parm.5.data = (void *) D.3980; parm.5.offset = -1; D.3981 = y != 0B ? &parm.5 : 0B; test (D.3981); } } That means y is only dereferenced if it is present, and UBSan does not throw an error anymore. This fix seems quite hacky to me with how late it applies, but at least it highlights the issue well to my mind. I suppose a better, proper fix would maybe wrap the whole initialization of the array descriptor in a conditional branch? So that the array descriptor is only assigned if the parameter is not absent. Perhaps that conditional could surround gfc_conv_expr_descriptor or even one level higher gfc_conv_array_parameter.
Confirmed.
Created attachment 52311 [details] Patch that regtests ok. The patch suggested by the reporter is rather close to this one. We need to special-case optional arguments of procedures with bind(c) attribute. I haven't understood the details yet, but excluding those prevents regressions in the c-binding testsuite, e.g. bind-c-contiguous-4.*. We need to fix the pattern for testcase gfortran.dg/missing_optional_dummy_6a.f90 which would have failed with -fsanitize=undefined without this patch.
Submitted: https://gcc.gnu.org/pipermail/fortran/2022-January/057496.html
(In reply to anlauf from comment #4) > Created attachment 52311 [details] > Patch that regtests ok. While playing some more with variations of the testcase, I figured that this patch is not complete. It does not fix cases where a call tree consists of procedures with and without the bind(c) attribute. So more work is needed.
The master branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>: https://gcc.gnu.org/g:3f3f0b7ee8022776d69ecaed1375e1559716f226 commit r14-9509-g3f3f0b7ee8022776d69ecaed1375e1559716f226 Author: Harald Anlauf <anlauf@gmx.de> Date: Fri Mar 15 20:14:07 2024 +0100 Fortran: fix for absent array argument passed to optional dummy [PR101135] 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 pattern. * gfortran.dg/ubsan/missing_optional_dummy_8.f90: New test.
The releases/gcc-13 branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>: https://gcc.gnu.org/g:344b60addb79278c95b7a5712aaf381111721a27 commit r13-8492-g344b60addb79278c95b7a5712aaf381111721a27 Author: Harald Anlauf <anlauf@gmx.de> Date: Fri Mar 15 20:14:07 2024 +0100 Fortran: fix for absent array argument passed to optional dummy [PR101135] 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 pattern. * gfortran.dg/ubsan/missing_optional_dummy_8.f90: New test.
Fixed on mainline for gcc-14, and backported to 13-branch. Closing. Thanks for the report and the draft patch!
Great, already working on compiler explorer with gfortran (trunk). Thanks a lot!