[PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[Sergei Trofimovich]

From: Sergei Trofimovich <siarheit@google.com>

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.

	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.
---
 gcc/cp/typeck.c                         |  6 +++++-
 gcc/testsuite/g++.dg/torture/pr101219.C | 10 ++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/torture/pr101219.C

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index a483e1f988d..aa91fd21c7b 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -3326,7 +3326,11 @@ build_ptrmemfunc_access_expr (tree ptrmem, tree member_name)
        member = DECL_CHAIN (member))
     if (DECL_NAME (member) == member_name)
       break;
-  return build_simple_component_ref (ptrmem, member);
+  tree r = build_simple_component_ref (ptrmem, member);
+  /* Suppress warning to avoid exposing missing BINFO for ptrmem
+     synthetic structs: PR101219.  */
+  suppress_warning(r);
+  return r;
 }
 
 /* Given an expression PTR for a pointer, return an expression
diff --git a/gcc/testsuite/g++.dg/torture/pr101219.C b/gcc/testsuite/g++.dg/torture/pr101219.C
new file mode 100644
index 00000000000..c8d30448187
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr101219.C
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-Wall" } */
+struct S { void m(); };
+
+template <int> bool f() {
+  /* In PR101219 gcc used to ICE in warning code. */
+  void (S::*mp)();
+
+  return &S::m == mp;
+}
-- 
2.32.0

Re: [PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[Jeff Law]

On 7/22/2021 5:15 PM, Sergei Trofimovich via Gcc-patches wrote:
> From: Sergei Trofimovich <siarheit@google.com>
>
> 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.
>
> 	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.
The C++ maintainers have the final say here, but ISTM that warning 
suppression shouldn't be used to avoid an ICE, even an ICE within the 
warning or diagnostic code itself.

jeff

Re: [PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[Sergei Trofimovich]

On Fri, 23 Jul 2021 10:33:09 -0600
Jeff Law <jeffreyalaw@gmail.com> wrote:

> On 7/22/2021 5:15 PM, Sergei Trofimovich via Gcc-patches wrote:
> > From: Sergei Trofimovich <siarheit@google.com>
> >
> > 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.
> >
> > 	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.  
> The C++ maintainers have the final say here, but ISTM that warning 
> suppression shouldn't be used to avoid an ICE, even an ICE within the 
> warning or diagnostic code itself.

Sounds good. I agree fixing it correctly is preferable and should improve
diagnostic on this very test case compared to gcc-11.

I'll need some help plumbing TYPE_BINFO() around build_ptrmemfunc_type().
My attempts to use `xref_basetypes (t, NULL_TREE);` to derive it for a
fresh expression only shuffles ICEs around.

-- 

  Sergei

Re: [PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[Jason Merrill]

On 7/22/21 7:15 PM, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <siarheit@google.com>
> 
> 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.

> 	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.

> ---
>   gcc/cp/typeck.c                         |  6 +++++-
>   gcc/testsuite/g++.dg/torture/pr101219.C | 10 ++++++++++
>   2 files changed, 15 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/torture/pr101219.C
> 
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index a483e1f988d..aa91fd21c7b 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -3326,7 +3326,11 @@ build_ptrmemfunc_access_expr (tree ptrmem, tree member_name)
>          member = DECL_CHAIN (member))
>       if (DECL_NAME (member) == member_name)
>         break;
> -  return build_simple_component_ref (ptrmem, member);
> +  tree r = build_simple_component_ref (ptrmem, member);
> +  /* Suppress warning to avoid exposing missing BINFO for ptrmem
> +     synthetic structs: PR101219.  */
> +  suppress_warning(r);
> +  return r;
>   }
>   
>   /* Given an expression PTR for a pointer, return an expression
> diff --git a/gcc/testsuite/g++.dg/torture/pr101219.C b/gcc/testsuite/g++.dg/torture/pr101219.C
> new file mode 100644
> index 00000000000..c8d30448187
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/torture/pr101219.C
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-Wall" } */
> +struct S { void m(); };
> +
> +template <int> bool f() {
> +  /* In PR101219 gcc used to ICE in warning code. */
> +  void (S::*mp)();
> +
> +  return &S::m == mp;
> +}
> 

Re: [PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 2940 bytes --]

On Thu, 29 Jul 2021 11:41:39 -0400
Jason Merrill <jason@redhat.com> wrote:

> On 7/22/21 7:15 PM, Sergei Trofimovich wrote:
> > From: Sergei Trofimovich <siarheit@google.com>
> > 
> > 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 <int> 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

[-- Attachment #2: v2-0001-c-fix-ptrmemfunc-template-instantiation-PR101219.patch --]
[-- Type: text/x-patch, Size: 2579 bytes --]

From 9c51dbc598d8633167729de9637c8cdb5f3089fe Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>
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 <siarheit@google.com>
---
 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 <int> bool f() {
+  void (S::*mp)();
+
+  return &S::m == mp; // no warning emitted here (no instantiation)
+}
-- 
2.32.0

Re: [PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[Jason Merrill]

On 8/6/21 11:34 AM, Sergei Trofimovich wrote:
> On Thu, 29 Jul 2021 11:41:39 -0400
> Jason Merrill <jason@redhat.com> wrote:
> 
>> On 7/22/21 7:15 PM, Sergei Trofimovich wrote:
>>> From: Sergei Trofimovich <siarheit@google.com>
>>>
>>> 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 <int> 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.

> +	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.

Jason

Re: [PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 4007 bytes --]

On Wed, 11 Aug 2021 15:19:58 -0400
Jason Merrill <jason@redhat.com> wrote:

> On 8/6/21 11:34 AM, Sergei Trofimovich wrote:
> > On Thu, 29 Jul 2021 11:41:39 -0400
> > Jason Merrill <jason@redhat.com> wrote:
> >   
> >> On 7/22/21 7:15 PM, Sergei Trofimovich wrote:  
> >>> From: Sergei Trofimovich <siarheit@google.com>
> >>>
> >>> 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 <int> 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

[-- Attachment #2: v3-0001-c-fix-ptrmemfunc-template-instantiation-PR101219.patch --]
[-- Type: text/x-patch, Size: 2571 bytes --]

From dbb17a22383faa7837bdd2ea9c902bfab53fa8f2 Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>
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 <int> bool f() {
+  void (S::*mp)();
+
+  return &S::m == mp; // no warning emitted here (no instantiation)
+}
-- 
2.32.0

Re: [PATCH] c++: suppress all warnings on memper pointers to work around dICE [PR101219]

[Jason Merrill]

On 8/11/21 6:36 PM, Sergei Trofimovich wrote:
> On Wed, 11 Aug 2021 15:19:58 -0400
> Jason Merrill <jason@redhat.com> wrote:
> 
>> On 8/6/21 11:34 AM, Sergei Trofimovich wrote:
>>> On Thu, 29 Jul 2021 11:41:39 -0400
>>> Jason Merrill <jason@redhat.com> wrote:
>>>    
>>>> On 7/22/21 7:15 PM, Sergei Trofimovich wrote:
>>>>> From: Sergei Trofimovich <siarheit@google.com>
>>>>>
>>>>> 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 <int> 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.

OK, thanks.

Jason