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

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

Commit Message

Jakub Jelinek April 16, 2018, 7:28 p.m.
Hi!

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

The patches causes a small regression on constexpr-nullptr-1.C, it tries
to compare (outside of constexpr contexts because it is undefined there)
null with non-equality comparison with &x->y and was expecting it to be
folded even when not optimizing, which is what doesn't happen anymore.
We optimize it in the GIMPLE passes fine, so I've just added -O1.

Earlier version of this patch has been bootstrapped/regtested on
x86_64-linux and i686-linux, is this one ok for trunk if it passes
bootstrap/regtest?

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

	PR c++/84463
	* typeck.c (cp_build_addr_expr_1): Don't use fold_offsetof_1 hack
	if val's operand does not fold into INTEGER_CST possibly wrapped
	with conversions.

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


	Jakub

Comments

Jason Merrill April 16, 2018, 10:55 p.m. | #1
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.


>

Patch

--- gcc/cp/typeck.c.jj	2018-04-16 18:11:54.784378158 +0200
+++ gcc/cp/typeck.c	2018-04-16 21:03:15.674152875 +0200
@@ -5902,8 +5902,13 @@  cp_build_addr_expr_1 (tree arg, bool str
       && INDIRECT_REF_P (val)
       && TREE_CONSTANT (TREE_OPERAND (val, 0)))
     {
-      tree type = build_pointer_type (argtype);
-      return fold_convert (type, fold_offsetof_1 (arg));
+      tree t = maybe_constant_value (TREE_OPERAND (val, 0));
+      STRIP_NOPS (t);
+      if (TREE_CODE (t) == INTEGER_CST)
+	{
+	  tree type = build_pointer_type (argtype);
+	  return fold_convert (type, fold_offsetof_1 (arg));
+	}
     }
 
   /* Handle complex lvalues (when permitted)
--- gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C.jj	2016-09-30 09:42:13.778446684 +0200
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C	2018-04-16 21:13:06.057428435 +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-84463.C.jj	2018-04-16 21:02:16.282128997 +0200
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-84463.C	2018-04-16 21:02:16.282128997 +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);