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: [PATCH, v2] PR libfortran/103634 - Runtime crash with PACK on zero-sized arrays
Date: Mon, 13 Dec 2021 21:25:26 +0100	[thread overview]
Message-ID: <b65efd76-0ea5-7edb-78d0-7e020b0e615d@gmx.de> (raw)
In-Reply-To: <6c67790c-bda2-299b-a253-dc96bb11fea0@orange.fr>

Hi Mikael,

Am 09.12.21 um 21:37 schrieb Mikael Morin:
> Hello,
>
> On 09/12/2021 21:05, Harald Anlauf via Fortran wrote:
>> Dear all,
>>
>> I had thought that we had fixed this in the past (see PR31001),
>> but it did fail for me with all gcc versions I have tried (7-12)
>> for a slightly more elaborate case as in the old testcase.
>>
>> The loop in pack_internal did try to access the first element of
>> the array argument to PACK even if one (or more) extents were zero.
>> This is not good.
>>
>> Solution: check the extents and return early.  (We already do a
>> related check for the vector argument if present).
>
> If there is a vector argument, aren’t we supposed to copy it to the
> result ?
> There is something else to pay attention for, the early return should
> come at least after the return array bounds have been set.  In the
> testcase an array with the correct bounds has been allocated beforehand
> to hold the return value, but it’s not always the case.

you are absolutely right, I had gotten that wrong.

> For what it’s worth, the non-generic variant in pack.m4 (or in
> pack_{i,f,c}{1,2,4,8,10,16}.c) has a zero extent check and it clears the
> source ptr in that case, which makes it setup the return array and then
> jump to the vector copy at the end of the function.
>

The code is so similar (for good reason) that it makes sense to keep
it synchronous.  I added code for 'zero_sized' array with the minor
difference that I made it boolean instead of integer.

I also extended the testcase so that it exercises PACK/pack_internal
a little, for argument 'vector' present as well as not.  (There are
existing tests for intrinsic types, but not for the issue at hand).

Regtested again, and checked the testcase (against other compilers
and also with valgrind).

OK now?

Thanks,
Harald

  reply	other threads:[~2021-12-13 20:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 20:05 [PATCH] " Harald Anlauf
2021-12-09 20:37 ` Mikael Morin
2021-12-13 20:25   ` Harald Anlauf [this message]
2021-12-13 20:27     ` [PATCH, v2] " Harald Anlauf
2021-12-13 20:27       ` Harald Anlauf
2021-12-14 11:50       ` 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=b65efd76-0ea5-7edb-78d0-7e020b0e615d@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).