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.

Reply via email to