Re: [PATCH, v2] Fortran: fix for absent array argument passed to optional dummy [PR101135]

[Harald Anlauf <anlauf@gmx.de>]

Hi Mikael,

On 3/17/24 22:04, Mikael Morin wrote:
>> diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
>> index 3673fa40720..a7717a8107e 100644
>> --- a/gcc/fortran/trans-array.cc
>> +++ b/gcc/fortran/trans-array.cc
>> @@ -7526,6 +7526,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.  Arguments of
>> BIND(C)
>> +     procedures 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
>> +      && !is_CFI_desc (NULL, expr))
>
> I think the condition could additionally check the lack of subreferences.
> But it's maybe not worth the trouble, and the patch is conservatively
> correct as is, so OK.

I have thought about the conditions here for some time and did not
find better ones.  They need to be broad enough to catch the case
in gfortran.dg/missing_optional_dummy_6a.f90 that (according to the
tree-dump) was not properly handled previously and would have triggered
ubsan at some point in the future when someone tried to change that
testcase from currently dg-do compile to dg-do run...
(After the patch it would pass, but I didn't dare to change the dg-do).

I have pushed the patch as-is, but feel free to post testcases
not covered (or improperly covered) to narrow this down further...

> Thanks for the patch.

Thanks for the review!

Harald

>> +    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);
>>  }
>>
>
>