[C++] Fix constexpr ICE with poor man's offsetof (PR c++/86738)

Message ID 20180803161840.GT17988@tucnak
State New
Headers show
Series
  • [C++] Fix constexpr ICE with poor man's offsetof (PR c++/86738)
Related show

Commit Message

Jakub Jelinek Aug. 3, 2018, 4:18 p.m.
Hi!

Some time ago, I've moved the poor man's offsetof recognizing hack to
cp_fold.  On the following testcase that means we don't fold it early during
parsing.  Now, if we try to evaluate those inside of constexpr contexts
with !ctx->quiet, it is diagnosed as invalid (but *non_constant_p is not
set).  Worse, with ctx->quiet, we pretend it is a constant expression but
don't actually fold it, and then we have e.g. in array index evaluation
  VERIFY_CONSTANT (index);
...
  VERIFY_CONSTANT (nelts);
  if ((lval
       ? !tree_int_cst_le (index, nelts)
       : !tree_int_cst_lt (index, nelts))
      || tree_int_cst_sgn (index) < 0)
    {
      diag_array_subscript (ctx, ary, index);
      *non_constant_p = true;
      return t;
    }
where VERIFY_CONSTANT is happy about it, even when index is not an
INTEGER_CST, but large complex TREE_CONSTANT expression.  The above though
assumes INTEGER_CST.  Perhaps we should check for INTEGER_CST somewhere (and
in other similar code too), but it isn't clear to me what exactly we should
do if those trees aren't INTEGER_CSTs, especially with !ctx->quiet.

This patch changes a different thing, the usual case (including other spots
for NULL pointer dereferences or arith) in constexpr.c is
  if (some condition)
    {
      if (!ctx->quiet)
	error* (...);
      *non_constant_p = true;
      return t;
    }
but the following two spots were different and that caused the array
handling to see those complex unsimplified constant expressions.
With this, it is not treated as constant for maybe_constant_value etc.
purposes, though following cp_fold can still fold it into a constant.

Bootstrapped/regtested on x86_64-linux and i686-linux (including
check-c++-all testing on both), ok for trunk and 8.3 after a while?

2018-08-03  Jakub Jelinek  <jakub@redhat.com>

	PR c++/86738
	* constexpr.c (cxx_eval_binary_expression): For arithmetics involving
	NULL pointer set *non_constant_p to true.
	(cxx_eval_component_reference): For dereferencing of a NULL pointer,
	set *non_constant_p to true and return t.

	* g++.dg/opt/pr86738.C: New test.


	Jakub

Comments

Jason Merrill Aug. 5, 2018, 2:30 p.m. | #1
OK

On Sat, Aug 4, 2018 at 2:18 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!

>

> Some time ago, I've moved the poor man's offsetof recognizing hack to

> cp_fold.  On the following testcase that means we don't fold it early during

> parsing.  Now, if we try to evaluate those inside of constexpr contexts

> with !ctx->quiet, it is diagnosed as invalid (but *non_constant_p is not

> set).  Worse, with ctx->quiet, we pretend it is a constant expression but

> don't actually fold it, and then we have e.g. in array index evaluation

>   VERIFY_CONSTANT (index);

> ...

>   VERIFY_CONSTANT (nelts);

>   if ((lval

>        ? !tree_int_cst_le (index, nelts)

>        : !tree_int_cst_lt (index, nelts))

>       || tree_int_cst_sgn (index) < 0)

>     {

>       diag_array_subscript (ctx, ary, index);

>       *non_constant_p = true;

>       return t;

>     }

> where VERIFY_CONSTANT is happy about it, even when index is not an

> INTEGER_CST, but large complex TREE_CONSTANT expression.  The above though

> assumes INTEGER_CST.  Perhaps we should check for INTEGER_CST somewhere (and

> in other similar code too), but it isn't clear to me what exactly we should

> do if those trees aren't INTEGER_CSTs, especially with !ctx->quiet.

>

> This patch changes a different thing, the usual case (including other spots

> for NULL pointer dereferences or arith) in constexpr.c is

>   if (some condition)

>     {

>       if (!ctx->quiet)

>         error* (...);

>       *non_constant_p = true;

>       return t;

>     }

> but the following two spots were different and that caused the array

> handling to see those complex unsimplified constant expressions.

> With this, it is not treated as constant for maybe_constant_value etc.

> purposes, though following cp_fold can still fold it into a constant.

>

> Bootstrapped/regtested on x86_64-linux and i686-linux (including

> check-c++-all testing on both), ok for trunk and 8.3 after a while?

>

> 2018-08-03  Jakub Jelinek  <jakub@redhat.com>

>

>         PR c++/86738

>         * constexpr.c (cxx_eval_binary_expression): For arithmetics involving

>         NULL pointer set *non_constant_p to true.

>         (cxx_eval_component_reference): For dereferencing of a NULL pointer,

>         set *non_constant_p to true and return t.

>

>         * g++.dg/opt/pr86738.C: New test.

>

> --- gcc/cp/constexpr.c.jj       2018-07-31 23:57:24.193432388 +0200

> +++ gcc/cp/constexpr.c  2018-08-03 14:54:13.302817282 +0200

> @@ -2082,6 +2082,7 @@ cxx_eval_binary_expression (const conste

>      {

>        if (!ctx->quiet)

>         error ("arithmetic involving a null pointer in %qE", lhs);

> +      *non_constant_p = true;

>        return t;

>      }

>    else if (code == POINTER_PLUS_EXPR)

> @@ -2522,9 +2523,13 @@ cxx_eval_component_reference (const cons

>                                              lval,

>                                              non_constant_p, overflow_p);

>    if (INDIRECT_REF_P (whole)

> -      && integer_zerop (TREE_OPERAND (whole, 0))

> -      && !ctx->quiet)

> -    error ("dereferencing a null pointer in %qE", orig_whole);

> +      && integer_zerop (TREE_OPERAND (whole, 0)))

> +    {

> +      if (!ctx->quiet)

> +       error ("dereferencing a null pointer in %qE", orig_whole);

> +      *non_constant_p = true;

> +      return t;

> +    }

>

>    if (TREE_CODE (whole) == PTRMEM_CST)

>      whole = cplus_expand_constant (whole);

> --- gcc/testsuite/g++.dg/opt/pr86738.C.jj       2018-08-03 15:03:51.477358712 +0200

> +++ gcc/testsuite/g++.dg/opt/pr86738.C  2018-08-03 15:02:51.940201694 +0200

> @@ -0,0 +1,12 @@

> +// PR c++/86738

> +// { dg-do compile }

> +

> +struct S { int s; };

> +unsigned char a[20];

> +unsigned char *p = &a[(__UINTPTR_TYPE__) &((S *) 0)->s];

> +

> +void

> +foo ()

> +{

> +  __builtin_memcpy (&a[15], &a[(__UINTPTR_TYPE__) &((S *) 0)->s], 2);

> +}

>

>         Jakub

Patch

--- gcc/cp/constexpr.c.jj	2018-07-31 23:57:24.193432388 +0200
+++ gcc/cp/constexpr.c	2018-08-03 14:54:13.302817282 +0200
@@ -2082,6 +2082,7 @@  cxx_eval_binary_expression (const conste
     {
       if (!ctx->quiet)
 	error ("arithmetic involving a null pointer in %qE", lhs);
+      *non_constant_p = true;
       return t;
     }
   else if (code == POINTER_PLUS_EXPR)
@@ -2522,9 +2523,13 @@  cxx_eval_component_reference (const cons
 					     lval,
 					     non_constant_p, overflow_p);
   if (INDIRECT_REF_P (whole)
-      && integer_zerop (TREE_OPERAND (whole, 0))
-      && !ctx->quiet)
-    error ("dereferencing a null pointer in %qE", orig_whole);
+      && integer_zerop (TREE_OPERAND (whole, 0)))
+    {
+      if (!ctx->quiet)
+	error ("dereferencing a null pointer in %qE", orig_whole);
+      *non_constant_p = true;
+      return t;
+    }
 
   if (TREE_CODE (whole) == PTRMEM_CST)
     whole = cplus_expand_constant (whole);
--- gcc/testsuite/g++.dg/opt/pr86738.C.jj	2018-08-03 15:03:51.477358712 +0200
+++ gcc/testsuite/g++.dg/opt/pr86738.C	2018-08-03 15:02:51.940201694 +0200
@@ -0,0 +1,12 @@ 
+// PR c++/86738
+// { dg-do compile }
+
+struct S { int s; };
+unsigned char a[20];
+unsigned char *p = &a[(__UINTPTR_TYPE__) &((S *) 0)->s];
+
+void
+foo ()
+{
+  __builtin_memcpy (&a[15], &a[(__UINTPTR_TYPE__) &((S *) 0)->s], 2);
+}