From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.17.21]) by sourceware.org (Postfix) with ESMTPS id 3B3943858026; Fri, 20 Aug 2021 19:43:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3B3943858026 X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [93.207.81.56] ([93.207.81.56]) by web-mail.gmx.net (3c-app-gmx-bs33.server.lan [172.19.170.85]) (via HTTP); Fri, 20 Aug 2021 21:43:12 +0200 MIME-Version: 1.0 Message-ID: From: Harald Anlauf To: Tobias Burnus Cc: jakub@redhat.com, "H.J. Lu" , gcc-patches@gcc.gnu.org, fortran Subject: Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514 Content-Type: multipart/mixed; boundary=rekceb-957c4094-76dd-44c1-b96e-680bd379751b Date: Fri, 20 Aug 2021 21:43:12 +0200 Importance: normal Sensitivity: Normal In-Reply-To: <08347883-b9a9-0291-14e8-f6e232eb9de0@codesourcery.com> References: <8d25c317-74fa-d8a2-724f-de6944fa602e@codesourcery.com> <20210820091618.GB2380545@tucnak> <20210820101251.GF2380545@tucnak> <08347883-b9a9-0291-14e8-f6e232eb9de0@codesourcery.com> X-UI-Message-Type: mail X-Priority: 3 X-Provags-ID: V03:K1:gWf6CH4X8njCDIaOXdWvR5mYiTATWL1yNjgyO5Day4HD0Xkcop3NjqWxjzNHefsaKgfwc MzWq3B7i6E0meDRBJy3VhbqxWvAfdTHs7MU4GgOk3S6zcoZcnrWM8ctwpGMaF+9f83fCKFjC4jPr frUKnNvcXfSNoFRcIOB3tgRX8WU2yhLxt0A6l5FyJZWx1V6YIDf0c66TQGBNwDXJ+iGdjINpfEXw Ko4HAtO3gTGKnyFm9gQwMoFgCOAD2KM9O2YH3j75zftWUrL8ug5kNzKnPKQyNTtHosyxX3wHf5So +o= X-UI-Out-Filterresults: notjunk:1;V03:K0:Atf3r1MYewk=:frSZTbePEr+Bs0zFmpfV7b doZBa+pPAGbxXgByb8Jb3PZCQjgRQZmx2euIrz+sLZHZxU6EE0WOJJBa1b4vWhxkwK9yl0AT8 N+jtVjJ6I2ocrfaojVGx4PW9NcBHd70WlgQMhSLFEu0rVLb8teuvfkH6eYBxZZYBqS0YT1pt3 QinN7F9I4nTavJINQK0V4sEoCrA6Ja8ZZmJgV34k5HAQ8vWZAWAHw8FrwDQakw1ZppnmPhphg cCiLFWNBqIURpNlNOqP0L/meN9uEW0Tq4PgwcFTKYTb1DV2QUY0Lp55uIMPqctj2rEafLMeZN 0q/VphyI1XVEGjkZtzsTq/u3TnAW5AuNw+RVaZksZiJv9a0VLMP3A0ivAfEKETCY/t1g/yp+w Tv7EonzVRMxBvbyBbqT2ktD0td62znOmPUF7jZumPY1jtv45DSwBKe2d2DYlFQElgJUPG4vFQ jiT48Nil6xa/SVzTN6TMBZ2pryrkLz/qgTkE0UtIvc8TD4V0xQaWG08HXmf26BVzWci4DDHfs JvKCnUUPgPxzrlT2g7OAy9MGXPUmjsFphekDwGlD0PKuNhF4TG99EOSkRzG+gcumJeqb9v8c9 NTCqLyaUjNzxcu4ZPDR8mlbgwvX2LFdwSy6CixR6ptSMuHkmatkPNf3qt3v6au/WV2zPcbONf gaoYIlg/gvQgV0HAeLHQVLMZjJkTVoffPksekrgTwBoA1WlBxrPRMoCoIVQJapjk7T9l239F+ SWw0j0NsLxSkUJLNsDMWvFsko8C6wiWRkni0ax9jnrUAOlzZHbECiJ8wnUMT/YcHlNtiUS134 2RsTg9zALLdfXYQfkUZHEmapq1XCQ== X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, GIT_PATCH_0, KAM_NUMSUBJECT, KAM_SHORT, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Aug 2021 19:43:31 -0000 --rekceb-957c4094-76dd-44c1-b96e-680bd379751b Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Tobias, > LGTM =E2=80=93 I am fine with either variant, but I am slightly inclined= to > removing the gcc_assert* > =E2=80=93 as I believe that the existing checks come early enough and do= seem to > work well=2E I played some more and found additional cases that we hadn't discussed before=2E (At least I hadn't thought of them because of the focus on deferred length strings=2E) - automatic string variables / arrays - assumed length strings - PDTs with character components=2E The last one actually turned out sort of "hopeless" for now, so I opened https://gcc=2Egnu=2Eorg/bugzilla/show_bug=2Ecgi?id=3D102003 to track this=2E I added the other cases to testcase pr100950=2Ef90 and reduced the checks and code within "substring_has_constant_len" to the bare minimum=2E See the attached follow-up patch=2E > Can you check ('grep') whether we already have sufficient coverage of > substring out of bounds? > We presumably have, but if you spot something, I think it makes sense to > add those to the testsuite=2E We do have some checks on substring indices (e=2Eg=2E substr_10=2Ef90), but not really extensive coverage=2E > Tobias > *I think GCC won't complain but if ENABLE_ASSERT_CHECKING is not defined= , > there is then a pointless 'length =3D' assignment, overridden before it = is > ever used=2E Yes=2E This is fixed below=2E I guess I have to apologize for things getting a bit out of control for this PR, but the results are on the other hand way beyond my initial expectations=2E=2E=2E Re-regtested on x86_64-pc-linux-gnu=2E Should be safe elsewhere=2E=2E=2E OK? Thanks, Harald Fortran - extend set of substring expressions handled in length simplifica= tion gcc/fortran/ChangeLog: PR fortran/100950 * simplify=2Ec (substring_has_constant_len): Minimize checks for substring expressions being allowed=2E gcc/testsuite/ChangeLog: PR fortran/100950 * gfortran=2Edg/pr100950=2Ef90: Extend coverage=2E --rekceb-957c4094-76dd-44c1-b96e-680bd379751b Content-Type: text/x-patch Content-Disposition: attachment; filename=pr100950-followup-v3.patch Content-Transfer-Encoding: quoted-printable diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c index 4cb73e836c7..b46cbfa90ab 100644 =2D-- a/gcc/fortran/simplify.c +++ b/gcc/fortran/simplify.c @@ -4533,14 +4533,7 @@ substring_has_constant_len (gfc_expr *e) || !ref->u.ss.start || ref->u.ss.start->expr_type !=3D EXPR_CONSTANT || !ref->u.ss.end - || ref->u.ss.end->expr_type !=3D EXPR_CONSTANT - || !ref->u.ss.length) - return false; - - /* For non-deferred strings the given length shall be constant. */ - if (!e->ts.deferred - && (!ref->u.ss.length->length - || ref->u.ss.length->length->expr_type !=3D EXPR_CONSTANT)) + || ref->u.ss.end->expr_type !=3D EXPR_CONSTANT) return false; /* Basic checks on substring starting and ending indices. */ @@ -4551,27 +4544,7 @@ substring_has_constant_len (gfc_expr *e) iend =3D gfc_mpz_get_hwi (ref->u.ss.end->value.integer); if (istart <=3D iend) - { - if (istart < 1) - { - gfc_error ("Substring start index (%wd) at %L below 1", - istart, &ref->u.ss.start->where); - return false; - } - - /* For deferred strings use end index as proxy for length. */ - if (e->ts.deferred) - length =3D iend; - else - length =3D gfc_mpz_get_hwi (ref->u.ss.length->length->value.integer); - if (iend > length) - { - gfc_error ("Substring end index (%wd) at %L exceeds string length", - iend, &ref->u.ss.end->where); - return false; - } - length =3D iend - istart + 1; - } + length =3D iend - istart + 1; else length =3D 0; diff --git a/gcc/testsuite/gfortran.dg/pr100950.f90 b/gcc/testsuite/gfortr= an.dg/pr100950.f90 index cb9d126bc18..a19409c2507 100644 =2D-- a/gcc/testsuite/gfortran.dg/pr100950.f90 +++ b/gcc/testsuite/gfortran.dg/pr100950.f90 @@ -46,6 +46,18 @@ program p integer, parameter :: l9 =3D len (r(1)%u(:)(3:4)) if (l9 /=3D 2) stop 13 end block + + call sub (42, "abcde") +contains + subroutine sub (m, c) + integer, intent(in) :: m + character(len=3D*), intent(in) :: c + character(len=3Dm) :: p, o(3) + integer, parameter :: l10 =3D len (p(6:7)) + integer, parameter :: l11 =3D len (o(:)(6:7)) + integer, parameter :: l12 =3D len (c(2:3)) + if (l10 /=3D 2 .or. l11 /=3D 2 .or. l12 /=3D 2) stop 14 + end subroutine sub end ! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 2 "original"= } } --rekceb-957c4094-76dd-44c1-b96e-680bd379751b--