From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.gentoo.org (smtp.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) by sourceware.org (Postfix) with ESMTP id DFF263854815; Fri, 6 Aug 2021 15:34:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DFF263854815 Date: Fri, 6 Aug 2021 16:34:28 +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: <20210806163428.06373da6@zn3> In-Reply-To: <67b15096-25fd-bf23-0be2-57af76462166@redhat.com> References: <20210722231524.2045951-1-slyfox@gentoo.org> <67b15096-25fd-bf23-0be2-57af76462166@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_/ixo8SIus3PejXJHw.fkgoZw" 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: Fri, 06 Aug 2021 15:34:35 -0000 --MP_/ixo8SIus3PejXJHw.fkgoZw Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Disposition: inline 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) Attached full patch as well. Full 'make check' shows no regressions on x86_64-linux. -- Sergei --MP_/ixo8SIus3PejXJHw.fkgoZw Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=v2-0001-c-fix-ptrmemfunc-template-instantiation-PR101219.patch >From 9c51dbc598d8633167729de9637c8cdb5f3089fe Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Fri, 6 Aug 2021 16:14:16 +0100 Subject: [PATCH v2] 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. Signed-off-by: Sergei Trofimovich --- 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..c7a0317cbfb 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_/ixo8SIus3PejXJHw.fkgoZw--