[C++] Fix constexpr handling of &x->y (PR c++/84463, take 2)

Message ID 20180417162838.GE8577@tucnak
State New
Headers show
Series
  • [C++] Fix constexpr handling of &x->y (PR c++/84463, take 2)
Related show

Commit Message

Jakub Jelinek April 17, 2018, 4:28 p.m.
On Mon, Apr 16, 2018 at 10:55:34PM +0000, Jason Merrill wrote:
> On Mon, Apr 16, 2018, 1:31 PM Jakub Jelinek <jakub@redhat.com> wrote:

> 

> > On Mon, Apr 16, 2018 at 09:28:43PM +0200, Jakub Jelinek wrote:

> > > On the following new testcase we emit 2 different constexpr errors

> > > because of premature folding, where the PR44100 hack which is supposed

> > > to fold expressions like &((S *)0)->f or

> > > &((S *)24)->f folds all the &x->y expressions if x is TREE_CONSTANT

> > > into (some type)(x + cst) where what we were actually trying to access

> > > is lost.

> > >

> > > The following patch limits the offsetof-like expression hack to

> > expressions

> > > where maybe_constant_value of val's operand is INTEGER_CST, or e.g.

> > > a cast of INTEGER_CST to some pointer type.  This way we don't regress

> > > e.g. init/struct2.C, but don't mess up with x is e.g. some constexpr

> > > variable initialized to address of something.  Or should it avoid

> > > maybe_constant_value and just handle the literal INTEGER_CST and cast

> > > thereof?  We wouldn't handle &((S *)(24 + 8))->f that way though...

> >

> > Or shall we move this folding to cp_fold instead of cp_build_addr_expr_1

> > (while keeping it limited to INTEGER_CST pointers)?

> 

> 

> Yes, I think that would be better.


Ok, here is a patch that does that.  Compared to previous patch, the -O1 for
constexpr-nullptr-1.C is still needed, there is different diagnostics
(dereference of null) in constexpr-nullptr-2.C on two lines and two lines
that were commented due to this hack can now be handled.  Similarly,
array-size2.C XPASSes now, so removed the xfail in there.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-04-17  Jakub Jelinek  <jakub@redhat.com>

	PR c++/84463
	* typeck.c (cp_build_addr_expr_1): Move handling of offsetof-like
	tricks from here to ...
	* cp-gimplify.c (cp_fold) <case ADDR_EXPR>: ... here.  Only use it
	if INDIRECT_REF's operand is INTEGER_CST cast to pointer type.

	* g++.dg/cpp0x/constexpr-nullptr-1.C: Add -O1 to dg-options.
	* g++.dg/cpp0x/constexpr-nullptr-2.C: Expect different diagnostics
	in two cases.  Uncomment two other tests and add expected dg-error for
	them.
	* g++.dg/init/struct2.C: Cast to int rather than long to avoid
	-Wnarrowing diagnostics on some targets for c++11.
	* g++.dg/parse/array-size2.C: Remove xfail.
	* g++.dg/cpp0x/constexpr-84463.C: New test.



	Jakub

Comments

Jason Merrill April 18, 2018, 1:24 a.m. | #1
Ok.

On Tue, Apr 17, 2018, 10:28 AM Jakub Jelinek <jakub@redhat.com> wrote:

> On Mon, Apr 16, 2018 at 10:55:34PM +0000, Jason Merrill wrote:

> > On Mon, Apr 16, 2018, 1:31 PM Jakub Jelinek <jakub@redhat.com> wrote:

> >

> > > On Mon, Apr 16, 2018 at 09:28:43PM +0200, Jakub Jelinek wrote:

> > > > On the following new testcase we emit 2 different constexpr errors

> > > > because of premature folding, where the PR44100 hack which is

> supposed

> > > > to fold expressions like &((S *)0)->f or

> > > > &((S *)24)->f folds all the &x->y expressions if x is TREE_CONSTANT

> > > > into (some type)(x + cst) where what we were actually trying to

> access

> > > > is lost.

> > > >

> > > > The following patch limits the offsetof-like expression hack to

> > > expressions

> > > > where maybe_constant_value of val's operand is INTEGER_CST, or e.g.

> > > > a cast of INTEGER_CST to some pointer type.  This way we don't

> regress

> > > > e.g. init/struct2.C, but don't mess up with x is e.g. some constexpr

> > > > variable initialized to address of something.  Or should it avoid

> > > > maybe_constant_value and just handle the literal INTEGER_CST and cast

> > > > thereof?  We wouldn't handle &((S *)(24 + 8))->f that way though...

> > >

> > > Or shall we move this folding to cp_fold instead of

> cp_build_addr_expr_1

> > > (while keeping it limited to INTEGER_CST pointers)?

> >

> >

> > Yes, I think that would be better.

>

> Ok, here is a patch that does that.  Compared to previous patch, the -O1

> for

> constexpr-nullptr-1.C is still needed, there is different diagnostics

> (dereference of null) in constexpr-nullptr-2.C on two lines and two lines

> that were commented due to this hack can now be handled.  Similarly,

> array-size2.C XPASSes now, so removed the xfail in there.

>

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

>

> 2018-04-17  Jakub Jelinek  <jakub@redhat.com>

>

>         PR c++/84463

>         * typeck.c (cp_build_addr_expr_1): Move handling of offsetof-like

>         tricks from here to ...

>         * cp-gimplify.c (cp_fold) <case ADDR_EXPR>: ... here.  Only use it

>         if INDIRECT_REF's operand is INTEGER_CST cast to pointer type.

>

>         * g++.dg/cpp0x/constexpr-nullptr-1.C: Add -O1 to dg-options.

>         * g++.dg/cpp0x/constexpr-nullptr-2.C: Expect different diagnostics

>         in two cases.  Uncomment two other tests and add expected dg-error

> for

>         them.

>         * g++.dg/init/struct2.C: Cast to int rather than long to avoid

>         -Wnarrowing diagnostics on some targets for c++11.

>         * g++.dg/parse/array-size2.C: Remove xfail.

>         * g++.dg/cpp0x/constexpr-84463.C: New test.

>

> --- gcc/cp/typeck.c.jj  2018-04-17 11:09:13.887127384 +0200

> +++ gcc/cp/typeck.c     2018-04-17 18:00:17.616039746 +0200

> @@ -5893,19 +5893,6 @@ cp_build_addr_expr_1 (tree arg, bool str

>        return arg;

>      }

>

> -  /* ??? Cope with user tricks that amount to offsetof.  */

> -  if (TREE_CODE (argtype) != FUNCTION_TYPE

> -      && TREE_CODE (argtype) != METHOD_TYPE

> -      && argtype != unknown_type_node

> -      && (val = get_base_address (arg))

> -      && COMPLETE_TYPE_P (TREE_TYPE (val))

> -      && INDIRECT_REF_P (val)

> -      && TREE_CONSTANT (TREE_OPERAND (val, 0)))

> -    {

> -      tree type = build_pointer_type (argtype);

> -      return fold_convert (type, fold_offsetof_1 (arg));

> -    }

> -

>    /* Handle complex lvalues (when permitted)

>       by reduction to simpler cases.  */

>    val = unary_complex_lvalue (ADDR_EXPR, arg);

> --- gcc/cp/cp-gimplify.c.jj     2018-04-17 11:09:13.886127383 +0200

> +++ gcc/cp/cp-gimplify.c        2018-04-17 18:00:17.626039748 +0200

> @@ -2215,6 +2215,28 @@ cp_fold (tree x)

>        goto unary;

>

>      case ADDR_EXPR:

> +      loc = EXPR_LOCATION (x);

> +      op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), false);

> +

> +      /* Cope with user tricks that amount to offsetof.  */

> +      if (op0 != error_mark_node

> +         && TREE_CODE (TREE_TYPE (op0)) != FUNCTION_TYPE

> +         && TREE_CODE (TREE_TYPE (op0)) != METHOD_TYPE)

> +       {

> +         tree val = get_base_address (op0);

> +         if (val

> +             && INDIRECT_REF_P (val)

> +             && COMPLETE_TYPE_P (TREE_TYPE (val))

> +             && TREE_CONSTANT (TREE_OPERAND (val, 0)))

> +           {

> +             val = TREE_OPERAND (val, 0);

> +             STRIP_NOPS (val);

> +             if (TREE_CODE (val) == INTEGER_CST)

> +               return fold_convert (TREE_TYPE (x), fold_offsetof_1 (op0));

> +           }

> +       }

> +      goto finish_unary;

> +

>      case REALPART_EXPR:

>      case IMAGPART_EXPR:

>        rval_ops = false;

> @@ -2232,6 +2254,7 @@ cp_fold (tree x)

>        loc = EXPR_LOCATION (x);

>        op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops);

>

> +    finish_unary:

>        if (op0 != TREE_OPERAND (x, 0))

>         {

>           if (op0 == error_mark_node)

> --- gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C.jj 2018-04-17

> 11:09:13.697127292 +0200

> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C    2018-04-17

> 18:00:17.705039771 +0200

> @@ -6,7 +6,7 @@

>  // c++/67376 on gcc-patches for additional background.

>

>  // { dg-do compile { target c++11 } }

> -// { dg-options "-fdelete-null-pointer-checks -fdump-tree-optimized" }

> +// { dg-options "-O1 -fdelete-null-pointer-checks -fdump-tree-optimized" }

>

>  // Runtime assert.  Used for potentially invalid expressions.

>  #define RA(e)  ((e) ? (void)0 : __builtin_abort ())

> --- gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C.jj 2016-08-06

> 12:11:30.933757938 +0200

> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C    2018-04-17

> 18:12:41.445247860 +0200

> @@ -192,12 +192,11 @@ constexpr bool b11 = ps >= (S*)0;

>  constexpr S* ps1 = ps;

>  constexpr S* ps2 = ps1;

>

> -// The following aren't diagnosed due to a bug.

> -// constexpr int* pi0 = &((S*)0)->i;

> -// constexpr int* pi1 = &((S*)nullptr)->i;

> +constexpr int* pi0 = &((S*)0)->i;      // { dg-error "null pointer|not a

> constant" }

> +constexpr int* pi1 = &((S*)nullptr)->i;        // { dg-error "null

> pointer|not a constant" }

>

> -constexpr int* pj0 = &((S*)0)->j;      // { dg-error "not a constant

> expression" }

> -constexpr int* pj1 = &((S*)nullptr)->j;  // { dg-error "not a constant

> expression" }

> +constexpr int* pj0 = &((S*)0)->j;      // { dg-error "null pointer|not a

> constant" }

> +constexpr int* pj1 = &((S*)nullptr)->j;        // { dg-error "null

> pointer|not a constant" }

>

>  constexpr int* psi = &ps->i;       // { dg-error "null pointer|not a

> constant" }

>  constexpr int* psj = &ps->j;       // { dg-error "null pointer|not a

> constant" }

> --- gcc/testsuite/g++.dg/init/struct2.C.jj      2018-04-17

> 11:09:13.746127314 +0200

> +++ gcc/testsuite/g++.dg/init/struct2.C 2018-04-17 18:00:17.750039783 +0200

> @@ -15,7 +15,7 @@ void saveOrLoad() {

>      };

>

>      SaveLoadEntry trackEntries = {

> -       ((long) (__SIZE_TYPE__) (&((Track *) 42)->soundName[0])) - 42,

> +       ((int) (__SIZE_TYPE__) (&((Track *) 42)->soundName[0])) - 42,

>          0, 1

>      };

>      saveLoadEntries(&trackEntries);

> --- gcc/testsuite/g++.dg/parse/array-size2.C.jj 2018-04-17

> 11:09:13.693127290 +0200

> +++ gcc/testsuite/g++.dg/parse/array-size2.C    2018-04-17

> 18:00:17.798039797 +0200

> @@ -15,6 +15,6 @@ void

>  foo (void)

>  {

>    char g[(char *) &((struct S *) 0)->b - (char *) 0]; // { dg-error

> "constant" }

> -  char h[(__SIZE_TYPE__) &((struct S *) 8)->b];              // {

> dg-error "constant" "" { xfail *-*-* } }

> +  char h[(__SIZE_TYPE__) &((struct S *) 8)->b];              // {

> dg-error "constant" }

>    bar (g, h);

>  }

> --- gcc/testsuite/g++.dg/cpp0x/constexpr-84463.C.jj     2018-04-17

> 18:00:17.799039797 +0200

> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-84463.C        2018-04-17

> 18:00:17.799039797 +0200

> @@ -0,0 +1,22 @@

> +// PR c++/84463

> +// { dg-do compile { target c++11 } }

> +

> +struct S { int r; const unsigned char s[5]; };

> +static constexpr S a[] = { { 0, "abcd" } };

> +struct T { const unsigned char s[5]; };

> +static constexpr T b[] = { { "abcd" } };

> +

> +constexpr int

> +foo (const unsigned char *x)

> +{

> +  return x[0];

> +}

> +

> +constexpr static const S *j = &a[0];

> +constexpr static const int k = j->s[0];

> +constexpr static int l = foo (a[0].s);

> +constexpr static int m = foo (j->s);

> +constexpr static const T *n = &b[0];

> +constexpr static const int o = n->s[0];

> +constexpr static int p = foo (b[0].s);

> +constexpr static int q = foo (n->s);

>

>

>         Jakub

>

Patch

--- gcc/cp/typeck.c.jj	2018-04-17 11:09:13.887127384 +0200
+++ gcc/cp/typeck.c	2018-04-17 18:00:17.616039746 +0200
@@ -5893,19 +5893,6 @@  cp_build_addr_expr_1 (tree arg, bool str
       return arg;
     }
 
-  /* ??? Cope with user tricks that amount to offsetof.  */
-  if (TREE_CODE (argtype) != FUNCTION_TYPE
-      && TREE_CODE (argtype) != METHOD_TYPE
-      && argtype != unknown_type_node
-      && (val = get_base_address (arg))
-      && COMPLETE_TYPE_P (TREE_TYPE (val))
-      && INDIRECT_REF_P (val)
-      && TREE_CONSTANT (TREE_OPERAND (val, 0)))
-    {
-      tree type = build_pointer_type (argtype);
-      return fold_convert (type, fold_offsetof_1 (arg));
-    }
-
   /* Handle complex lvalues (when permitted)
      by reduction to simpler cases.  */
   val = unary_complex_lvalue (ADDR_EXPR, arg);
--- gcc/cp/cp-gimplify.c.jj	2018-04-17 11:09:13.886127383 +0200
+++ gcc/cp/cp-gimplify.c	2018-04-17 18:00:17.626039748 +0200
@@ -2215,6 +2215,28 @@  cp_fold (tree x)
       goto unary;
 
     case ADDR_EXPR:
+      loc = EXPR_LOCATION (x);
+      op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), false);
+
+      /* Cope with user tricks that amount to offsetof.  */
+      if (op0 != error_mark_node
+	  && TREE_CODE (TREE_TYPE (op0)) != FUNCTION_TYPE
+	  && TREE_CODE (TREE_TYPE (op0)) != METHOD_TYPE)
+	{
+	  tree val = get_base_address (op0);
+	  if (val
+	      && INDIRECT_REF_P (val)
+	      && COMPLETE_TYPE_P (TREE_TYPE (val))
+	      && TREE_CONSTANT (TREE_OPERAND (val, 0)))
+	    {
+	      val = TREE_OPERAND (val, 0);
+	      STRIP_NOPS (val);
+	      if (TREE_CODE (val) == INTEGER_CST)
+		return fold_convert (TREE_TYPE (x), fold_offsetof_1 (op0));
+	    }
+	}
+      goto finish_unary;
+
     case REALPART_EXPR:
     case IMAGPART_EXPR:
       rval_ops = false;
@@ -2232,6 +2254,7 @@  cp_fold (tree x)
       loc = EXPR_LOCATION (x);
       op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops);
 
+    finish_unary:
       if (op0 != TREE_OPERAND (x, 0))
 	{
 	  if (op0 == error_mark_node)
--- gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C.jj	2018-04-17 11:09:13.697127292 +0200
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C	2018-04-17 18:00:17.705039771 +0200
@@ -6,7 +6,7 @@ 
 // c++/67376 on gcc-patches for additional background.
 
 // { dg-do compile { target c++11 } }
-// { dg-options "-fdelete-null-pointer-checks -fdump-tree-optimized" }
+// { dg-options "-O1 -fdelete-null-pointer-checks -fdump-tree-optimized" }
 
 // Runtime assert.  Used for potentially invalid expressions.
 #define RA(e)  ((e) ? (void)0 : __builtin_abort ())
--- gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C.jj	2016-08-06 12:11:30.933757938 +0200
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C	2018-04-17 18:12:41.445247860 +0200
@@ -192,12 +192,11 @@  constexpr bool b11 = ps >= (S*)0;
 constexpr S* ps1 = ps;
 constexpr S* ps2 = ps1;
 
-// The following aren't diagnosed due to a bug.
-// constexpr int* pi0 = &((S*)0)->i;
-// constexpr int* pi1 = &((S*)nullptr)->i;
+constexpr int* pi0 = &((S*)0)->i;	// { dg-error "null pointer|not a constant" }
+constexpr int* pi1 = &((S*)nullptr)->i;	// { dg-error "null pointer|not a constant" }
 
-constexpr int* pj0 = &((S*)0)->j;	// { dg-error "not a constant expression" }
-constexpr int* pj1 = &((S*)nullptr)->j;  // { dg-error "not a constant expression" }
+constexpr int* pj0 = &((S*)0)->j;	// { dg-error "null pointer|not a constant" }
+constexpr int* pj1 = &((S*)nullptr)->j;	// { dg-error "null pointer|not a constant" }
 
 constexpr int* psi = &ps->i;	    // { dg-error "null pointer|not a constant" }
 constexpr int* psj = &ps->j;	    // { dg-error "null pointer|not a constant" }
--- gcc/testsuite/g++.dg/init/struct2.C.jj	2018-04-17 11:09:13.746127314 +0200
+++ gcc/testsuite/g++.dg/init/struct2.C	2018-04-17 18:00:17.750039783 +0200
@@ -15,7 +15,7 @@  void saveOrLoad() {
     };    
 
     SaveLoadEntry trackEntries = {
-	((long) (__SIZE_TYPE__) (&((Track *) 42)->soundName[0])) - 42,
+	((int) (__SIZE_TYPE__) (&((Track *) 42)->soundName[0])) - 42,
         0, 1
     };
     saveLoadEntries(&trackEntries);
--- gcc/testsuite/g++.dg/parse/array-size2.C.jj	2018-04-17 11:09:13.693127290 +0200
+++ gcc/testsuite/g++.dg/parse/array-size2.C	2018-04-17 18:00:17.798039797 +0200
@@ -15,6 +15,6 @@  void
 foo (void)
 {
   char g[(char *) &((struct S *) 0)->b - (char *) 0]; // { dg-error "constant" }
-  char h[(__SIZE_TYPE__) &((struct S *) 8)->b];	      // { dg-error "constant" "" { xfail *-*-* } }
+  char h[(__SIZE_TYPE__) &((struct S *) 8)->b];	      // { dg-error "constant" }
   bar (g, h);
 }
--- gcc/testsuite/g++.dg/cpp0x/constexpr-84463.C.jj	2018-04-17 18:00:17.799039797 +0200
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-84463.C	2018-04-17 18:00:17.799039797 +0200
@@ -0,0 +1,22 @@ 
+// PR c++/84463
+// { dg-do compile { target c++11 } }
+
+struct S { int r; const unsigned char s[5]; };
+static constexpr S a[] = { { 0, "abcd" } };
+struct T { const unsigned char s[5]; };
+static constexpr T b[] = { { "abcd" } };
+
+constexpr int
+foo (const unsigned char *x)
+{
+  return x[0];
+}
+
+constexpr static const S *j = &a[0];
+constexpr static const int k = j->s[0];
+constexpr static int l = foo (a[0].s);
+constexpr static int m = foo (j->s);
+constexpr static const T *n = &b[0];
+constexpr static const int o = n->s[0];
+constexpr static int p = foo (b[0].s);
+constexpr static int q = foo (n->s);