[PATCH] C++: Fix ICE in fold_for_warn on CAST_EXPR (PR c++/83974)

[PATCH] C++: Fix ICE in fold_for_warn on CAST_EXPR (PR c++/83974)

[David Malcolm]

PR c++/83974 reports an ICE within fold_for_warn when calling
cxx_eval_constant_expression on a CAST_EXPR.

This comes from a pointer-to-member-function.  The result of
build_ptrmemfunc (within cp_convert_to_pointer for a null ptr) is
a CONSTRUCTOR containing, amongst other things a CAST_EXPR of a
TREE_LIST containing the zero INTEGER_CST.

After r256804, fold_for_warn within a template calls
fold_non_dependent_expr.

For this tree, is_nondependent_constant_expression returns true.

potential_constant_expression_1 has these cases:

    case TREE_LIST:
      {
	gcc_assert (TREE_PURPOSE (t) == NULL_TREE
		    || DECL_P (TREE_PURPOSE (t)));
	if (!RECUR (TREE_VALUE (t), want_rval))
	  return false;
	if (TREE_CHAIN (t) == NULL_TREE)
	  return true;
	return RECUR (TREE_CHAIN (t), want_rval);
      }

and:

    case CAST_EXPR:
    case CONST_CAST_EXPR:
    case STATIC_CAST_EXPR:
    case REINTERPRET_CAST_EXPR:
    case IMPLICIT_CONV_EXPR:
      if (cxx_dialect < cxx11
	  && !dependent_type_p (TREE_TYPE (t))
	  && !INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t)))
	/* In C++98, a conversion to non-integral type can't be part of a
	   constant expression.  */
	  {
	  // reject it
	  }
	// accept it

and thus returns true for the CAST_EXPR and TREE_LIST, and hence for the
CONSTRUCTOR as a whole.

However, cxx_eval_constant_expression does not support these tree codes,
ICEing in the default case with:

  internal_error ("unexpected expression %qE of kind %s", t,

It seems that, given potential_constant_expression_1 can return true for
these cases, then cxx_eval_constant_expression ought to handle them, which
this patch implements, fixing the ICE.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
OK for trunk?

gcc/cp/ChangeLog:
	PR c++/83974
	* constexpr.c (cxx_eval_constant_expression): Handle case
	TREE_LIST.  Handle CAST_EXPR, CONST_CAST_EXPR, STATIC_CAST_EXPR,
	REINTERPRET_CAST_EXPR and IMPLICIT_CONV_EXPR via
	cxx_eval_unary_expression.

gcc/testsuite/ChangeLog:
	PR c++/83974
	* g++.dg/warn/pr83974.C: New test case.
---
 gcc/cp/constexpr.c                  | 27 +++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/warn/pr83974.C | 11 +++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/warn/pr83974.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index ca7f369..a593132 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4348,6 +4348,24 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
       }
       break;
 
+    case TREE_LIST:
+      {
+	tree value, chain;
+	gcc_assert (TREE_PURPOSE (t) == NULL_TREE
+		    || DECL_P (TREE_PURPOSE (t)));
+	value = cxx_eval_constant_expression (ctx, TREE_VALUE (t), lval,
+					      non_constant_p, overflow_p,
+					      jump_target);
+	if (TREE_CHAIN (t))
+	  chain = cxx_eval_constant_expression (ctx, TREE_CHAIN (t), lval,
+						non_constant_p, overflow_p,
+						jump_target);
+	else
+	  chain = NULL_TREE;
+	r = tree_cons (TREE_PURPOSE (t), value, chain);
+      }
+      break;
+
     case POINTER_PLUS_EXPR:
     case POINTER_DIFF_EXPR:
     case PLUS_EXPR:
@@ -4594,6 +4612,15 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
       return cxx_eval_statement_list (&new_ctx, t,
 				      non_constant_p, overflow_p, jump_target);
 
+    case CAST_EXPR:
+    case CONST_CAST_EXPR:
+    case STATIC_CAST_EXPR:
+    case REINTERPRET_CAST_EXPR:
+    case IMPLICIT_CONV_EXPR:
+      r = cxx_eval_unary_expression (ctx, t, lval,
+				     non_constant_p, overflow_p);
+      break;
+
     case BIND_EXPR:
       return cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
 					   lval,
diff --git a/gcc/testsuite/g++.dg/warn/pr83974.C b/gcc/testsuite/g++.dg/warn/pr83974.C
new file mode 100644
index 0000000..af12c2d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr83974.C
@@ -0,0 +1,11 @@
+// { dg-options "-Wtautological-compare" }
+
+struct A {
+  typedef void (A::*B) ();
+  operator B ();
+};
+template <typename>
+struct C {
+  void foo () { d == 0; }
+  A d;
+};
-- 
1.8.5.3

Re: [PATCH] C++: Fix ICE in fold_for_warn on CAST_EXPR (PR c++/83974)

[Jason Merrill]

On Tue, Jan 23, 2018 at 9:27 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> PR c++/83974 reports an ICE within fold_for_warn when calling
> cxx_eval_constant_expression on a CAST_EXPR.
>
> This comes from a pointer-to-member-function.  The result of
> build_ptrmemfunc (within cp_convert_to_pointer for a null ptr) is
> a CONSTRUCTOR containing, amongst other things a CAST_EXPR of a
> TREE_LIST containing the zero INTEGER_CST.
>
> After r256804, fold_for_warn within a template calls
> fold_non_dependent_expr.
>
> For this tree, is_nondependent_constant_expression returns true.
>
> potential_constant_expression_1 has these cases:
>
>     case TREE_LIST:
>       {
>         gcc_assert (TREE_PURPOSE (t) == NULL_TREE
>                     || DECL_P (TREE_PURPOSE (t)));
>         if (!RECUR (TREE_VALUE (t), want_rval))
>           return false;
>         if (TREE_CHAIN (t) == NULL_TREE)
>           return true;
>         return RECUR (TREE_CHAIN (t), want_rval);
>       }
>
> and:
>
>     case CAST_EXPR:
>     case CONST_CAST_EXPR:
>     case STATIC_CAST_EXPR:
>     case REINTERPRET_CAST_EXPR:
>     case IMPLICIT_CONV_EXPR:
>       if (cxx_dialect < cxx11
>           && !dependent_type_p (TREE_TYPE (t))
>           && !INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t)))
>         /* In C++98, a conversion to non-integral type can't be part of a
>            constant expression.  */
>           {
>           // reject it
>           }
>         // accept it
>
> and thus returns true for the CAST_EXPR and TREE_LIST, and hence for the
> CONSTRUCTOR as a whole.
>
> However, cxx_eval_constant_expression does not support these tree codes,

Because they are template-only codes, that
cxx_eval_constant_expression should never see.  They shouldn't survive
the call to instantiate_non_dependent_expr_internal from
fold_non_dependent_expr.

Jason

[PATCH v2] C++: Fix ICE in fold_for_warn on CAST_EXPR (PR c++/83974)

[David Malcolm]

On Tue, 2018-01-23 at 09:54 -0500, Jason Merrill wrote:
> On Tue, Jan 23, 2018 at 9:27 AM, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > PR c++/83974 reports an ICE within fold_for_warn when calling
> > cxx_eval_constant_expression on a CAST_EXPR.
> > 
> > This comes from a pointer-to-member-function.  The result of
> > build_ptrmemfunc (within cp_convert_to_pointer for a null ptr) is
> > a CONSTRUCTOR containing, amongst other things a CAST_EXPR of a
> > TREE_LIST containing the zero INTEGER_CST.
> > 
> > After r256804, fold_for_warn within a template calls
> > fold_non_dependent_expr.
> > 
> > For this tree, is_nondependent_constant_expression returns true.
> > 
> > potential_constant_expression_1 has these cases:
> > 
> >     case TREE_LIST:
> >       {
> >         gcc_assert (TREE_PURPOSE (t) == NULL_TREE
> >                     || DECL_P (TREE_PURPOSE (t)));
> >         if (!RECUR (TREE_VALUE (t), want_rval))
> >           return false;
> >         if (TREE_CHAIN (t) == NULL_TREE)
> >           return true;
> >         return RECUR (TREE_CHAIN (t), want_rval);
> >       }
> > 
> > and:
> > 
> >     case CAST_EXPR:
> >     case CONST_CAST_EXPR:
> >     case STATIC_CAST_EXPR:
> >     case REINTERPRET_CAST_EXPR:
> >     case IMPLICIT_CONV_EXPR:
> >       if (cxx_dialect < cxx11
> >           && !dependent_type_p (TREE_TYPE (t))
> >           && !INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t)))
> >         /* In C++98, a conversion to non-integral type can't be
> > part of a
> >            constant expression.  */
> >           {
> >           // reject it
> >           }
> >         // accept it
> > 
> > and thus returns true for the CAST_EXPR and TREE_LIST, and hence
> > for the
> > CONSTRUCTOR as a whole.
> > 
> > However, cxx_eval_constant_expression does not support these tree
> > codes,
> 
> Because they are template-only codes, that
> cxx_eval_constant_expression should never see.  They shouldn't
> survive
> the call to instantiate_non_dependent_expr_internal from
> fold_non_dependent_expr.
> 
> Jason

Aha - thanks.

instantiate_non_dependent_expr_internal calls tsubst_copy_and_build, and
tsubst_copy_and_build is bailing out here:

18103		/* digest_init will do the wrong thing if we let it.  */
18104		if (type && TYPE_PTRMEMFUNC_P (type))
18105		  RETURN (t);

leaving the CONSTRUCTOR uncopied, and thus containing the CAST_EXPR and
TREE_LIST, leading to the ICE within cxx_eval_constant_expression.

The above code dates back to 33643032d70f56c4e00028da8185bcac4023e646 (r61409)
from 2003, which added tsubst_copy_and_build.

The call to digest_init in tsubst_copy_and_build's CONSTRUCTOR case was
removed in 1d8baa0efe4be51729c604adf7be9c36e786edff (r117832, 2006-10-17,
fixing PR c++/27270), which amongst other things made this change,
removing the use of digest_init:

              ce->value = RECUR (ce->value);
            }

  -       r = build_constructor (NULL_TREE, n);
  -       TREE_HAS_CONSTRUCTOR (r) = TREE_HAS_CONSTRUCTOR (t);
  +       if (TREE_HAS_CONSTRUCTOR (t))
  +         return finish_compound_literal (type, n);

  -       if (type)
  -         {
  -           r = reshape_init (type, r);
  -           return digest_init (type, r);
  -         }
  -       return r;
  +       return build_constructor (NULL_TREE, n);
         }

Given that, is the early bailout for TYPE_PTRMEMFUNC_P still needed?

On the assumption that it's outdated, this patch removes it.  With that
change, tsubst_copy_and_build successfully copies the CONSTRUCTOR, with
the CAST_EXPR and TREE_LIST eliminated, fixing the ICE in fold_for_warn.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
OK for trunk?

gcc/cp/ChangeLog:
	PR c++/83974
	* pt.c (tsubst_copy_and_build) <CONSTRUCTOR>: Remove early bailout
	for pointer to member function types.

gcc/testsuite/ChangeLog:
	PR c++/83974
	* g++.dg/warn/pr83974.C: New test case.
---
 gcc/cp/pt.c                         |  4 ----
 gcc/testsuite/g++.dg/warn/pr83974.C | 11 +++++++++++
 2 files changed, 11 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/pr83974.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 0296845..0e48a13 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -18100,10 +18100,6 @@ tsubst_copy_and_build (tree t,
 	if (type == error_mark_node)
 	  RETURN (error_mark_node);
 
-	/* digest_init will do the wrong thing if we let it.  */
-	if (type && TYPE_PTRMEMFUNC_P (type))
-	  RETURN (t);
-
 	/* We do not want to process the index of aggregate
 	   initializers as they are identifier nodes which will be
 	   looked up by digest_init.  */
diff --git a/gcc/testsuite/g++.dg/warn/pr83974.C b/gcc/testsuite/g++.dg/warn/pr83974.C
new file mode 100644
index 0000000..af12c2d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr83974.C
@@ -0,0 +1,11 @@
+// { dg-options "-Wtautological-compare" }
+
+struct A {
+  typedef void (A::*B) ();
+  operator B ();
+};
+template <typename>
+struct C {
+  void foo () { d == 0; }
+  A d;
+};
-- 
1.8.5.3

Re: [PATCH v2] C++: Fix ICE in fold_for_warn on CAST_EXPR (PR c++/83974)

[Jason Merrill]

On Tue, Jan 23, 2018 at 1:06 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Tue, 2018-01-23 at 09:54 -0500, Jason Merrill wrote:
>> On Tue, Jan 23, 2018 at 9:27 AM, David Malcolm <dmalcolm@redhat.com>
>> wrote:
>> > PR c++/83974 reports an ICE within fold_for_warn when calling
>> > cxx_eval_constant_expression on a CAST_EXPR.
>> >
>> > This comes from a pointer-to-member-function.  The result of
>> > build_ptrmemfunc (within cp_convert_to_pointer for a null ptr) is
>> > a CONSTRUCTOR containing, amongst other things a CAST_EXPR of a
>> > TREE_LIST containing the zero INTEGER_CST.
>> >
>> > After r256804, fold_for_warn within a template calls
>> > fold_non_dependent_expr.
>> >
>> > For this tree, is_nondependent_constant_expression returns true.
>> >
>> > potential_constant_expression_1 has these cases:
>> >
>> >     case TREE_LIST:
>> >       {
>> >         gcc_assert (TREE_PURPOSE (t) == NULL_TREE
>> >                     || DECL_P (TREE_PURPOSE (t)));
>> >         if (!RECUR (TREE_VALUE (t), want_rval))
>> >           return false;
>> >         if (TREE_CHAIN (t) == NULL_TREE)
>> >           return true;
>> >         return RECUR (TREE_CHAIN (t), want_rval);
>> >       }
>> >
>> > and:
>> >
>> >     case CAST_EXPR:
>> >     case CONST_CAST_EXPR:
>> >     case STATIC_CAST_EXPR:
>> >     case REINTERPRET_CAST_EXPR:
>> >     case IMPLICIT_CONV_EXPR:
>> >       if (cxx_dialect < cxx11
>> >           && !dependent_type_p (TREE_TYPE (t))
>> >           && !INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t)))
>> >         /* In C++98, a conversion to non-integral type can't be
>> > part of a
>> >            constant expression.  */
>> >           {
>> >           // reject it
>> >           }
>> >         // accept it
>> >
>> > and thus returns true for the CAST_EXPR and TREE_LIST, and hence
>> > for the
>> > CONSTRUCTOR as a whole.
>> >
>> > However, cxx_eval_constant_expression does not support these tree
>> > codes,
>>
>> Because they are template-only codes, that
>> cxx_eval_constant_expression should never see.  They shouldn't
>> survive
>> the call to instantiate_non_dependent_expr_internal from
>> fold_non_dependent_expr.
>>
>> Jason
>
> Aha - thanks.
>
> instantiate_non_dependent_expr_internal calls tsubst_copy_and_build, and
> tsubst_copy_and_build is bailing out here:
>
> 18103           /* digest_init will do the wrong thing if we let it.  */
> 18104           if (type && TYPE_PTRMEMFUNC_P (type))
> 18105             RETURN (t);
>
> leaving the CONSTRUCTOR uncopied, and thus containing the CAST_EXPR and
> TREE_LIST, leading to the ICE within cxx_eval_constant_expression.

Wow, how has that not broken things before now?

The patch is OK.

Jason