public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Harald Anlauf <anlauf@gmx.de>
To: Mikael Morin <morin-mikael@orange.fr>,
	fortran <fortran@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] PR/101135 - Load of null pointer when passing absent assumed-shape array argument for an optional dummy argument
Date: Sun, 6 Feb 2022 22:04:31 +0100	[thread overview]
Message-ID: <24718b24-981a-730c-4cd3-b6f4727797a0@gmx.de> (raw)
In-Reply-To: <3fd50892-dbef-d43a-8efe-148a8ffa94a9@orange.fr>

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


  reply	other threads:[~2022-02-06 21:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-29 21:41 Harald Anlauf
2022-02-04 10:45 ` Mikael Morin
2022-02-06 21:04   ` Harald Anlauf [this message]
2024-03-15 19:32     ` [PATCH, v2] Fortran: fix for absent array argument passed to optional dummy [PR101135] Harald Anlauf
2024-03-17 21:04       ` Mikael Morin
2024-03-17 22:10         ` Harald Anlauf
2024-03-18 18:47           ` Mikael Morin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=24718b24-981a-730c-4cd3-b6f4727797a0@gmx.de \
    --to=anlauf@gmx.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=morin-mikael@orange.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).