[C++] Don't clear TREE_CONSTANT on ADDR_EXPRs (PR c++/83993)

Message ID 20180124232224.GK2063@tucnak
State New
Headers show
Series
  • [C++] Don't clear TREE_CONSTANT on ADDR_EXPRs (PR c++/83993)
Related show

Commit Message

Jakub Jelinek Jan. 24, 2018, 11:22 p.m.
Hi!

cxx_eval_outermost_constant_expr clears TREE_CONSTANT on ADDR_EXPRs that
aren't considered by C++ constant expressions, but that breaks middle-end
which relies on TREE_CONSTANT being set on ADDR_EXPR where the address
is constant.

The following patch just special cases ADDR_EXPR not to clear it.

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

As I wrote in the PR, another option would be to restore TREE_CONSTANT
during genericization if it was cleared earlier in the FE.

2018-01-24  Jakub Jelinek  <jakub@redhat.com>

	PR c++/83993
	* constexpr.c (cxx_eval_outermost_constant_expr): Don't clear
	TREE_CONSTANT on ADDR_EXPRs.

	* g++.dg/init/pr83993-2.C: New test.


	Jakub

Comments

Jason Merrill Jan. 31, 2018, 4:05 p.m. | #1
On Wed, Jan 24, 2018 at 6:22 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!

>

> cxx_eval_outermost_constant_expr clears TREE_CONSTANT on ADDR_EXPRs that

> aren't considered by C++ constant expressions, but that breaks middle-end

> which relies on TREE_CONSTANT being set on ADDR_EXPR where the address

> is constant.

>

> The following patch just special cases ADDR_EXPR not to clear it.

>

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

>

> As I wrote in the PR, another option would be to restore TREE_CONSTANT

> during genericization if it was cleared earlier in the FE.

>

> 2018-01-24  Jakub Jelinek  <jakub@redhat.com>

>

>         PR c++/83993

>         * constexpr.c (cxx_eval_outermost_constant_expr): Don't clear

>         TREE_CONSTANT on ADDR_EXPRs.

>

>         * g++.dg/init/pr83993-2.C: New test.

>

> --- gcc/cp/constexpr.c.jj       2018-01-24 13:38:40.572913190 +0100

> +++ gcc/cp/constexpr.c  2018-01-24 17:03:16.821440047 +0100

> @@ -4832,7 +4832,7 @@ cxx_eval_outermost_constant_expr (tree t

>

>    if (non_constant_p && !allow_non_constant)

>      return error_mark_node;

> -  else if (non_constant_p && TREE_CONSTANT (r))

> +  else if (non_constant_p && TREE_CONSTANT (r) && TREE_CODE (r) != ADDR_EXPR)

>      {

>        /* This isn't actually constant, so unset TREE_CONSTANT.  */

>        if (EXPR_P (r))


Hmm, what if we check for ADDR_EXPR in the EXPR_P test, so we build a
non-TREE_CONSTANT NOP_EXPR around the ADDR_EXPR instead?

Jason
Jakub Jelinek Jan. 31, 2018, 4:21 p.m. | #2
On Wed, Jan 31, 2018 at 11:05:17AM -0500, Jason Merrill wrote:
> > 2018-01-24  Jakub Jelinek  <jakub@redhat.com>

> >

> >         PR c++/83993

> >         * constexpr.c (cxx_eval_outermost_constant_expr): Don't clear

> >         TREE_CONSTANT on ADDR_EXPRs.

> >

> >         * g++.dg/init/pr83993-2.C: New test.

> >

> > --- gcc/cp/constexpr.c.jj       2018-01-24 13:38:40.572913190 +0100

> > +++ gcc/cp/constexpr.c  2018-01-24 17:03:16.821440047 +0100

> > @@ -4832,7 +4832,7 @@ cxx_eval_outermost_constant_expr (tree t

> >

> >    if (non_constant_p && !allow_non_constant)

> >      return error_mark_node;

> > -  else if (non_constant_p && TREE_CONSTANT (r))

> > +  else if (non_constant_p && TREE_CONSTANT (r) && TREE_CODE (r) != ADDR_EXPR)

> >      {

> >        /* This isn't actually constant, so unset TREE_CONSTANT.  */

> >        if (EXPR_P (r))

> 

> Hmm, what if we check for ADDR_EXPR in the EXPR_P test, so we build a

> non-TREE_CONSTANT NOP_EXPR around the ADDR_EXPR instead?


That looks like a good idea and works on the testcase in question,
ok for trunk if it passes bootstrap/regtest?

2018-01-31  Jason Merrill  <jason@redhat.com>
	    Jakub Jelinek  <jakub@redhat.com>

	PR c++/83993
	* constexpr.c (cxx_eval_outermost_constant_expr): Build NOP_EXPR
	around non-constant ADDR_EXPRs rather than clearing TREE_CONSTANT
	on ADDR_EXPR.

	* g++.dg/init/pr83993-2.C: New test.

--- gcc/cp/constexpr.c.jj	2018-01-30 12:30:24.279362188 +0100
+++ gcc/cp/constexpr.c	2018-01-31 17:18:03.316302653 +0100
@@ -4824,7 +4824,7 @@ cxx_eval_outermost_constant_expr (tree t
   else if (non_constant_p && TREE_CONSTANT (r))
     {
       /* This isn't actually constant, so unset TREE_CONSTANT.  */
-      if (EXPR_P (r))
+      if (EXPR_P (r) && TREE_CODE (r) != ADDR_EXPR)
 	r = copy_node (r);
       else if (TREE_CODE (r) == CONSTRUCTOR)
 	r = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (r), r);
--- gcc/testsuite/g++.dg/init/pr83993-2.C.jj	2018-01-31 17:17:04.755290928 +0100
+++ gcc/testsuite/g++.dg/init/pr83993-2.C	2018-01-31 17:17:04.755290928 +0100
@@ -0,0 +1,14 @@
+// PR c++/83993
+// { dg-do compile }
+// { dg-options "-w" }
+
+int a[5];
+extern int b[];
+int *const c = &a[6];
+int *const d = &b[1];
+
+int
+foo ()
+{
+  return c[-4] + d[-1];
+}


	Jakub
Jason Merrill Jan. 31, 2018, 5:01 p.m. | #3
On Wed, Jan 31, 2018 at 11:21 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Jan 31, 2018 at 11:05:17AM -0500, Jason Merrill wrote:

>> > 2018-01-24  Jakub Jelinek  <jakub@redhat.com>

>> >

>> >         PR c++/83993

>> >         * constexpr.c (cxx_eval_outermost_constant_expr): Don't clear

>> >         TREE_CONSTANT on ADDR_EXPRs.

>> >

>> >         * g++.dg/init/pr83993-2.C: New test.

>> >

>> > --- gcc/cp/constexpr.c.jj       2018-01-24 13:38:40.572913190 +0100

>> > +++ gcc/cp/constexpr.c  2018-01-24 17:03:16.821440047 +0100

>> > @@ -4832,7 +4832,7 @@ cxx_eval_outermost_constant_expr (tree t

>> >

>> >    if (non_constant_p && !allow_non_constant)

>> >      return error_mark_node;

>> > -  else if (non_constant_p && TREE_CONSTANT (r))

>> > +  else if (non_constant_p && TREE_CONSTANT (r) && TREE_CODE (r) != ADDR_EXPR)

>> >      {

>> >        /* This isn't actually constant, so unset TREE_CONSTANT.  */

>> >        if (EXPR_P (r))

>>

>> Hmm, what if we check for ADDR_EXPR in the EXPR_P test, so we build a

>> non-TREE_CONSTANT NOP_EXPR around the ADDR_EXPR instead?

>

> That looks like a good idea and works on the testcase in question,

> ok for trunk if it passes bootstrap/regtest?

>

> 2018-01-31  Jason Merrill  <jason@redhat.com>

>             Jakub Jelinek  <jakub@redhat.com>

>

>         PR c++/83993

>         * constexpr.c (cxx_eval_outermost_constant_expr): Build NOP_EXPR

>         around non-constant ADDR_EXPRs rather than clearing TREE_CONSTANT

>         on ADDR_EXPR.

>

>         * g++.dg/init/pr83993-2.C: New test.

>

> --- gcc/cp/constexpr.c.jj       2018-01-30 12:30:24.279362188 +0100

> +++ gcc/cp/constexpr.c  2018-01-31 17:18:03.316302653 +0100

> @@ -4824,7 +4824,7 @@ cxx_eval_outermost_constant_expr (tree t

>    else if (non_constant_p && TREE_CONSTANT (r))

>      {

>        /* This isn't actually constant, so unset TREE_CONSTANT.  */

> -      if (EXPR_P (r))

> +      if (EXPR_P (r) && TREE_CODE (r) != ADDR_EXPR)


OK with an added comment about the middle end needing TREE_CONSTANT to
stay set on ADDR_EXPR.

Jason

Patch

--- gcc/cp/constexpr.c.jj	2018-01-24 13:38:40.572913190 +0100
+++ gcc/cp/constexpr.c	2018-01-24 17:03:16.821440047 +0100
@@ -4832,7 +4832,7 @@  cxx_eval_outermost_constant_expr (tree t
 
   if (non_constant_p && !allow_non_constant)
     return error_mark_node;
-  else if (non_constant_p && TREE_CONSTANT (r))
+  else if (non_constant_p && TREE_CONSTANT (r) && TREE_CODE (r) != ADDR_EXPR)
     {
       /* This isn't actually constant, so unset TREE_CONSTANT.  */
       if (EXPR_P (r))
--- gcc/testsuite/g++.dg/init/pr83993-2.C.jj	2018-01-24 17:04:18.823456178 +0100
+++ gcc/testsuite/g++.dg/init/pr83993-2.C	2018-01-24 17:04:39.593454636 +0100
@@ -0,0 +1,14 @@ 
+// PR c++/83993
+// { dg-do compile }
+// { dg-options "-w" }
+
+int a[5];
+extern int b[];
+int *const c = &a[6];
+int *const d = &b[1];
+
+int
+foo ()
+{
+  return c[-4] + d[-1];
+}