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 B6F283858D28; Thu, 21 Mar 2024 20:59:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B6F283858D28 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=gmx.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmx.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B6F283858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=212.227.15.18 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711054765; cv=none; b=FA4pU4gPhjsFuMCXc9xjwQ1sxUeBQP3f0yjkVTXRivIJ1Qu0koVrW4eV1jmkofXrTOvxiC7OG9m2H0JxI42tWo+TCIg8433WWnc61UeXey19ILvwOEM08W6+VZlOG2Bp7j0WvolwQ0unRGB23DihqxCRmkhO56d2Pnfykfymgm0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711054765; c=relaxed/simple; bh=wcr3KoRWbz6hi4PXHcubFaJzV2k3+w8S1fId0zVHxBE=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=pMsdVHJA25aCl/HW1DL/l2aJqw0pyq6O2pHq+o/UTlwMuwN1mDsuq696k4hZkDTR9WLVNZEOkUxgQwzICJnhmpdHKs2lGax3k4VMuHTW071GID509EmCH5kMuSbbUXO/RKkV+dFswvdbAMrysykP/l8HaxH7ucv0AY2bNBt1EDg= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmx.de; s=s31663417; t=1711054761; x=1711659561; i=anlauf@gmx.de; bh=i9ZuYzYbtSSA/kq9W7T0E8xTWkjPRaVC955/TzEDW+k=; h=X-UI-Sender-Class:Date:Subject:To:References:From:In-Reply-To; b=Pvmj/x4876XwRVwfGHxfuL3tAB3+P/YupLDTHz7Y0TbEJjpE3Y6HoAri7EM4yq9L X1EBHRXGdwo/c9Pu+VV5AWpyqW7wPXn7/d7a6n2POJgEfNKNowtJKRHCpqw1OcgWo Iq61J3QcI906SS8WvUAFlQwGhY+YKdHlvBBmqYZrO8f/UCYWaWfD8rrjXZ1swmty+ xVRXekTebP3RBFu/P/2Ca3V9Yjvn+MxS3fzvNOnGtwA2XkiHy6IyLjhD0Nph6Elkk ddqvfnew5FndmVkY8F+uogLSKRiAGcZhsIhwow9ysoKAF7JcxOwR7BffSskAqLJhC Vk59rnG2BVPHDj2uIg== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from [192.168.178.29] ([93.207.91.12]) by mail.gmx.net (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MIx3I-1rUDh50x5e-00KTID; Thu, 21 Mar 2024 21:59:21 +0100 Message-ID: <57f0a554-c6a7-41ad-a5a4-98e49c9b1380@gmx.de> Date: Thu, 21 Mar 2024 21:59:19 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] fortran: Ignore use statements on error [PR107426] To: Mikael Morin , fortran@gcc.gnu.org, gcc-patches@gcc.gnu.org Newsgroups: gmane.comp.gcc.patches,gmane.comp.gcc.fortran References: <20240321162754.1353561-1-mikael@gcc.gnu.org> Content-Language: en-US From: Harald Anlauf In-Reply-To: <20240321162754.1353561-1-mikael@gcc.gnu.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:4PF8fwtzUsLtLW9kx1f4JsEmH+7rq98QqXd88vsrqLHj6Cw4x61 tyZ5fEF0JzYVd1ga7j4BAVGk2XzFqIsyUmWKwH5vexHMFjcwizeJZus2/klEKJBZhbMHqsQ 73l5fCpDBP1yeuL5PSp/Fe0rG47nbh4pObgkHe88KA7ANcSBSYoJCIY8QrFFZ8eqyM7qUik q33MksGYNWWkXLk5dAZkg== UI-OutboundReport: notjunk:1;M01:P0:N24Qc2ixDMo=;KnQ8jXttcXaWq5a94ybewkdKpdy ooMRhmWBtdlT4quqLavFteJ++bS0i8Z7ErmwvUArJTh9GfKVohgLpTegn7o85YYfZo4xDL+K5 v5OKjgwCk1w3F8ppacOtks+dBc7SzF1m/O/xz4egEbiMd4lGHN2WR0GuNHLZPgSlM7fl37Kuc 61gNFQVBg6b+q/tA3nj3ftuceeMmp3rrmoFEo9SvPuupVtcNBv1wmUDt/yLmQd3Ajijd84qIR Svmcx+7aCoflKa9RiolXWQmceyMeWPelonzTkpd0AKuiAhKg4LmtNTGyjVe1Kda3hrECiSoa9 f3ZCqYmkFAPRz4V72iEZAJYJOgBBJd102o2HXE7fWj8X3GdaiyMeO1DZSbNfR3vZkppemwcdd t141kvgs0pL5Vbwd5yY6hhDBLUXHHzOYZ5MekWEMnaQO7e1GGRz/nzgWT5i9bRYdX0fE/Rk0H DeK3AmJYa3KVb4+/QYFTTC7NqBVPVbeO7HOEfGkhfhob5AinQNl2d9uRwCltE1hMbrfeRaeqf ecVowT2rsgrv5Q0hrQvvId8mTK6E7OJNguqnktJUpHop09WgJErRIgOQ6YBfxxnoggJejk/i9 aALUJNpaBr5vjZHDvyVTV/T2/1NfTrnfq4wOvkR4N3HOIpSr+HqMlOiDRSFpTORuSTtRzgO2W yWiTXSBfaEHTAkRFfkn8P0dtOU+5oG8hP4hA/kqvx/4Y/V1I0JEnZ6Pc0qBNpuWwZH3G798d6 JVuSmI4UNH88zSK8mtNZfzqNA7Y8AmhUI3mBUxUz9FLUh19UMuElFS7rBT/J0zdmvBRmxCFrH fY6EBo8sZ6PltNf6b4hD4ES/t3yfgeOLqs//l0RFXf5cY= X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Mikael, this looks all good to me. I wouldn't mind the minor side-effects of better error recovery, as you are (successfully) trying hard to keep the namespaces sane. So OK for mainline. Thanks for the patch! Harald On 3/21/24 17:27, Mikael Morin wrote: > Hello, > > here is a fix for an ICE caused by dangling pointers to ISO_C_BINDING's > C_PTR symbol in the global intrinsic symbol for C_LOC. > I tried to fix it by making the intrinsic symbol use its own copy of > C_PTR, but it regressed heavily. > > Instead, I propose this which is based on a patch I attached to the PR > one year ago. It's sufficient to remove the access to freed memory. > > However, an underlying problem remains that successive use-associations > of ISO_C_BINDING's symbols in different scopes cause the return type > of the C_LOC global intrinsic symbol to be set to the C_PTR from each > scope successively, with the last one "winning". Not very pretty. > > Anyway, there are two changed messages in the testsuite as a side-effect > of the proposed change. I regard them as acceptable, albeit slightly wo= rse. > No regression otherwise on x86_64-pc-linux-gnu. > Ok for 14 master? > > Mikael > > -- >8 -- > > This fixes an access to freed memory on the testcase from the PR. > The problem comes from an invalid subroutine statement in an interface, > which is ignored and causes the following statements forming the procedu= re > body to be rejected. One of them use-associates the intrinsic ISO_C_BIN= DING > module, which imports new symbols in a namespace that is freed at the ti= me > the statement is rejected. However, this creates dangling pointers as > ISO_C_BINDING is special and its import creates a reference to the impor= ted > C_PTR symbol in the return type of the global intrinsic symbol for C_LOC > (see the function create_intrinsic_function). > > This change saves and restores the list of use statements, so that rejec= ted > use statements are removed before they have a chance to be applied to th= e > current namespace and create dangling pointers. > > PR fortran/107426 > > gcc/fortran/ChangeLog: > > * gfortran.h (gfc_save_module_list, gfc_restore_old_module_list): > New declarations. > * module.cc (old_module_list_tail): New global variable. > (gfc_save_module_list, gfc_restore_old_module_list): New functions. > (gfc_use_modules): Set module_list and old_module_list_tail. > * parse.cc (next_statement): Save module_list before doing any work. > (reject_statement): Restore module_list to its saved value. > > gcc/testsuite/ChangeLog: > > * gfortran.dg/pr89943_3.f90: Update error pattern. > * gfortran.dg/pr89943_4.f90: Likewise. > * gfortran.dg/use_31.f90: New test. > --- > gcc/fortran/gfortran.h | 2 ++ > gcc/fortran/module.cc | 31 +++++++++++++++++++++++++ > gcc/fortran/parse.cc | 4 ++++ > gcc/testsuite/gfortran.dg/pr89943_3.f90 | 2 +- > gcc/testsuite/gfortran.dg/pr89943_4.f90 | 2 +- > gcc/testsuite/gfortran.dg/use_31.f90 | 25 ++++++++++++++++++++ > 6 files changed, 64 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gfortran.dg/use_31.f90 > > diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h > index c7039730fad..fec7b53ff1a 100644 > --- a/gcc/fortran/gfortran.h > +++ b/gcc/fortran/gfortran.h > @@ -3926,6 +3926,8 @@ void gfc_module_done_2 (void); > void gfc_dump_module (const char *, int); > bool gfc_check_symbol_access (gfc_symbol *); > void gfc_free_use_stmts (gfc_use_list *); > +void gfc_save_module_list (); > +void gfc_restore_old_module_list (); > const char *gfc_dt_lower_string (const char *); > const char *gfc_dt_upper_string (const char *); > > diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc > index d1de53cbdb4..c565b84d61b 100644 > --- a/gcc/fortran/module.cc > +++ b/gcc/fortran/module.cc > @@ -195,7 +195,12 @@ static const char *module_name; > /* The name of the .smod file that the submodule will write to. */ > static const char *submodule_name; > > +/* The list of use statements to apply to the current namespace > + before parsing the non-use statements. */ > static gfc_use_list *module_list; > +/* The end of the MODULE_LIST list above at the time the recognition > + of the current statement started. */ > +static gfc_use_list **old_module_list_tail; > > /* If we're reading an intrinsic module, this is its ID. */ > static intmod_id current_intmod; > @@ -7561,6 +7566,8 @@ gfc_use_modules (void) > gfc_use_module (module_list); > free (module_list); > } > + module_list =3D NULL; > + old_module_list_tail =3D &module_list; > gfc_rename_list =3D NULL; > } > > @@ -7584,6 +7591,30 @@ gfc_free_use_stmts (gfc_use_list *use_stmts) > } > > > +/* Remember the end of the MODULE_LIST list, so that the list can be re= stored > + to its previous state if the current statement is erroneous. */ > + > +void > +gfc_save_module_list () > +{ > + gfc_use_list **tail =3D &module_list; > + while (*tail !=3D NULL) > + tail =3D &(*tail)->next; > + old_module_list_tail =3D tail; > +} > + > + > +/* Restore the MODULE_LIST list to its previous value and free the use > + statements that are no longer part of the list. */ > + > +void > +gfc_restore_old_module_list () > +{ > + gfc_free_use_stmts (*old_module_list_tail); > + *old_module_list_tail =3D NULL; > +} > + > + > void > gfc_module_init_2 (void) > { > diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc > index a2bf328f681..79c810c86ba 100644 > --- a/gcc/fortran/parse.cc > +++ b/gcc/fortran/parse.cc > @@ -1800,6 +1800,7 @@ next_statement (void) > locus old_locus; > > gfc_enforce_clean_symbol_state (); > + gfc_save_module_list (); > > gfc_new_block =3D NULL; > > @@ -3104,6 +3105,9 @@ reject_statement (void) > > gfc_reject_data (gfc_current_ns); > > + /* Don't queue use-association of a module if we reject the use state= ment. */ > + gfc_restore_old_module_list (); > + > gfc_new_block =3D NULL; > gfc_undo_symbols (); > gfc_clear_warning (); > diff --git a/gcc/testsuite/gfortran.dg/pr89943_3.f90 b/gcc/testsuite/gfo= rtran.dg/pr89943_3.f90 > index 38b723e2458..84a9fb74741 100644 > --- a/gcc/testsuite/gfortran.dg/pr89943_3.f90 > +++ b/gcc/testsuite/gfortran.dg/pr89943_3.f90 > @@ -22,7 +22,7 @@ submodule(Foo_mod) Foo_smod > module subroutine runFoo4C(ndim) bind(C, name=3D"runFu") ! { d= g-error "Mismatch in BIND" } > use, intrinsic :: iso_c_binding ! { dg-error = "Unexpected USE statement" } > implicit none ! { dg-error = "Unexpected IMPLICIT NONE statement" } > - integer(c_int32_t) , intent(in) :: ndim ! { dg-error "= Unexpected data declaration" } > + integer(c_int32_t) , intent(in) :: ndim ! { dg-error "= Symbol 'c_int32_t' at .1. has no IMPLICIT type" } > end subroutine runFoo4C ! { dg-error = " Expecting END SUBMODULE" } > > end submodule Foo_smod > diff --git a/gcc/testsuite/gfortran.dg/pr89943_4.f90 b/gcc/testsuite/gfo= rtran.dg/pr89943_4.f90 > index 8eba2eda171..cb955d01c88 100644 > --- a/gcc/testsuite/gfortran.dg/pr89943_4.f90 > +++ b/gcc/testsuite/gfortran.dg/pr89943_4.f90 > @@ -23,7 +23,7 @@ submodule(Foo_mod) Foo_smod > module function runFoo4C(ndim) bind(C, name=3D"runFu") ! { dg-e= rror "Mismatch in BIND" } > use, intrinsic :: iso_c_binding ! { dg-error "Unexpected = USE statement in" } > implicit none ! { dg-error "Unexpected = IMPLICIT NONE statement" } > - integer(c_int32_t) , intent(in) :: ndim ! { dg-error "Unexpe= cted data declaration" } > + integer(c_int32_t) , intent(in) :: ndim ! { dg-error "Symbol= 'c_int32_t' at .1. has no IMPLICIT type" } > end function runFoo4C ! { dg-error "Expecting E= ND SUBMODULE" } > > end submodule Foo_smod > diff --git a/gcc/testsuite/gfortran.dg/use_31.f90 b/gcc/testsuite/gfortr= an.dg/use_31.f90 > new file mode 100644 > index 00000000000..818f2e30b09 > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/use_31.f90 > @@ -0,0 +1,25 @@ > +! { dg-do compile } > +! > +! PR fortran/107426 > +! This example used to generate an ICE, caused by the use stmt from the= nested > +! procedure declaration wrongly applying to the host procedure and over= writing > +! the symbols there. > +! > +! Contributed by Gerhard Steinmetz > + > +module m > +contains > + subroutine p() bind(c) > + use, intrinsic :: iso_c_binding > + integer, target :: a =3D 1 > + type(c_ptr) :: z > + interface > + subroutine s(x) bind(cc) ! { dg-error "Missing clos= ing paren" } > + use, intrinsic :: iso_c_binding ! { dg-error "Unexpected U= SE statement in INTERFACE block" } > + integer(c_int), value :: x ! { dg-error "Parameter 'c= _int' at .1. has not been declared" } > + end ! { dg-error "END INTERFAC= E statement expected" } > + end interface > + z =3D c_loc(a) > + call s(z) > + end > +end