From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.15.18]) by sourceware.org (Postfix) with ESMTPS id E389D3858D35; Thu, 25 Nov 2021 20:03:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E389D3858D35 X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from gluon.fritz.box ([79.251.11.137]) by mail.gmx.net (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1N1fn0-1mf5c52swK-011z4D; Thu, 25 Nov 2021 21:03:25 +0100 Subject: Re: [PATCH] PR fortran/103411 - ICE in gfc_conv_array_initializer, at fortran/trans-array.c:6377 To: Mikael Morin , fortran , gcc-patches Newsgroups: gmane.comp.gcc.patches,gmane.comp.gcc.fortran References: <4fb65fbe-c10f-3fd4-9961-9978ff386bf9@orange.fr> From: Harald Anlauf Message-ID: <24ff1ac7-79ce-2843-a199-9e85865304f6@gmx.de> Date: Thu, 25 Nov 2021 21:03:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <4fb65fbe-c10f-3fd4-9961-9978ff386bf9@orange.fr> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:a/0OP/D8vzhQIZWgYxrFTL9U9PBSuh7qEj5aACel3E306ZgXWuZ 0uDPQrbP0CogzwQC7l5/qHz7Ah23VpczDYnAG4nySCo9Eq30l8SlW0fO4obfWNOKADxhKma wN/wpxN6EWciG5an5Qnroc2lmhiGTwJrUURGSguoFV4M+bfgkYyoTW1P3abbMPvW1zG/8BO pjAUtqtEkrYujhDE/Di8g== X-UI-Out-Filterresults: notjunk:1;V03:K0:PdpLefoCVWQ=:Tv66OL8Y4VKBMX4JE20Ap2 Qytnsfs8uu0hbxPZI42AeOZIu2rdy7RsBw7B8P+P/cm45Mdk4Scqdlz8TkJxf0HUJaSvC/wOT cysYqmbb21bp1JMKQCyV/2FSZHS3lYwoWY3crJ23riegtf85rA8NFVQhlIxxZjsM6Vmu1MNZ+ ceR0Wld1+73nUeXv/h/WFif8Br3RpqX6pUMcUqNby97H0TjiQixIY9iMLV3Hmurybnumr36O6 b+1TbUfqnAgyU1udDuLba8orVJU51sYNi3knXoncXmv+NJ6jn9TPFdzrYXY20NABWbYKnWl3s y3cr/8hHbDLH6BqFS3B+RvTvcXm4SxaNm5mk/6fm/rhDFwnAWKKyqjbHl53PWe+U5sBOuSoy+ xeGAqrmlCbfB0hEBDAQl3A5ci4wuzqxBhHP5XtaKrxI/VU7IzAgjWWdmOumc8p5VG+EBjgGw4 fxXCDAZItgXKV8v9ai342PP7yxpcaADj1LLYv4cDN3pG/1QtO/9oVyXJ14kmft6pZhJADdxI7 dT+zqjHxb9ygh8UYxaf9XUSG5Gnh9uKv+ZR6ws5sx/RBznIuFgwyFrUGUdL9pI0jfG0t4Guxp 0UlHJZd2kwXmimRb0t5ztjxoE3Np2vqAxJve9ZHB+bVTUagdXwz7KF1337IHO9SOqbef7gwod mMf5khcygSKAJTT5SK7ryy6EADAYrZir2KBRazbH/+zvConV1kIMoWsyofnvG+KnNpAoUZQTx iiGj55rQZ9G+voXW53/yHsLWVfom55O2xqGP2SWBwtQVZq1ZX+MCc9xo4Qq5/MboI8+7AT95e A1GoBXODwOAJO+SM+sv+EZiUoDlNgn4+FZx20/pet3k/SAKEA9m2UtuaRvwNemor6c0KJal2f 4V0X/Q5Oo/jQfzzs87lwLNbgOKvQ3ipf3aEnay3szCzCEXa+pZ7nJGN06wAZbREYCAGq7vj8T HyM0tM0GK8WXmAfB63hYX4AASZCVN7JBBP1nVej1YPKV1yEuQgSGi9Pv9UOh0IvwuRH+KfCWY vt56B9ZlHbCUBIS3C8A5faBlKQY0XfsKg6KwJm3AdCAmCaV1aZEbdDsAa1KZgEjH+liJ+0cpS Yl9szdt/FvYDVI= X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, GIT_PATCH_0, KAM_NUMSUBJECT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Nov 2021 20:03:30 -0000 Hi Mikael, Am 25.11.21 um 17:46 schrieb Mikael Morin: > Hello, > > Le 24/11/2021 =C3=A0 22:32, Harald Anlauf via Fortran a =C3=A9crit=C2=A0= : >> diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c >> index 5a5aca10ebe..837eb0912c0 100644 >> --- a/gcc/fortran/check.c >> +++ b/gcc/fortran/check.c >> @@ -4866,10 +4868,17 @@ gfc_check_reshape (gfc_expr *source, gfc_expr >> *shape, >> =C2=A0=C2=A0=C2=A0=C2=A0 { >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 gfc_constructor *c; >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 bool test; >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 gfc_constructor_base b; >> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (shape->expr_type =3D=3D EXPR_ARRAY) >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 b =3D shape->value.construc= tor; >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 else if (shape->expr_type =3D=3D EXPR_V= ARIABLE) >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 b =3D shape->symtree->n.sym= ->value->value.constructor; > > This misses a check that shape->symtree->n.sym->value is an array, so > that it makes sense to access its constructor. there are checks further above for the cases shape->expr_type =3D=3D EXPR_ARRAY and for shape->expr_type =3D=3D EXPR_VARIABLE which look at the elements of array shape to see if they are non-negative. Only in those cases where the full "if ()'s" pass we set shape_is_const =3D true; and proceed. The purpose of the auxiliary bool shape_is_const is to avoid repeating the lengthy if's again. Only then the above cited code segment should get executed. For shape->expr_type =3D=3D EXPR_ARRAY there is really no change in logic. For shape->expr_type =3D=3D EXPR_VARIABLE the above snipped is now execute= d, but then we already had else if (shape->expr_type =3D=3D EXPR_VARIABLE && shape->ref && shape->ref->u.ar.type =3D=3D AR_FULL && shape->ref->u.ar.dimen =3D= =3D 1 && shape->ref->u.ar.as && shape->ref->u.ar.as->lower[0]->expr_type =3D=3D EXPR_CONSTANT && shape->ref->u.ar.as->lower[0]->ts.type =3D=3D BT_INTEGER && shape->ref->u.ar.as->upper[0]->expr_type =3D=3D EXPR_CONSTANT && shape->ref->u.ar.as->upper[0]->ts.type =3D=3D BT_INTEGER && shape->symtree->n.sym->attr.flavor =3D=3D FL_PARAMETER && shape->symtree->n.sym->value) In which situations do I miss anything new? > Actually, this only supports the case where the parameter value is > defined by an array; but it could be an intrinsic call, a sum of > parameters, a reference to an other parameter, etc. E.g. the following (still) does get rejected: print *, reshape([1,2,3,4,5], a+1) print *, reshape([1,2,3,4,5], a+a) print *, reshape([1,2,3,4,5], 2*a) print *, reshape([1,2,3,4,5], [3,3]) print *, reshape([1,2,3,4,5], spread(3,dim=3D1,ncopies=3D2)) and has been rejected before. > The usual way to handle this is to call gfc_reduce_init_expr which (pray > for it) will make an array out of whatever the shape expression is. Can you give an example where it fails? I think the current code would almost certainly fail, too. > The rest looks good. > In the test, can you add a comment telling what it is testing? > Something like: "This tests that constant shape expressions passed to > the reshape intrinsic are properly simplified before being used to > diagnose invalid values" Can do. > We also used to put a comment mentioning the person who submitted the > test, but not everybody seems to do it these days. Can do. > Mikael > Harald From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ciao.gmane.io (ciao.gmane.io [116.202.254.214]) by sourceware.org (Postfix) with ESMTPS id 454FC3858410 for ; Thu, 25 Nov 2021 20:03:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 454FC3858410 Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1mqKxy-0005z9-0r for gcc-patches@gcc.gnu.org; Thu, 25 Nov 2021 21:03:34 +0100 X-Injected-Via-Gmane: http://gmane.org/ To: gcc-patches@gcc.gnu.org From: Harald Anlauf Subject: Re: [PATCH] PR fortran/103411 - ICE in gfc_conv_array_initializer, at fortran/trans-array.c:6377 Date: Thu, 25 Nov 2021 21:03:20 +0100 Message-ID: <24ff1ac7-79ce-2843-a199-9e85865304f6@gmx.de> References: <4fb65fbe-c10f-3fd4-9961-9978ff386bf9@orange.fr> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 In-Reply-To: <4fb65fbe-c10f-3fd4-9961-9978ff386bf9@orange.fr> Content-Language: en-US Cc: fortran@gcc.gnu.org X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_NUMSUBJECT, NICE_REPLY_A, 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Nov 2021 20:03:38 -0000 Message-ID: <20211125200320.CHAzTNMyRlDf62jSgWTOEcP11CBwj_SVZ6xaBP6QEqE@z> Hi Mikael, Am 25.11.21 um 17:46 schrieb Mikael Morin: > Hello, > > Le 24/11/2021 à 22:32, Harald Anlauf via Fortran a écrit : >> diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c >> index 5a5aca10ebe..837eb0912c0 100644 >> --- a/gcc/fortran/check.c >> +++ b/gcc/fortran/check.c >> @@ -4866,10 +4868,17 @@ gfc_check_reshape (gfc_expr *source, gfc_expr >> *shape, >>      { >>        gfc_constructor *c; >>        bool test; >> +      gfc_constructor_base b; >> >> +      if (shape->expr_type == EXPR_ARRAY) >> +        b = shape->value.constructor; >> +      else if (shape->expr_type == EXPR_VARIABLE) >> +        b = shape->symtree->n.sym->value->value.constructor; > > This misses a check that shape->symtree->n.sym->value is an array, so > that it makes sense to access its constructor. there are checks further above for the cases shape->expr_type == EXPR_ARRAY and for shape->expr_type == EXPR_VARIABLE which look at the elements of array shape to see if they are non-negative. Only in those cases where the full "if ()'s" pass we set shape_is_const = true; and proceed. The purpose of the auxiliary bool shape_is_const is to avoid repeating the lengthy if's again. Only then the above cited code segment should get executed. For shape->expr_type == EXPR_ARRAY there is really no change in logic. For shape->expr_type == EXPR_VARIABLE the above snipped is now executed, but then we already had else if (shape->expr_type == EXPR_VARIABLE && shape->ref && shape->ref->u.ar.type == AR_FULL && shape->ref->u.ar.dimen == 1 && shape->ref->u.ar.as && shape->ref->u.ar.as->lower[0]->expr_type == EXPR_CONSTANT && shape->ref->u.ar.as->lower[0]->ts.type == BT_INTEGER && shape->ref->u.ar.as->upper[0]->expr_type == EXPR_CONSTANT && shape->ref->u.ar.as->upper[0]->ts.type == BT_INTEGER && shape->symtree->n.sym->attr.flavor == FL_PARAMETER && shape->symtree->n.sym->value) In which situations do I miss anything new? > Actually, this only supports the case where the parameter value is > defined by an array; but it could be an intrinsic call, a sum of > parameters, a reference to an other parameter, etc. E.g. the following (still) does get rejected: print *, reshape([1,2,3,4,5], a+1) print *, reshape([1,2,3,4,5], a+a) print *, reshape([1,2,3,4,5], 2*a) print *, reshape([1,2,3,4,5], [3,3]) print *, reshape([1,2,3,4,5], spread(3,dim=1,ncopies=2)) and has been rejected before. > The usual way to handle this is to call gfc_reduce_init_expr which (pray > for it) will make an array out of whatever the shape expression is. Can you give an example where it fails? I think the current code would almost certainly fail, too. > The rest looks good. > In the test, can you add a comment telling what it is testing? > Something like: "This tests that constant shape expressions passed to > the reshape intrinsic are properly simplified before being used to > diagnose invalid values" Can do. > We also used to put a comment mentioning the person who submitted the > test, but not everybody seems to do it these days. Can do. > Mikael > Harald