[Bug fortran/101135] New: Use of absent assumed-shape array argument as an actual argument for an optional dummy argument mistakenly flagged as error by UndefinedBehaviourSanitizer

[Bug fortran/101135] New: Use of absent assumed-shape array argument as an actual argument for an optional dummy argument mistakenly flagged as error by UndefinedBehaviourSanitizer

[marcel.jacobse at ewetel dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

            Bug ID: 101135
           Summary: Use of absent assumed-shape array argument as an
                    actual argument for an optional dummy argument
                    mistakenly flagged as error by
                    UndefinedBehaviourSanitizer
           Product: gcc
           Version: 11.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marcel.jacobse at ewetel dot net
  Target Milestone: ---

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.

[Bug fortran/101135] Use of absent assumed-shape array argument as an actual argument for an optional dummy argument mistakenly flagged as error by UndefinedBehaviourSanitizer

[marcel.jacobse at ewetel dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

--- Comment #1 from Marcel Jacobse <marcel.jacobse at ewetel dot net> ---
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.

[Bug fortran/101135] Use of absent assumed-shape array argument as an actual argument for an optional dummy argument mistakenly flagged as error by UndefinedBehaviourSanitizer

[marcel.jacobse at ewetel dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

--- Comment #2 from Marcel Jacobse <marcel.jacobse at ewetel dot net> ---
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.

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

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anlauf at gcc dot gnu.org
   Last reconfirmed|                            |2022-01-27
             Status|UNCONFIRMED                 |NEW
           Priority|P3                          |P4
     Ever confirmed|0                           |1

--- Comment #3 from anlauf at gcc dot gnu.org ---
Confirmed.

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

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

--- Comment #4 from anlauf at gcc dot gnu.org ---
Created attachment 52311
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52311&action=edit
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.

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

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |anlauf at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #5 from anlauf at gcc dot gnu.org ---
Submitted: https://gcc.gnu.org/pipermail/fortran/2022-January/057496.html

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

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW

--- Comment #6 from anlauf at gcc dot gnu.org ---
(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.

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

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
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.

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

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
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.

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

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
   Target Milestone|---                         |13.3
             Status|NEW                         |RESOLVED

--- Comment #9 from anlauf at gcc dot gnu.org ---
Fixed on mainline for gcc-14, and backported to 13-branch.  Closing.

Thanks for the report and the draft patch!

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

[marcel.jacobse at ewetel dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

--- Comment #10 from Marcel Jacobse <marcel.jacobse at ewetel dot net> ---
Great, already working on compiler explorer with gfortran (trunk). Thanks a
lot!