diff mbox series

[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) | expand

Commit Message

Jakub Jelinek Jan. 24, 2018, 11:22 p.m. UTC 
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. UTC | #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. UTC | #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. UTC | #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
diff mbox series

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];
+}