Dear all,

as there has been some good progress in the handling of optional dummy
arguments, I looked again at this PR and a patch for it that I withdrew
as it turned out incomplete.

It turned out that it now needs only a minor adjustment for optional
dummy arguments of procedures with bind(c) attribute so that ubsan
checking does not trigger.

Along this way I extended the previous testcase to exercise to some
extent combinations of bind(c) and non-bind(c) procedures and found
one failure (since at least gcc-9) that is genuine: passing a missing
optional from a bind(c) procedure to an assumed-rank dummy, see
PR114355.  The corresponding test is commented in the testcase.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


On 2/6/22 22:04, Harald Anlauf wrote:
> Hi Mikael,
>
> Am 04.02.22 um 11:45 schrieb Mikael Morin:
>> Hello,
>>
>> Le 29/01/2022 à 22:41, Harald Anlauf via Fortran a écrit :
>>> 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.
>
> only after submitting the patch I figured that the patch is incomplete.
>
> When we have a call chain of procedures with and without bind(c),
> there are still cases left where the failure with the sanitizer
> is not fixed.  Just add "bind(c)" to subroutine test_wrapper only
> in the original PR.
>
> I have added a corresponding comment in the PR.
>
>>> 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.
>>>
>> I have an other (equally unimportant) concern that it may create an
>> unnecessary conditional when passing a subobject of an optional
>> argument.  In that case we can assume that the optional is present.
>> It’s not a correctness issue, so let’s not bother at this stage.
>
> Judging from the dump tree of the cases I looked at I did not see
> anything that would pose a problem to the optimizer.
>
>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>
>> OK.
>
> Given my latest observations I'd rather withdraw the current version of
> the patch and rethink.  I also did not see an issue with bind(c)
> procedures calling alikes.
>
> It would help if one would not only know the properties of the actual
> argument, but also of the formal one, which is not available at that
> point in the code.  I'll have another look and resubmit.
>
>> Thanks.
>>
>
> Thanks for the review!
>
> Harald
>