From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.gentoo.org (dev.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) by sourceware.org (Postfix) with ESMTP id D38F3385801A; Wed, 11 Aug 2021 22:36:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D38F3385801A Date: Wed, 11 Aug 2021 23:36:42 +0100 From: Sergei Trofimovich To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, Martin Sebor , Sergei Trofimovich Subject: Re: [PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219] Message-ID: <20210811233642.55961437@zn3> In-Reply-To: <116cc93c-0839-87bd-ee30-bec924c9c9be@redhat.com> References: <20210722231524.2045951-1-slyfox@gentoo.org> <67b15096-25fd-bf23-0be2-57af76462166@redhat.com> <20210806163428.06373da6@zn3> <116cc93c-0839-87bd-ee30-bec924c9c9be@redhat.com> X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.29; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/_wIJbg2x+J_/ltQ37sL/3R4" X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_PASS, 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: Wed, 11 Aug 2021 22:36:49 -0000 --MP_/_wIJbg2x+J_/ltQ37sL/3R4 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Disposition: inline On Wed, 11 Aug 2021 15:19:58 -0400 Jason Merrill wrote: > On 8/6/21 11:34 AM, Sergei Trofimovich wrote: > > On Thu, 29 Jul 2021 11:41:39 -0400 > > Jason Merrill wrote: > > > >> On 7/22/21 7:15 PM, Sergei Trofimovich wrote: > >>> From: Sergei Trofimovich > >>> > >>> r12-1804 ("cp: add support for per-location warning groups.") among other > >>> things removed warning suppression from a few places including ptrmemfuncs. > >>> > >>> Currently ptrmemfuncs don't have valid BINFO attached which causes ICEs > >>> in access checks: > >>> > >>> crash_signal > >>> gcc/toplev.c:328 > >>> perform_or_defer_access_check(tree_node*, tree_node*, tree_node*, int, access_failure_info*) > >>> gcc/cp/semantics.c:490 > >>> finish_non_static_data_member(tree_node*, tree_node*, tree_node*) > >>> gcc/cp/semantics.c:2208 > >>> ... > >>> > >>> The change suppresses warnings again until we provide BINFOs for ptrmemfuncs. > >> > >> We don't need BINFOs for PMFs, we need to avoid paths that expect them. > >> > >> It looks like the problem is with tsubst_copy_and_build calling > >> finish_non_static_data_member instead of build_ptrmemfunc_access_expr. > > > > Sounds good. I'm not sure what would be the best way to match it. Here is > > my attempt seems to survive all regtests: > > > > --- a/gcc/cp/pt.c > > +++ b/gcc/cp/pt.c > > @@ -20530,7 +20530,13 @@ tsubst_copy_and_build (tree t, > > if (member == error_mark_node) > > RETURN (error_mark_node); > > > > - if (TREE_CODE (member) == FIELD_DECL) > > + if (object_type && TYPE_PTRMEMFUNC_P(object_type) > > + && TREE_CODE (member) == FIELD_DECL) > > + { > > + r = build_ptrmemfunc_access_expr (object, DECL_NAME(member)); > > + RETURN (r); > > + } > > + else if (TREE_CODE (member) == FIELD_DECL) > > { > > r = finish_non_static_data_member (member, object, NULL_TREE); > > if (TREE_CODE (r) == COMPONENT_REF) > > > >>> PR c++/101219 > >>> > >>> gcc/cp/ChangeLog: > >>> > >>> * typeck.c (build_ptrmemfunc_access_expr): Suppress all warnings > >>> to avoid ICE. > >>> > >>> gcc/testsuite/ChangeLog: > >>> > >>> * g++.dg/torture/pr101219.C: New test. > >> > >> This doesn't need to be in torture; it has nothing to do with optimization. > > > > Aha, moved to gcc/testsuite/g++.dg/warn/pr101219.C. > > > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/warn/pr101219.C > > @@ -0,0 +1,11 @@ > > +/* PR c++/101219 - ICE on use of uninitialized memfun pointer > > + { dg-do compile } > > + { dg-options "-Wall" } */ > > + > > +struct S { void m(); }; > > + > > +template bool f() { > > + void (S::*mp)(); > > + > > + return &S::m == mp; // no warning emitted here (no instantiation) > > +} > > > > Another question: Is it expected that gcc generates no warnings here? > > It's an uninstantiated function (-1 for warn), but from what I > > understand it's guaranteed to generate comparison with uninitialized > > data if it ever gets instantiated. Given that we used to ICE in > > warning code gcc could possibly flag it? (+1 for warn) > > Generally it's desirable to diagnose templates for which no valid > instantiation is possible. It seems reasonable in most cases to also > warn about templates for which all instantiations would warn. > > But uninitialized warnings rely on flow analysis that we only do on > instantiated functions, and in any case the ICE doesn't depend on mp > being uninitialized; I get the same crash if I add = 0 to the declaration. Aha. That makes sense. Let's just fix ICE then. > > + if (object_type && TYPE_PTRMEMFUNC_P(object_type) > > Missing space before (. > > > + && TREE_CODE (member) == FIELD_DECL) > > + { > > + r = build_ptrmemfunc_access_expr (object, DECL_NAME(member)); > > And here. Added both. Attached as v3. -- Sergei --MP_/_wIJbg2x+J_/ltQ37sL/3R4 Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=v3-0001-c-fix-ptrmemfunc-template-instantiation-PR101219.patch >From dbb17a22383faa7837bdd2ea9c902bfab53fa8f2 Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Fri, 6 Aug 2021 16:14:16 +0100 Subject: [PATCH v3] c++: fix ptrmemfunc template instantiation [PR101219] r12-1804 ("cp: add support for per-location warning groups.") among other things removed warning suppression from a few places including ptrmemfuncs. This exposed a bug in warning detection code as a reference to missing BINFO (it's intentionally missing for ptrmemfunc types): crash_signal gcc/toplev.c:328 perform_or_defer_access_check(tree_node*, tree_node*, tree_node*, int, access_failure_info*) gcc/cp/semantics.c:490 finish_non_static_data_member(tree_node*, tree_node*, tree_node*) gcc/cp/semantics.c:2208 ... The change special cases ptrmemfuncs in templace substitution by using build_ptrmemfunc_access_expr() instead of finish_non_static_data_member(). PR c++/101219 gcc/cp/ChangeLog: * pt.c (tsubst_copy_and_build): Use build_ptrmemfunc_access_expr to construct ptrmemfunc expression instantiation. gcc/testsuite/ChangeLog: * g++.dg/warn/pr101219.C: New test. --- Change since v2: fix whitespace around macros. gcc/cp/pt.c | 8 +++++++- gcc/testsuite/g++.dg/warn/pr101219.C | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/warn/pr101219.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index b396ddd0089..42ea51cebc0 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -20530,7 +20530,13 @@ tsubst_copy_and_build (tree t, if (member == error_mark_node) RETURN (error_mark_node); - if (TREE_CODE (member) == FIELD_DECL) + if (object_type && TYPE_PTRMEMFUNC_P (object_type) + && TREE_CODE (member) == FIELD_DECL) + { + r = build_ptrmemfunc_access_expr (object, DECL_NAME (member)); + RETURN (r); + } + else if (TREE_CODE (member) == FIELD_DECL) { r = finish_non_static_data_member (member, object, NULL_TREE); if (TREE_CODE (r) == COMPONENT_REF) diff --git a/gcc/testsuite/g++.dg/warn/pr101219.C b/gcc/testsuite/g++.dg/warn/pr101219.C new file mode 100644 index 00000000000..0d23d73c9ec --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr101219.C @@ -0,0 +1,11 @@ +/* PR c++/101219 - ICE on use of uninitialized memfun pointer + { dg-do compile } + { dg-options "-Wall" } */ + +struct S { void m(); }; + +template bool f() { + void (S::*mp)(); + + return &S::m == mp; // no warning emitted here (no instantiation) +} -- 2.32.0 --MP_/_wIJbg2x+J_/ltQ37sL/3R4--