public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074]
@ 2022-01-06  9:24 Jakub Jelinek
  2022-01-10 14:10 ` Richard Biener
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Jakub Jelinek @ 2022-01-06  9:24 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

The following testcase used to be incorrectly accepted.  The match.pd
optimization that uses address_compare punts on folding comparison
of start of one object and end of another one only when those addresses
are cast to integral types, when the comparison is done on pointer types
it assumes undefined behavior and decides to fold the comparison such
that the addresses don't compare equal even when they at runtime they
could be equal.
But C++ says it is undefined behavior and so during constant evaluation
we should reject those, so this patch adds !folding_initializer &&
check to that spot.

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

Note, address_compare has some special cases, e.g. it assumes that
static vars are never adjacent to automatic vars, which is the case
for the usual layout where automatic vars are on the stack and after
.rodata/.data sections there is heap:
  /* Assume that automatic variables can't be adjacent to global
     variables.  */
  else if (is_global_var (base0) != is_global_var (base1))
    ;
Is it ok that during constant evaluation we don't treat those as undefined
behavior, or shall that be with !folding_initializer && too?

Another special case is:
  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
       || (TREE_CODE (base0) == STRING_CST
           && TREE_CODE (base1) == STRING_CST
           && ioff0 >= 0 && ioff1 >= 0
           && ioff0 < TREE_STRING_LENGTH (base0)
           && ioff1 < TREE_STRING_LENGTH (base1)
          /* This is a too conservative test that the STRING_CSTs
             will not end up being string-merged.  */
           && strncmp (TREE_STRING_POINTER (base0) + ioff0,
                       TREE_STRING_POINTER (base1) + ioff1,
                       MIN (TREE_STRING_LENGTH (base0) - ioff0,
                            TREE_STRING_LENGTH (base1) - ioff1)) != 0))
    ;
  else if (!DECL_P (base0) || !DECL_P (base1))
    return 2;
Here we similarly assume that vars aren't adjacent to string literals
or vice versa.  Do we need to stick !folding_initializer && to those
DECL_P vs. STRING_CST cases?  Though, because of the return 2; for
non-DECL_P that would mean rejecting comparisons like &var == &"foobar"[3]
etc. which ought to be fine, no?  So perhaps we need to watch for
decls. vs. STRING_CSTs like for DECLs whether the address is at the start
or at the end of the string literal or somewhere in between (at least
for folding_initializer)?
And yet another chapter but probably unsolvable is comparison of
string literal addresses.  I think pedantically in C++
&"foo"[0] == &"foo"[0] is undefined behavior, different occurences of
the same string literals might still not be merged in some implementations.
But constexpr const char *s = "foo"; &s[0] == &s[0] should be well defined,
and we aren't tracking anywhere whether the string literal was the same one
or different (and I think other compilers don't track that either).

2022-01-06  Jakub Jelinek  <jakub@redhat.com>

	PR c++/89074
	* fold-const.c (address_compare): Punt on comparison of address of
	one object with address of end of another object if
	folding_initializer.

	* g++.dg/cpp1y/constexpr-89074-1.C: New test.

--- gcc/fold-const.c.jj	2022-01-05 20:30:08.731806756 +0100
+++ gcc/fold-const.c	2022-01-05 20:34:52.277822349 +0100
@@ -16627,7 +16627,7 @@ address_compare (tree_code code, tree ty
   /* If this is a pointer comparison, ignore for now even
      valid equalities where one pointer is the offset zero
      of one object and the other to one past end of another one.  */
-  else if (!INTEGRAL_TYPE_P (type))
+  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
     ;
   /* Assume that automatic variables can't be adjacent to global
      variables.  */
--- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-1.C.jj	2022-01-05 20:43:03.696917484 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-1.C	2022-01-05 20:42:12.676634044 +0100
@@ -0,0 +1,28 @@
+// PR c++/89074
+// { dg-do compile { target c++14 } }
+
+constexpr bool
+foo ()
+{
+  int a[] = { 1, 2 };
+  int b[] = { 3, 4 };
+
+  if (&a[0] == &b[0])
+    return false;
+
+  if (&a[1] == &b[0])
+    return false;
+
+  if (&a[1] == &b[1])
+    return false;
+
+  if (&a[2] == &b[1])
+    return false;
+
+  if (&a[2] == &b[0])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool a = foo ();

	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074]
  2022-01-06  9:24 [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074] Jakub Jelinek
@ 2022-01-10 14:10 ` Richard Biener
  2022-01-11  3:24   ` Andrew Pinski
  2022-01-13 17:35 ` Patch ping (Re: [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074]) Jakub Jelinek
  2022-01-13 21:18 ` [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074] Jason Merrill
  2 siblings, 1 reply; 21+ messages in thread
From: Richard Biener @ 2022-01-10 14:10 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, GCC Patches

On Thu, Jan 6, 2022 at 10:25 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi!
>
> The following testcase used to be incorrectly accepted.  The match.pd
> optimization that uses address_compare punts on folding comparison
> of start of one object and end of another one only when those addresses
> are cast to integral types, when the comparison is done on pointer types
> it assumes undefined behavior and decides to fold the comparison such
> that the addresses don't compare equal even when they at runtime they
> could be equal.
> But C++ says it is undefined behavior and so during constant evaluation
> we should reject those, so this patch adds !folding_initializer &&
> check to that spot.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Note, address_compare has some special cases, e.g. it assumes that
> static vars are never adjacent to automatic vars, which is the case
> for the usual layout where automatic vars are on the stack and after
> .rodata/.data sections there is heap:
>   /* Assume that automatic variables can't be adjacent to global
>      variables.  */
>   else if (is_global_var (base0) != is_global_var (base1))
>     ;
> Is it ok that during constant evaluation we don't treat those as undefined
> behavior, or shall that be with !folding_initializer && too?
>
> Another special case is:
>   if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
>        || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
>        || (TREE_CODE (base0) == STRING_CST
>            && TREE_CODE (base1) == STRING_CST
>            && ioff0 >= 0 && ioff1 >= 0
>            && ioff0 < TREE_STRING_LENGTH (base0)
>            && ioff1 < TREE_STRING_LENGTH (base1)
>           /* This is a too conservative test that the STRING_CSTs
>              will not end up being string-merged.  */
>            && strncmp (TREE_STRING_POINTER (base0) + ioff0,
>                        TREE_STRING_POINTER (base1) + ioff1,
>                        MIN (TREE_STRING_LENGTH (base0) - ioff0,
>                             TREE_STRING_LENGTH (base1) - ioff1)) != 0))
>     ;
>   else if (!DECL_P (base0) || !DECL_P (base1))
>     return 2;
> Here we similarly assume that vars aren't adjacent to string literals
> or vice versa.  Do we need to stick !folding_initializer && to those
> DECL_P vs. STRING_CST cases?  Though, because of the return 2; for
> non-DECL_P that would mean rejecting comparisons like &var == &"foobar"[3]
> etc. which ought to be fine, no?  So perhaps we need to watch for
> decls. vs. STRING_CSTs like for DECLs whether the address is at the start
> or at the end of the string literal or somewhere in between (at least
> for folding_initializer)?
> And yet another chapter but probably unsolvable is comparison of
> string literal addresses.  I think pedantically in C++
> &"foo"[0] == &"foo"[0] is undefined behavior, different occurences of
> the same string literals might still not be merged in some implementations.
> But constexpr const char *s = "foo"; &s[0] == &s[0] should be well defined,
> and we aren't tracking anywhere whether the string literal was the same one
> or different (and I think other compilers don't track that either).

On my TODO list is to make &"foo" invalid and instead require &CONST_DECL
(and DECL_INITIAL of it then being "foo"), that would make it possible to
track the "original" string literal and perform string merging in a
more considerate
way.

Richard.

>
> 2022-01-06  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/89074
>         * fold-const.c (address_compare): Punt on comparison of address of
>         one object with address of end of another object if
>         folding_initializer.
>
>         * g++.dg/cpp1y/constexpr-89074-1.C: New test.
>
> --- gcc/fold-const.c.jj 2022-01-05 20:30:08.731806756 +0100
> +++ gcc/fold-const.c    2022-01-05 20:34:52.277822349 +0100
> @@ -16627,7 +16627,7 @@ address_compare (tree_code code, tree ty
>    /* If this is a pointer comparison, ignore for now even
>       valid equalities where one pointer is the offset zero
>       of one object and the other to one past end of another one.  */
> -  else if (!INTEGRAL_TYPE_P (type))
> +  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
>      ;
>    /* Assume that automatic variables can't be adjacent to global
>       variables.  */
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-1.C.jj   2022-01-05 20:43:03.696917484 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-1.C      2022-01-05 20:42:12.676634044 +0100
> @@ -0,0 +1,28 @@
> +// PR c++/89074
> +// { dg-do compile { target c++14 } }
> +
> +constexpr bool
> +foo ()
> +{
> +  int a[] = { 1, 2 };
> +  int b[] = { 3, 4 };
> +
> +  if (&a[0] == &b[0])
> +    return false;
> +
> +  if (&a[1] == &b[0])
> +    return false;
> +
> +  if (&a[1] == &b[1])
> +    return false;
> +
> +  if (&a[2] == &b[1])
> +    return false;
> +
> +  if (&a[2] == &b[0])          // { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool a = foo ();
>
>         Jakub
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074]
  2022-01-10 14:10 ` Richard Biener
@ 2022-01-11  3:24   ` Andrew Pinski
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Pinski @ 2022-01-11  3:24 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jakub Jelinek, GCC Patches

On Mon, Jan 10, 2022 at 6:11 AM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Thu, Jan 6, 2022 at 10:25 AM Jakub Jelinek via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Hi!
> >
> > The following testcase used to be incorrectly accepted.  The match.pd
> > optimization that uses address_compare punts on folding comparison
> > of start of one object and end of another one only when those addresses
> > are cast to integral types, when the comparison is done on pointer types
> > it assumes undefined behavior and decides to fold the comparison such
> > that the addresses don't compare equal even when they at runtime they
> > could be equal.
> > But C++ says it is undefined behavior and so during constant evaluation
> > we should reject those, so this patch adds !folding_initializer &&
> > check to that spot.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > Note, address_compare has some special cases, e.g. it assumes that
> > static vars are never adjacent to automatic vars, which is the case
> > for the usual layout where automatic vars are on the stack and after
> > .rodata/.data sections there is heap:
> >   /* Assume that automatic variables can't be adjacent to global
> >      variables.  */
> >   else if (is_global_var (base0) != is_global_var (base1))
> >     ;
> > Is it ok that during constant evaluation we don't treat those as undefined
> > behavior, or shall that be with !folding_initializer && too?
> >
> > Another special case is:
> >   if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> >        || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> >        || (TREE_CODE (base0) == STRING_CST
> >            && TREE_CODE (base1) == STRING_CST
> >            && ioff0 >= 0 && ioff1 >= 0
> >            && ioff0 < TREE_STRING_LENGTH (base0)
> >            && ioff1 < TREE_STRING_LENGTH (base1)
> >           /* This is a too conservative test that the STRING_CSTs
> >              will not end up being string-merged.  */
> >            && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> >                        TREE_STRING_POINTER (base1) + ioff1,
> >                        MIN (TREE_STRING_LENGTH (base0) - ioff0,
> >                             TREE_STRING_LENGTH (base1) - ioff1)) != 0))
> >     ;
> >   else if (!DECL_P (base0) || !DECL_P (base1))
> >     return 2;
> > Here we similarly assume that vars aren't adjacent to string literals
> > or vice versa.  Do we need to stick !folding_initializer && to those
> > DECL_P vs. STRING_CST cases?  Though, because of the return 2; for
> > non-DECL_P that would mean rejecting comparisons like &var == &"foobar"[3]
> > etc. which ought to be fine, no?  So perhaps we need to watch for
> > decls. vs. STRING_CSTs like for DECLs whether the address is at the start
> > or at the end of the string literal or somewhere in between (at least
> > for folding_initializer)?
> > And yet another chapter but probably unsolvable is comparison of
> > string literal addresses.  I think pedantically in C++
> > &"foo"[0] == &"foo"[0] is undefined behavior, different occurences of
> > the same string literals might still not be merged in some implementations.
> > But constexpr const char *s = "foo"; &s[0] == &s[0] should be well defined,
> > and we aren't tracking anywhere whether the string literal was the same one
> > or different (and I think other compilers don't track that either).
>
> On my TODO list is to make &"foo" invalid and instead require &CONST_DECL
> (and DECL_INITIAL of it then being "foo"), that would make it possible to
> track the "original" string literal and perform string merging in a
> more considerate way.

Interesting because I wrote this would be one way to fix PR88925.

Thanks,
Andrew Pinski

>
> Richard.
>
> >
> > 2022-01-06  Jakub Jelinek  <jakub@redhat.com>
> >
> >         PR c++/89074
> >         * fold-const.c (address_compare): Punt on comparison of address of
> >         one object with address of end of another object if
> >         folding_initializer.
> >
> >         * g++.dg/cpp1y/constexpr-89074-1.C: New test.
> >
> > --- gcc/fold-const.c.jj 2022-01-05 20:30:08.731806756 +0100
> > +++ gcc/fold-const.c    2022-01-05 20:34:52.277822349 +0100
> > @@ -16627,7 +16627,7 @@ address_compare (tree_code code, tree ty
> >    /* If this is a pointer comparison, ignore for now even
> >       valid equalities where one pointer is the offset zero
> >       of one object and the other to one past end of another one.  */
> > -  else if (!INTEGRAL_TYPE_P (type))
> > +  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
> >      ;
> >    /* Assume that automatic variables can't be adjacent to global
> >       variables.  */
> > --- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-1.C.jj   2022-01-05 20:43:03.696917484 +0100
> > +++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-1.C      2022-01-05 20:42:12.676634044 +0100
> > @@ -0,0 +1,28 @@
> > +// PR c++/89074
> > +// { dg-do compile { target c++14 } }
> > +
> > +constexpr bool
> > +foo ()
> > +{
> > +  int a[] = { 1, 2 };
> > +  int b[] = { 3, 4 };
> > +
> > +  if (&a[0] == &b[0])
> > +    return false;
> > +
> > +  if (&a[1] == &b[0])
> > +    return false;
> > +
> > +  if (&a[1] == &b[1])
> > +    return false;
> > +
> > +  if (&a[2] == &b[1])
> > +    return false;
> > +
> > +  if (&a[2] == &b[0])          // { dg-error "is not a constant expression" }
> > +    return false;
> > +
> > +  return true;
> > +}
> > +
> > +constexpr bool a = foo ();
> >
> >         Jakub
> >

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Patch ping (Re: [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074])
  2022-01-06  9:24 [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074] Jakub Jelinek
  2022-01-10 14:10 ` Richard Biener
@ 2022-01-13 17:35 ` Jakub Jelinek
  2022-01-13 21:18 ` [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074] Jason Merrill
  2 siblings, 0 replies; 21+ messages in thread
From: Jakub Jelinek @ 2022-01-13 17:35 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

I'd like to ping this patch:

> 2022-01-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89074
> 	* fold-const.c (address_compare): Punt on comparison of address of
> 	one object with address of end of another object if
> 	folding_initializer.
> 
> 	* g++.dg/cpp1y/constexpr-89074-1.C: New test.

Thanks.

	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074]
  2022-01-06  9:24 [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074] Jakub Jelinek
  2022-01-10 14:10 ` Richard Biener
  2022-01-13 17:35 ` Patch ping (Re: [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074]) Jakub Jelinek
@ 2022-01-13 21:18 ` Jason Merrill
  2022-01-18 10:17   ` [PATCH] c++: Further address_compare fixes [PR89074] Jakub Jelinek
  2 siblings, 1 reply; 21+ messages in thread
From: Jason Merrill @ 2022-01-13 21:18 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 1/6/22 04:24, Jakub Jelinek wrote:
> 
> The following testcase used to be incorrectly accepted.  The match.pd
> optimization that uses address_compare punts on folding comparison
> of start of one object and end of another one only when those addresses
> are cast to integral types, when the comparison is done on pointer types
> it assumes undefined behavior and decides to fold the comparison such
> that the addresses don't compare equal even when they at runtime they
> could be equal.
> But C++ says it is undefined behavior and so during constant evaluation
> we should reject those, so this patch adds !folding_initializer &&
> check to that spot.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> Note, address_compare has some special cases, e.g. it assumes that
> static vars are never adjacent to automatic vars, which is the case
> for the usual layout where automatic vars are on the stack and after
> .rodata/.data sections there is heap:
>    /* Assume that automatic variables can't be adjacent to global
>       variables.  */
>    else if (is_global_var (base0) != is_global_var (base1))
>      ;
> Is it ok that during constant evaluation we don't treat those as undefined
> behavior, or shall that be with !folding_initializer && too?

I guess that's undefined as well.

> Another special case is:
>    if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
>         || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
>         || (TREE_CODE (base0) == STRING_CST
>             && TREE_CODE (base1) == STRING_CST
>             && ioff0 >= 0 && ioff1 >= 0
>             && ioff0 < TREE_STRING_LENGTH (base0)
>             && ioff1 < TREE_STRING_LENGTH (base1)
>            /* This is a too conservative test that the STRING_CSTs
>               will not end up being string-merged.  */
>             && strncmp (TREE_STRING_POINTER (base0) + ioff0,
>                         TREE_STRING_POINTER (base1) + ioff1,
>                         MIN (TREE_STRING_LENGTH (base0) - ioff0,
>                              TREE_STRING_LENGTH (base1) - ioff1)) != 0))
>      ;
>    else if (!DECL_P (base0) || !DECL_P (base1))
>      return 2;
> Here we similarly assume that vars aren't adjacent to string literals
> or vice versa.  Do we need to stick !folding_initializer && to those
> DECL_P vs. STRING_CST cases?

Seems so.

> Though, because of the return 2; for
> non-DECL_P that would mean rejecting comparisons like &var == &"foobar"[3]
> etc. which ought to be fine, no?  So perhaps we need to watch for
> decls. vs. STRING_CSTs like for DECLs whether the address is at the start
> or at the end of the string literal or somewhere in between (at least
> for folding_initializer)?

Agreed.

> And yet another chapter but probably unsolvable is comparison of
> string literal addresses.  I think pedantically in C++
> &"foo"[0] == &"foo"[0] is undefined behavior, different occurences of
> the same string literals might still not be merged in some implementations.

I disagree; it's unspecified whether string literals are merged, but I 
think the comparison result is well specified depending on that 
implementation behavior.

> But constexpr const char *s = "foo"; &s[0] == &s[0] should be well defined,
> and we aren't tracking anywhere whether the string literal was the same one
> or different (and I think other compilers don't track that either).
> 
> 2022-01-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89074
> 	* fold-const.c (address_compare): Punt on comparison of address of
> 	one object with address of end of another object if
> 	folding_initializer.
> 
> 	* g++.dg/cpp1y/constexpr-89074-1.C: New test.
> 
> --- gcc/fold-const.c.jj	2022-01-05 20:30:08.731806756 +0100
> +++ gcc/fold-const.c	2022-01-05 20:34:52.277822349 +0100
> @@ -16627,7 +16627,7 @@ address_compare (tree_code code, tree ty
>     /* If this is a pointer comparison, ignore for now even
>        valid equalities where one pointer is the offset zero
>        of one object and the other to one past end of another one.  */
> -  else if (!INTEGRAL_TYPE_P (type))
> +  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
>       ;
>     /* Assume that automatic variables can't be adjacent to global
>        variables.  */
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-1.C.jj	2022-01-05 20:43:03.696917484 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-1.C	2022-01-05 20:42:12.676634044 +0100
> @@ -0,0 +1,28 @@
> +// PR c++/89074
> +// { dg-do compile { target c++14 } }
> +
> +constexpr bool
> +foo ()
> +{
> +  int a[] = { 1, 2 };
> +  int b[] = { 3, 4 };
> +
> +  if (&a[0] == &b[0])
> +    return false;
> +
> +  if (&a[1] == &b[0])
> +    return false;
> +
> +  if (&a[1] == &b[1])
> +    return false;
> +
> +  if (&a[2] == &b[1])
> +    return false;
> +
> +  if (&a[2] == &b[0])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool a = foo ();


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] c++: Further address_compare fixes [PR89074]
  2022-01-13 21:18 ` [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074] Jason Merrill
@ 2022-01-18 10:17   ` Jakub Jelinek
  2022-01-18 12:30     ` Jakub Jelinek
  2022-01-18 16:25     ` Jason Merrill
  0 siblings, 2 replies; 21+ messages in thread
From: Jakub Jelinek @ 2022-01-18 10:17 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Thu, Jan 13, 2022 at 04:18:33PM -0500, Jason Merrill wrote:
> > Note, address_compare has some special cases, e.g. it assumes that
> > static vars are never adjacent to automatic vars, which is the case
> > for the usual layout where automatic vars are on the stack and after
> > .rodata/.data sections there is heap:
> >    /* Assume that automatic variables can't be adjacent to global
> >       variables.  */
> >    else if (is_global_var (base0) != is_global_var (base1))
> >      ;
> > Is it ok that during constant evaluation we don't treat those as undefined
> > behavior, or shall that be with !folding_initializer && too?
> 
> I guess that's undefined as well.

Ok, following patch seems to guard that too

> > Another special case is:
> >    if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> >         || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> >         || (TREE_CODE (base0) == STRING_CST
> >             && TREE_CODE (base1) == STRING_CST
> >             && ioff0 >= 0 && ioff1 >= 0
> >             && ioff0 < TREE_STRING_LENGTH (base0)
> >             && ioff1 < TREE_STRING_LENGTH (base1)
> >            /* This is a too conservative test that the STRING_CSTs
> >               will not end up being string-merged.  */
> >             && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> >                         TREE_STRING_POINTER (base1) + ioff1,
> >                         MIN (TREE_STRING_LENGTH (base0) - ioff0,
> >                              TREE_STRING_LENGTH (base1) - ioff1)) != 0))
> >      ;
> >    else if (!DECL_P (base0) || !DECL_P (base1))
> >      return 2;
> > Here we similarly assume that vars aren't adjacent to string literals
> > or vice versa.  Do we need to stick !folding_initializer && to those
> > DECL_P vs. STRING_CST cases?
> 
> Seems so.

and this too

> > Though, because of the return 2; for
> > non-DECL_P that would mean rejecting comparisons like &var == &"foobar"[3]
> > etc. which ought to be fine, no?  So perhaps we need to watch for
> > decls. vs. STRING_CSTs like for DECLs whether the address is at the start
> > or at the end of the string literal or somewhere in between (at least
> > for folding_initializer)?
> 
> Agreed.

and this as well.
Furthermore I've fixed constexpr-compare2.C by assuming if
folding_initializer that addresses of non-aliased (visibly to the compiler)
FUNCTION_DECLs are different and that functions are non-zero sized for the
purpose of var vs. function comparisons.

> > And yet another chapter but probably unsolvable is comparison of
> > string literal addresses.  I think pedantically in C++
> > &"foo"[0] == &"foo"[0] is undefined behavior, different occurences of
> > the same string literals might still not be merged in some implementations.
> 
> I disagree; it's unspecified whether string literals are merged, but I think
> the comparison result is well specified depending on that implementation
> behavior.

Can you please comment on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86369#c1
then?

Anyway, the following has been successfully bootstrapped/regtested on
x86_64-linux and i686-linux, ok for trunk?

2022-01-18  Jakub Jelinek  <jakub@redhat.com>

	PR c++/89074
	PR c++/104033
	* fold-const.c (address_compare): Restrict the decl vs. STRING_CST
	or vice versa or STRING_CST vs. STRING_CST or
	is_global_var != is_global_var optimizations to !folding_initializer.
	Punt for FUNCTION_DECLs with non-zero offsets.  If folding_initializer,
	assume non-aliased functions have non-zero size and have different
	addresses.  For folding_initializer, punt on comparisons of start
	of some object and end of another one, regardless whether it is a decl
	or string literal.  Also punt for folding_initializer of
	STRING_CST vs. STRING_CST comparisons if the two literals could be
	overlapping.

	* g++.dg/cpp1y/constexpr-89074-3.C: New test.

--- gcc/fold-const.c.jj	2022-01-17 14:19:08.817376382 +0100
+++ gcc/fold-const.c	2022-01-17 15:50:16.687211071 +0100
@@ -16608,21 +16608,27 @@ address_compare (tree_code code, tree ty
   HOST_WIDE_INT ioff0 = -1, ioff1 = -1;
   off0.is_constant (&ioff0);
   off1.is_constant (&ioff1);
-  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
-       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
-       || (TREE_CODE (base0) == STRING_CST
-	   && TREE_CODE (base1) == STRING_CST
-	   && ioff0 >= 0 && ioff1 >= 0
-	   && ioff0 < TREE_STRING_LENGTH (base0)
-	   && ioff1 < TREE_STRING_LENGTH (base1)
-	  /* This is a too conservative test that the STRING_CSTs
-	     will not end up being string-merged.  */
-	   && strncmp (TREE_STRING_POINTER (base0) + ioff0,
-		       TREE_STRING_POINTER (base1) + ioff1,
-		       MIN (TREE_STRING_LENGTH (base0) - ioff0,
-			    TREE_STRING_LENGTH (base1) - ioff1)) != 0))
+  if (!folding_initializer
+      && ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
+	  || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
+	  || (TREE_CODE (base0) == STRING_CST
+	      && TREE_CODE (base1) == STRING_CST
+	      && ioff0 >= 0 && ioff1 >= 0
+	      && ioff0 < TREE_STRING_LENGTH (base0)
+	      && ioff1 < TREE_STRING_LENGTH (base1)
+	      /* This is a too conservative test that the STRING_CSTs
+		 will not end up being string-merged.  */
+	      && strncmp (TREE_STRING_POINTER (base0) + ioff0,
+			  TREE_STRING_POINTER (base1) + ioff1,
+			  MIN (TREE_STRING_LENGTH (base0) - ioff0,
+			       TREE_STRING_LENGTH (base1) - ioff1)) != 0)))
     ;
-  else if (!DECL_P (base0) || !DECL_P (base1))
+  /* Punt on non-zero offsets from functions.  */
+  else if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
+	   || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
+    return 2;
+  else if ((!DECL_P (base0) && TREE_CODE (base0) != STRING_CST)
+	   || (!DECL_P (base1) && TREE_CODE (base1) != STRING_CST))
     return 2;
   /* If this is a pointer comparison, ignore for now even
      valid equalities where one pointer is the offset zero
@@ -16631,18 +16637,62 @@ address_compare (tree_code code, tree ty
     ;
   /* Assume that automatic variables can't be adjacent to global
      variables.  */
-  else if (is_global_var (base0) != is_global_var (base1))
+  else if (!folding_initializer
+	   && is_global_var (base0) != is_global_var (base1))
+    ;
+  /* For initializers, assume addresses of different functions are
+     different.  */
+  else if (folding_initializer
+	   && TREE_CODE (base0) == FUNCTION_DECL
+	   && TREE_CODE (base1) == FUNCTION_DECL)
     ;
   else
     {
-      tree sz0 = DECL_SIZE_UNIT (base0);
-      tree sz1 = DECL_SIZE_UNIT (base1);
-      /* If sizes are unknown, e.g. VLA or not representable, punt.  */
-      if (!tree_fits_poly_int64_p (sz0) || !tree_fits_poly_int64_p (sz1))
-	return 2;
+      poly_int64 size0, size1;
+      if (TREE_CODE (base0) == STRING_CST)
+	{
+	  if (!folding_initializer
+	      || ioff0 < 0
+	      || ioff0 > TREE_STRING_LENGTH (base0))
+	    return 2;
+	  size0 = TREE_STRING_LENGTH (base0);
+	}
+      /* For initializers, assume function decls don't overlap and have
+	 non-empty size.  */
+      else if (folding_initializer && TREE_CODE (base0) == FUNCTION_DECL)
+	size0 = 1;
+      else
+	{
+	  tree sz0 = DECL_SIZE_UNIT (base0);
+	  /* If sizes are unknown, e.g. VLA or not representable, punt.  */
+	  if (!tree_fits_poly_int64_p (sz0))
+	    return 2;
+
+	  size0 = tree_to_poly_int64 (sz0);
+	}
+
+      if (TREE_CODE (base1) == STRING_CST)
+	{
+	  if (!folding_initializer
+	      || ioff1 < 0
+	      || ioff1 > TREE_STRING_LENGTH (base1))
+	    return 2;
+	  size1 = TREE_STRING_LENGTH (base1);
+	}
+      /* For initializers, assume function decls don't overlap and have
+	 non-empty size.  */
+      else if (folding_initializer && TREE_CODE (base1) == FUNCTION_DECL)
+	size1 = 1;
+      else
+	{
+	  tree sz1 = DECL_SIZE_UNIT (base1);
+	  /* If sizes are unknown, e.g. VLA or not representable, punt.  */
+	  if (!tree_fits_poly_int64_p (sz1))
+	    return 2;
+
+	  size1 = tree_to_poly_int64 (sz1);
+	}
 
-      poly_int64 size0 = tree_to_poly_int64 (sz0);
-      poly_int64 size1 = tree_to_poly_int64 (sz1);
       /* If one offset is pointing (or could be) to the beginning of one
 	 object and the other is pointing to one past the last byte of the
 	 other object, punt.  */
@@ -16658,6 +16708,27 @@ address_compare (tree_code code, tree ty
 	  && (known_ne (off0, 0)
 	      || (known_ne (size0, 0) && known_ne (size1, 0))))
 	equal = 0;
+      if (equal == 0
+	  && TREE_CODE (base0) == STRING_CST
+	  && TREE_CODE (base1) == STRING_CST)
+	{
+	  /* If the bytes in the string literals starting at the pointers
+	     differ, the pointers need to be different.  */
+	  if (memcmp (TREE_STRING_POINTER (base0) + ioff0,
+		      TREE_STRING_POINTER (base1) + ioff1,
+		      MIN (TREE_STRING_LENGTH (base0) - ioff0,
+			   TREE_STRING_LENGTH (base1) - ioff1)) == 0)
+	    {
+	      HOST_WIDE_INT ioffmin = MIN (ioff0, ioff1);
+	      if (memcmp (TREE_STRING_POINTER (base0) + ioff0 - ioffmin,
+			  TREE_STRING_POINTER (base1) + ioff1 - ioffmin,
+			  ioffmin) == 0)
+		/* If even the bytes in the string literal before the
+		   pointers are the same, the string literals could be
+		   tail merged.  */
+		return 2;
+	    }
+	}
      }
   return equal;
 }
--- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C.jj	2022-01-17 15:22:43.743566175 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C	2022-01-17 16:10:19.182230570 +0100
@@ -0,0 +1,132 @@
+// PR c++/89074
+// { dg-do compile { target c++14 } }
+
+int fn1 (void) { return 0; }
+int fn2 (void) { return 1; }
+
+constexpr bool
+f1 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  if (&a[1] == "foo")
+    return false;
+
+  if (&a[1] == &"foo"[4])
+    return false;
+
+  if (&"foo"[1] == &a[0])
+    return false;
+
+  if (&"foo"[3] == &a[4])
+    return false;
+
+  if (&a[0] == "foo")
+    return false;
+
+  // Pointer to start of one object (var) and end of another one (literal)
+  if (&a[0] == &"foo"[4])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f2 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  // Pointer to end of one object (var) and start of another one (literal)
+  if (&a[4] == "foo")		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+char v[] = { 1, 2, 3, 4 };
+
+constexpr bool
+f3 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  if (&a[1] == &v[1])
+    return false;
+
+  if (&a[0] == &v[3])
+    return false;
+
+  if (&a[2] == &v[4])
+    return false;
+
+  // Pointer to start of one object (automatic var) and end of another one (non-automagic var)
+  if (&a[0] == &v[4])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f4 ()
+{
+  char a[] = { 1, 2, 3, 4, 5 };
+
+  // Pointer to end of one object (automatic var) and start of another one (non-automagic var)
+  if (&a[5] == &v[0])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f5 ()
+{
+  if (fn1 != fn1)
+    return false;
+
+  if (fn1 == fn2)
+    return false;
+
+  if (&"abcde"[0] == &"edcba"[1])
+    return false;
+
+  if (&"abcde"[1] == &"edcba"[6])
+    return false;
+
+  // Pointer to start of one object (literal) and end of another one (literal)
+  if (&"abcde"[0] == &"edcba"[6])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f6 ()
+{
+  // Pointer to start of one object (literal) and end of another one (literal)
+  if (&"abcde"[6] == &"edcba"[0])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f7 ()
+{
+  if (&"abcde"[3] == &"fabcde"[3])
+    return false;
+
+  // These could be suffix merged, with &"abcde"[0] == &"fabcde"[1].
+  if (&"abcde"[3] == &"fabcde"[4])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool a = f1 ();
+constexpr bool b = f2 ();
+constexpr bool c = f3 ();
+constexpr bool d = f4 ();
+constexpr bool e = f5 ();
+constexpr bool f = f6 ();
+constexpr bool g = f7 ();

	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++: Further address_compare fixes [PR89074]
  2022-01-18 10:17   ` [PATCH] c++: Further address_compare fixes [PR89074] Jakub Jelinek
@ 2022-01-18 12:30     ` Jakub Jelinek
  2022-01-18 16:25     ` Jason Merrill
  1 sibling, 0 replies; 21+ messages in thread
From: Jakub Jelinek @ 2022-01-18 12:30 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Tue, Jan 18, 2022 at 11:17:41AM +0100, Jakub Jelinek via Gcc-patches wrote:
> Anyway, the following has been successfully bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?

Actually, I missed one regression (thought it is caused by PR104025 patch
but it is this one):
+FAIL: experimental/source_location/1.cc execution test

The test does (in multiple spots):
  VERIFY( loc.file_name() == __FILE__ );
where file_name() is constexpr and returns const char *.
address_compare sees EQ_EXPR where the string literals have exactly the
same bytes, but are different tree nodes.  The test clearly relies on
the string literal merging GCC does...

To restore the previous behavior (note, this was !folding_initializer case),
I need following incremental patch, i.e. make sure I don't change behavior
for the !folding_initializer case.

But, perhaps if we think that different STRING_CSTs with identical
content should compare equal (which is likely true as expand_expr_constant
-> output_constant_def should ensure that), then maybe we should
earlier in address_compare for two different STRING_CSTs that have
the same TREE_STRING_LENGTH memcmp the content and set equal to 0
earlier.  Again, question whether to do that always, or just for
!folding_initializer, or just for folding_initializer.

--- gcc/fold-const.cc.jj	2022-01-18 13:10:56.864364624 +0100
+++ gcc/fold-const.cc	2022-01-18 13:25:08.057491249 +0100
@@ -16627,8 +16627,10 @@ address_compare (tree_code code, tree ty
   else if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
 	   || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
     return 2;
-  else if ((!DECL_P (base0) && TREE_CODE (base0) != STRING_CST)
-	   || (!DECL_P (base1) && TREE_CODE (base1) != STRING_CST))
+  else if ((!DECL_P (base0)
+  	    && (!folding_initializer || TREE_CODE (base0) != STRING_CST))
+	   || (!DECL_P (base1)
+	       && (!folding_initializer || TREE_CODE (base1) != STRING_CST)))
     return 2;
   /* If this is a pointer comparison, ignore for now even
      valid equalities where one pointer is the offset zero
@@ -16651,8 +16653,7 @@ address_compare (tree_code code, tree ty
       poly_int64 size0, size1;
       if (TREE_CODE (base0) == STRING_CST)
 	{
-	  if (!folding_initializer
-	      || ioff0 < 0
+	  if (ioff0 < 0
 	      || ioff0 > TREE_STRING_LENGTH (base0))
 	    return 2;
 	  size0 = TREE_STRING_LENGTH (base0);
@@ -16673,8 +16674,7 @@ address_compare (tree_code code, tree ty
 
       if (TREE_CODE (base1) == STRING_CST)
 	{
-	  if (!folding_initializer
-	      || ioff1 < 0
+	  if (ioff1 < 0
 	      || ioff1 > TREE_STRING_LENGTH (base1))
 	    return 2;
 	  size1 = TREE_STRING_LENGTH (base1);


	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++: Further address_compare fixes [PR89074]
  2022-01-18 10:17   ` [PATCH] c++: Further address_compare fixes [PR89074] Jakub Jelinek
  2022-01-18 12:30     ` Jakub Jelinek
@ 2022-01-18 16:25     ` Jason Merrill
  2022-01-18 16:40       ` Jakub Jelinek
  1 sibling, 1 reply; 21+ messages in thread
From: Jason Merrill @ 2022-01-18 16:25 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Richard Biener

On 1/18/22 05:17, Jakub Jelinek wrote:
> On Thu, Jan 13, 2022 at 04:18:33PM -0500, Jason Merrill wrote:
>>> Note, address_compare has some special cases, e.g. it assumes that
>>> static vars are never adjacent to automatic vars, which is the case
>>> for the usual layout where automatic vars are on the stack and after
>>> .rodata/.data sections there is heap:
>>>     /* Assume that automatic variables can't be adjacent to global
>>>        variables.  */
>>>     else if (is_global_var (base0) != is_global_var (base1))
>>>       ;
>>> Is it ok that during constant evaluation we don't treat those as undefined
>>> behavior, or shall that be with !folding_initializer && too?
>>
>> I guess that's undefined as well.
> 
> Ok, following patch seems to guard that too
> 
>>> Another special case is:
>>>     if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
>>>          || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
>>>          || (TREE_CODE (base0) == STRING_CST
>>>              && TREE_CODE (base1) == STRING_CST
>>>              && ioff0 >= 0 && ioff1 >= 0
>>>              && ioff0 < TREE_STRING_LENGTH (base0)
>>>              && ioff1 < TREE_STRING_LENGTH (base1)
>>>             /* This is a too conservative test that the STRING_CSTs
>>>                will not end up being string-merged.  */
>>>              && strncmp (TREE_STRING_POINTER (base0) + ioff0,
>>>                          TREE_STRING_POINTER (base1) + ioff1,
>>>                          MIN (TREE_STRING_LENGTH (base0) - ioff0,
>>>                               TREE_STRING_LENGTH (base1) - ioff1)) != 0))
>>>       ;
>>>     else if (!DECL_P (base0) || !DECL_P (base1))
>>>       return 2;
>>> Here we similarly assume that vars aren't adjacent to string literals
>>> or vice versa.  Do we need to stick !folding_initializer && to those
>>> DECL_P vs. STRING_CST cases?
>>
>> Seems so.
> 
> and this too
> 
>>> Though, because of the return 2; for
>>> non-DECL_P that would mean rejecting comparisons like &var == &"foobar"[3]
>>> etc. which ought to be fine, no?  So perhaps we need to watch for
>>> decls. vs. STRING_CSTs like for DECLs whether the address is at the start
>>> or at the end of the string literal or somewhere in between (at least
>>> for folding_initializer)?
>>
>> Agreed.
> 
> and this as well.
> Furthermore I've fixed constexpr-compare2.C by assuming if
> folding_initializer that addresses of non-aliased (visibly to the compiler)
> FUNCTION_DECLs are different and that functions are non-zero sized for the
> purpose of var vs. function comparisons.
> 
>>> And yet another chapter but probably unsolvable is comparison of
>>> string literal addresses.  I think pedantically in C++
>>> &"foo"[0] == &"foo"[0] is undefined behavior, different occurences of
>>> the same string literals might still not be merged in some implementations.
>>
>> I disagree; it's unspecified whether string literals are merged, but I think
>> the comparison result is well specified depending on that implementation
>> behavior.
> 
> Can you please comment on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86369#c1
> then?

Done.

About the rest of the patch, I thought I had seen richi comment on IRC 
(but can't find the comment now) that these cases where we could give a 
constant answer but decide not to because of C++ rules should be handled 
in the front end rather than generic code, which makes sense to me; that 
is, use folding_initializer only for giving more constant results, not 
for giving fewer constant results.  Maybe add another flag for limiting 
constant results if we think it's significantly easier to recognize 
these cases in fold?

> Anyway, the following has been successfully bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?
> 
> 2022-01-18  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89074
> 	PR c++/104033
> 	* fold-const.c (address_compare): Restrict the decl vs. STRING_CST
> 	or vice versa or STRING_CST vs. STRING_CST or
> 	is_global_var != is_global_var optimizations to !folding_initializer.
> 	Punt for FUNCTION_DECLs with non-zero offsets.  If folding_initializer,
> 	assume non-aliased functions have non-zero size and have different
> 	addresses.  For folding_initializer, punt on comparisons of start
> 	of some object and end of another one, regardless whether it is a decl
> 	or string literal.  Also punt for folding_initializer of
> 	STRING_CST vs. STRING_CST comparisons if the two literals could be
> 	overlapping.
> 
> 	* g++.dg/cpp1y/constexpr-89074-3.C: New test.
> 
> --- gcc/fold-const.c.jj	2022-01-17 14:19:08.817376382 +0100
> +++ gcc/fold-const.c	2022-01-17 15:50:16.687211071 +0100
> @@ -16608,21 +16608,27 @@ address_compare (tree_code code, tree ty
>     HOST_WIDE_INT ioff0 = -1, ioff1 = -1;
>     off0.is_constant (&ioff0);
>     off1.is_constant (&ioff1);
> -  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> -       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> -       || (TREE_CODE (base0) == STRING_CST
> -	   && TREE_CODE (base1) == STRING_CST
> -	   && ioff0 >= 0 && ioff1 >= 0
> -	   && ioff0 < TREE_STRING_LENGTH (base0)
> -	   && ioff1 < TREE_STRING_LENGTH (base1)
> -	  /* This is a too conservative test that the STRING_CSTs
> -	     will not end up being string-merged.  */
> -	   && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> -		       TREE_STRING_POINTER (base1) + ioff1,
> -		       MIN (TREE_STRING_LENGTH (base0) - ioff0,
> -			    TREE_STRING_LENGTH (base1) - ioff1)) != 0))
> +  if (!folding_initializer
> +      && ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> +	  || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> +	  || (TREE_CODE (base0) == STRING_CST
> +	      && TREE_CODE (base1) == STRING_CST
> +	      && ioff0 >= 0 && ioff1 >= 0
> +	      && ioff0 < TREE_STRING_LENGTH (base0)
> +	      && ioff1 < TREE_STRING_LENGTH (base1)
> +	      /* This is a too conservative test that the STRING_CSTs
> +		 will not end up being string-merged.  */
> +	      && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> +			  TREE_STRING_POINTER (base1) + ioff1,
> +			  MIN (TREE_STRING_LENGTH (base0) - ioff0,
> +			       TREE_STRING_LENGTH (base1) - ioff1)) != 0)))
>       ;
> -  else if (!DECL_P (base0) || !DECL_P (base1))
> +  /* Punt on non-zero offsets from functions.  */
> +  else if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
> +	   || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
> +    return 2;
> +  else if ((!DECL_P (base0) && TREE_CODE (base0) != STRING_CST)
> +	   || (!DECL_P (base1) && TREE_CODE (base1) != STRING_CST))
>       return 2;
>     /* If this is a pointer comparison, ignore for now even
>        valid equalities where one pointer is the offset zero
> @@ -16631,18 +16637,62 @@ address_compare (tree_code code, tree ty
>       ;
>     /* Assume that automatic variables can't be adjacent to global
>        variables.  */
> -  else if (is_global_var (base0) != is_global_var (base1))
> +  else if (!folding_initializer
> +	   && is_global_var (base0) != is_global_var (base1))
> +    ;
> +  /* For initializers, assume addresses of different functions are
> +     different.  */
> +  else if (folding_initializer
> +	   && TREE_CODE (base0) == FUNCTION_DECL
> +	   && TREE_CODE (base1) == FUNCTION_DECL)
>       ;
>     else
>       {
> -      tree sz0 = DECL_SIZE_UNIT (base0);
> -      tree sz1 = DECL_SIZE_UNIT (base1);
> -      /* If sizes are unknown, e.g. VLA or not representable, punt.  */
> -      if (!tree_fits_poly_int64_p (sz0) || !tree_fits_poly_int64_p (sz1))
> -	return 2;
> +      poly_int64 size0, size1;
> +      if (TREE_CODE (base0) == STRING_CST)
> +	{
> +	  if (!folding_initializer
> +	      || ioff0 < 0
> +	      || ioff0 > TREE_STRING_LENGTH (base0))
> +	    return 2;
> +	  size0 = TREE_STRING_LENGTH (base0);
> +	}
> +      /* For initializers, assume function decls don't overlap and have
> +	 non-empty size.  */
> +      else if (folding_initializer && TREE_CODE (base0) == FUNCTION_DECL)
> +	size0 = 1;
> +      else
> +	{
> +	  tree sz0 = DECL_SIZE_UNIT (base0);
> +	  /* If sizes are unknown, e.g. VLA or not representable, punt.  */
> +	  if (!tree_fits_poly_int64_p (sz0))
> +	    return 2;
> +
> +	  size0 = tree_to_poly_int64 (sz0);
> +	}
> +
> +      if (TREE_CODE (base1) == STRING_CST)
> +	{
> +	  if (!folding_initializer
> +	      || ioff1 < 0
> +	      || ioff1 > TREE_STRING_LENGTH (base1))
> +	    return 2;
> +	  size1 = TREE_STRING_LENGTH (base1);
> +	}
> +      /* For initializers, assume function decls don't overlap and have
> +	 non-empty size.  */
> +      else if (folding_initializer && TREE_CODE (base1) == FUNCTION_DECL)
> +	size1 = 1;
> +      else
> +	{
> +	  tree sz1 = DECL_SIZE_UNIT (base1);
> +	  /* If sizes are unknown, e.g. VLA or not representable, punt.  */
> +	  if (!tree_fits_poly_int64_p (sz1))
> +	    return 2;
> +
> +	  size1 = tree_to_poly_int64 (sz1);
> +	}
>   
> -      poly_int64 size0 = tree_to_poly_int64 (sz0);
> -      poly_int64 size1 = tree_to_poly_int64 (sz1);
>         /* If one offset is pointing (or could be) to the beginning of one
>   	 object and the other is pointing to one past the last byte of the
>   	 other object, punt.  */
> @@ -16658,6 +16708,27 @@ address_compare (tree_code code, tree ty
>   	  && (known_ne (off0, 0)
>   	      || (known_ne (size0, 0) && known_ne (size1, 0))))
>   	equal = 0;
> +      if (equal == 0
> +	  && TREE_CODE (base0) == STRING_CST
> +	  && TREE_CODE (base1) == STRING_CST)
> +	{
> +	  /* If the bytes in the string literals starting at the pointers
> +	     differ, the pointers need to be different.  */
> +	  if (memcmp (TREE_STRING_POINTER (base0) + ioff0,
> +		      TREE_STRING_POINTER (base1) + ioff1,
> +		      MIN (TREE_STRING_LENGTH (base0) - ioff0,
> +			   TREE_STRING_LENGTH (base1) - ioff1)) == 0)
> +	    {
> +	      HOST_WIDE_INT ioffmin = MIN (ioff0, ioff1);
> +	      if (memcmp (TREE_STRING_POINTER (base0) + ioff0 - ioffmin,
> +			  TREE_STRING_POINTER (base1) + ioff1 - ioffmin,
> +			  ioffmin) == 0)
> +		/* If even the bytes in the string literal before the
> +		   pointers are the same, the string literals could be
> +		   tail merged.  */
> +		return 2;
> +	    }
> +	}
>        }
>     return equal;
>   }
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C.jj	2022-01-17 15:22:43.743566175 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C	2022-01-17 16:10:19.182230570 +0100
> @@ -0,0 +1,132 @@
> +// PR c++/89074
> +// { dg-do compile { target c++14 } }
> +
> +int fn1 (void) { return 0; }
> +int fn2 (void) { return 1; }
> +
> +constexpr bool
> +f1 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  if (&a[1] == "foo")
> +    return false;
> +
> +  if (&a[1] == &"foo"[4])
> +    return false;
> +
> +  if (&"foo"[1] == &a[0])
> +    return false;
> +
> +  if (&"foo"[3] == &a[4])
> +    return false;
> +
> +  if (&a[0] == "foo")
> +    return false;
> +
> +  // Pointer to start of one object (var) and end of another one (literal)
> +  if (&a[0] == &"foo"[4])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f2 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  // Pointer to end of one object (var) and start of another one (literal)
> +  if (&a[4] == "foo")		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +char v[] = { 1, 2, 3, 4 };
> +
> +constexpr bool
> +f3 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  if (&a[1] == &v[1])
> +    return false;
> +
> +  if (&a[0] == &v[3])
> +    return false;
> +
> +  if (&a[2] == &v[4])
> +    return false;
> +
> +  // Pointer to start of one object (automatic var) and end of another one (non-automagic var)
> +  if (&a[0] == &v[4])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f4 ()
> +{
> +  char a[] = { 1, 2, 3, 4, 5 };
> +
> +  // Pointer to end of one object (automatic var) and start of another one (non-automagic var)
> +  if (&a[5] == &v[0])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f5 ()
> +{
> +  if (fn1 != fn1)
> +    return false;
> +
> +  if (fn1 == fn2)
> +    return false;
> +
> +  if (&"abcde"[0] == &"edcba"[1])
> +    return false;
> +
> +  if (&"abcde"[1] == &"edcba"[6])
> +    return false;
> +
> +  // Pointer to start of one object (literal) and end of another one (literal)
> +  if (&"abcde"[0] == &"edcba"[6])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f6 ()
> +{
> +  // Pointer to start of one object (literal) and end of another one (literal)
> +  if (&"abcde"[6] == &"edcba"[0])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f7 ()
> +{
> +  if (&"abcde"[3] == &"fabcde"[3])
> +    return false;
> +
> +  // These could be suffix merged, with &"abcde"[0] == &"fabcde"[1].
> +  if (&"abcde"[3] == &"fabcde"[4])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool a = f1 ();
> +constexpr bool b = f2 ();
> +constexpr bool c = f3 ();
> +constexpr bool d = f4 ();
> +constexpr bool e = f5 ();
> +constexpr bool f = f6 ();
> +constexpr bool g = f7 ();
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++: Further address_compare fixes [PR89074]
  2022-01-18 16:25     ` Jason Merrill
@ 2022-01-18 16:40       ` Jakub Jelinek
  2022-01-18 16:56         ` Jason Merrill
  2022-02-03 15:52         ` [PATCH] c++, v2: " Jakub Jelinek
  0 siblings, 2 replies; 21+ messages in thread
From: Jakub Jelinek @ 2022-01-18 16:40 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Richard Biener

On Tue, Jan 18, 2022 at 11:25:38AM -0500, Jason Merrill wrote:
> > Can you please comment on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86369#c1
> > then?
> 
> Done.

Thanks.

> About the rest of the patch, I thought I had seen richi comment on IRC (but
> can't find the comment now) that these cases where we could give a constant
> answer but decide not to because of C++ rules should be handled in the front
> end rather than generic code, which makes sense to me; that is, use
> folding_initializer only for giving more constant results, not for giving
> fewer constant results.  Maybe add another flag for limiting constant
> results if we think it's significantly easier to recognize these cases in
> fold?

I'm afraid avoiding the match.pd & fold-const code here would be a lot of
work.
The match.pd code looks like:
(for cmp (simple_comparison)
 (simplify
  (cmp (convert1?@2 addr@0) (convert2? addr@1))
  (with
   {
     poly_int64 off0, off1;
     tree base0, base1;
     int equal = address_compare (cmp, TREE_TYPE (@2), @0, @1, base0, base1,
                                  off0, off1, GENERIC);
   }
   (if (equal == 1)
    (switch
     (if (cmp == EQ_EXPR && (known_eq (off0, off1) || known_ne (off0, off1)))
      { constant_boolean_node (known_eq (off0, off1), type); })
     (if (cmp == NE_EXPR && (known_eq (off0, off1) || known_ne (off0, off1)))
      { constant_boolean_node (known_ne (off0, off1), type); })
     (if (cmp == LT_EXPR && (known_lt (off0, off1) || known_ge (off0, off1)))
      { constant_boolean_node (known_lt (off0, off1), type); })
     (if (cmp == LE_EXPR && (known_le (off0, off1) || known_gt (off0, off1)))
      { constant_boolean_node (known_le (off0, off1), type); })
     (if (cmp == GE_EXPR && (known_ge (off0, off1) || known_lt (off0, off1)))
      { constant_boolean_node (known_ge (off0, off1), type); })
     (if (cmp == GT_EXPR && (known_gt (off0, off1) || known_le (off0, off1)))
      { constant_boolean_node (known_gt (off0, off1), type); }))
    (if (equal == 0)
     (switch
      (if (cmp == EQ_EXPR)
       { constant_boolean_node (false, type); })
      (if (cmp == NE_EXPR)
       { constant_boolean_node (true, type); })))))))
and
(for minmax (min max)
     cmp (lt gt)
 (simplify
  (minmax (convert1?@2 addr@0) (convert2?@3 addr@1))
  (with
   {
     poly_int64 off0, off1;
     tree base0, base1;
     int equal = address_compare (cmp, TREE_TYPE (@2), @0, @1, base0, base1,
                                  off0, off1, GENERIC);
   }
   (if (equal == 1)
    (if (minmax == MIN_EXPR)
     (if (known_le (off0, off1))
      @2
      (if (known_gt (off0, off1))
       @3))
     (if (known_ge (off0, off1))
      @2
      (if (known_lt (off0, off1))
       @3)))))))
and address_compare is a fairly large routine and uses equal_address_to
which is another quite large routine and we'd need to redo big chunks
of that code in constexpr.c.
Not using match.pd and fold-const.cc at all during constexpr evaluation
(except perhaps for folding of builtins) seems like a nice ultimate goal
(we would only optimize what we are required to and nothing else, at least
in the strict modes), but I'm afraid it would take several years to implement.

Having another flag next to folding_initialize that would be used in
fold-const.c in the meantime looks fine to me, any suggestion on how to call
it?

	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++: Further address_compare fixes [PR89074]
  2022-01-18 16:40       ` Jakub Jelinek
@ 2022-01-18 16:56         ` Jason Merrill
  2022-02-03 15:52         ` [PATCH] c++, v2: " Jakub Jelinek
  1 sibling, 0 replies; 21+ messages in thread
From: Jason Merrill @ 2022-01-18 16:56 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Richard Biener

On 1/18/22 11:40, Jakub Jelinek wrote:
> On Tue, Jan 18, 2022 at 11:25:38AM -0500, Jason Merrill wrote:
>>> Can you please comment on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86369#c1
>>> then?
>>
>> Done.
> 
> Thanks.
> 
>> About the rest of the patch, I thought I had seen richi comment on IRC (but
>> can't find the comment now) that these cases where we could give a constant
>> answer but decide not to because of C++ rules should be handled in the front
>> end rather than generic code, which makes sense to me; that is, use
>> folding_initializer only for giving more constant results, not for giving
>> fewer constant results.  Maybe add another flag for limiting constant
>> results if we think it's significantly easier to recognize these cases in
>> fold?
> 
> I'm afraid avoiding the match.pd & fold-const code here would be a lot of
> work.
> The match.pd code looks like:
> (for cmp (simple_comparison)
>   (simplify
>    (cmp (convert1?@2 addr@0) (convert2? addr@1))
>    (with
>     {
>       poly_int64 off0, off1;
>       tree base0, base1;
>       int equal = address_compare (cmp, TREE_TYPE (@2), @0, @1, base0, base1,
>                                    off0, off1, GENERIC);
>     }
>     (if (equal == 1)
>      (switch
>       (if (cmp == EQ_EXPR && (known_eq (off0, off1) || known_ne (off0, off1)))
>        { constant_boolean_node (known_eq (off0, off1), type); })
>       (if (cmp == NE_EXPR && (known_eq (off0, off1) || known_ne (off0, off1)))
>        { constant_boolean_node (known_ne (off0, off1), type); })
>       (if (cmp == LT_EXPR && (known_lt (off0, off1) || known_ge (off0, off1)))
>        { constant_boolean_node (known_lt (off0, off1), type); })
>       (if (cmp == LE_EXPR && (known_le (off0, off1) || known_gt (off0, off1)))
>        { constant_boolean_node (known_le (off0, off1), type); })
>       (if (cmp == GE_EXPR && (known_ge (off0, off1) || known_lt (off0, off1)))
>        { constant_boolean_node (known_ge (off0, off1), type); })
>       (if (cmp == GT_EXPR && (known_gt (off0, off1) || known_le (off0, off1)))
>        { constant_boolean_node (known_gt (off0, off1), type); }))
>      (if (equal == 0)
>       (switch
>        (if (cmp == EQ_EXPR)
>         { constant_boolean_node (false, type); })
>        (if (cmp == NE_EXPR)
>         { constant_boolean_node (true, type); })))))))
> and
> (for minmax (min max)
>       cmp (lt gt)
>   (simplify
>    (minmax (convert1?@2 addr@0) (convert2?@3 addr@1))
>    (with
>     {
>       poly_int64 off0, off1;
>       tree base0, base1;
>       int equal = address_compare (cmp, TREE_TYPE (@2), @0, @1, base0, base1,
>                                    off0, off1, GENERIC);
>     }
>     (if (equal == 1)
>      (if (minmax == MIN_EXPR)
>       (if (known_le (off0, off1))
>        @2
>        (if (known_gt (off0, off1))
>         @3))
>       (if (known_ge (off0, off1))
>        @2
>        (if (known_lt (off0, off1))
>         @3)))))))
> and address_compare is a fairly large routine and uses equal_address_to
> which is another quite large routine and we'd need to redo big chunks
> of that code in constexpr.c.
> Not using match.pd and fold-const.cc at all during constexpr evaluation
> (except perhaps for folding of builtins) seems like a nice ultimate goal
> (we would only optimize what we are required to and nothing else, at least
> in the strict modes), but I'm afraid it would take several years to implement.
> 
> Having another flag next to folding_initialize that would be used in
> fold-const.c in the meantime looks fine to me, any suggestion on how to call
> it?

flag_unspecified_compare?

Jason


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] c++, v2: Further address_compare fixes [PR89074]
  2022-01-18 16:40       ` Jakub Jelinek
  2022-01-18 16:56         ` Jason Merrill
@ 2022-02-03 15:52         ` Jakub Jelinek
  2022-02-03 20:07           ` Jason Merrill
  1 sibling, 1 reply; 21+ messages in thread
From: Jakub Jelinek @ 2022-02-03 15:52 UTC (permalink / raw)
  To: Jason Merrill, Richard Biener; +Cc: gcc-patches

On Tue, Jan 18, 2022 at 05:40:48PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > About the rest of the patch, I thought I had seen richi comment on IRC (but
> > can't find the comment now) that these cases where we could give a constant
> > answer but decide not to because of C++ rules should be handled in the front
> > end rather than generic code, which makes sense to me; that is, use
> > folding_initializer only for giving more constant results, not for giving
> > fewer constant results.  Maybe add another flag for limiting constant
> > results if we think it's significantly easier to recognize these cases in
> > fold?
> 
> I'm afraid avoiding the match.pd & fold-const code here would be a lot of
> work.

Sorry for dropping the ball on this.

Here is a patch that introduces folding_cxx_constexpr flag and documents
that folding_initializer should be used for enabling more folding
optimizations, while folding_cxx_constexpr should be used to prevent
optimizations C++ doesn't allow in the constant expression evaluation.

Ok for trunk if it passes bootstrap/regtest?

2022-02-03  Jakub Jelinek  <jakub@redhat.com>

	PR c++/89074
	PR c++/104033
	* fold-const.h (folding_initializer): Adjust comment.
	(folding_cxx_constexpr): Declare.
	* fold-const.cc (folding_initializer): Adjust comment.
	(folding_cxx_constexpr): New variable.
	(address_compare): Restrict the decl vs. STRING_CST
	or vice versa or STRING_CST vs. STRING_CST or
	is_global_var != is_global_var optimizations to !folding_cxx_constexpr.
	Punt for FUNCTION_DECLs with non-zero offsets.  If folding_initializer,
	assume non-aliased functions have non-zero size and have different
	addresses.  For folding_cxx_constexpr, punt on comparisons of start
	of some object and end of another one, regardless whether it is a decl
	or string literal.  Also punt for folding_cxx_constexpr on
	STRING_CST vs. STRING_CST comparisons if the two literals could be
	overlapping.

	* constexpr.cc (cxx_eval_binary_expression): Temporarily set
	folding_cxx_constexpr.

	* g++.dg/cpp1y/constexpr-89074-3.C: New test.

--- gcc/fold-const.h.jj	2022-02-01 20:10:51.235856007 +0100
+++ gcc/fold-const.h	2022-02-03 15:02:02.700228631 +0100
@@ -20,9 +20,16 @@ along with GCC; see the file COPYING3.
 #ifndef GCC_FOLD_CONST_H
 #define GCC_FOLD_CONST_H
 
-/* Non-zero if we are folding constants inside an initializer; zero
-   otherwise.  */
+/* Nonzero if we are folding constants inside an initializer or a C++
+   manifestly-constant-evaluated context; zero otherwise.
+   Should be used when folding in initializer enables additional
+   optimizations.  */
 extern int folding_initializer;
+/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero
+   otherwise.
+   Should be used when certain constructs shouldn't be optimized
+   during folding in that context.  */
+extern bool folding_cxx_constexpr;
 
 /* Convert between trees and native memory representation.  */
 extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
--- gcc/fold-const.cc.jj	2022-02-03 14:31:32.243129408 +0100
+++ gcc/fold-const.cc	2022-02-03 16:25:27.541467112 +0100
@@ -86,9 +86,17 @@ along with GCC; see the file COPYING3.
 #include "gimple-range.h"
 
 /* Nonzero if we are folding constants inside an initializer or a C++
-   manifestly-constant-evaluated context; zero otherwise.  */
+   manifestly-constant-evaluated context; zero otherwise.
+   Should be used when folding in initializer enables additional
+   optimizations.  */
 int folding_initializer = 0;
 
+/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero
+   otherwise.
+   Should be used when certain constructs shouldn't be optimized
+   during folding in that context.  */
+bool folding_cxx_constexpr = false;
+
 /* The following constants represent a bit based encoding of GCC's
    comparison operators.  This encoding simplifies transformations
    on relational comparison operators, such as AND and OR.  */
@@ -16628,41 +16636,91 @@ address_compare (tree_code code, tree ty
   HOST_WIDE_INT ioff0 = -1, ioff1 = -1;
   off0.is_constant (&ioff0);
   off1.is_constant (&ioff1);
-  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
-       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
-       || (TREE_CODE (base0) == STRING_CST
-	   && TREE_CODE (base1) == STRING_CST
-	   && ioff0 >= 0 && ioff1 >= 0
-	   && ioff0 < TREE_STRING_LENGTH (base0)
-	   && ioff1 < TREE_STRING_LENGTH (base1)
-	  /* This is a too conservative test that the STRING_CSTs
-	     will not end up being string-merged.  */
-	   && strncmp (TREE_STRING_POINTER (base0) + ioff0,
-		       TREE_STRING_POINTER (base1) + ioff1,
-		       MIN (TREE_STRING_LENGTH (base0) - ioff0,
-			    TREE_STRING_LENGTH (base1) - ioff1)) != 0))
+  if (!folding_cxx_constexpr
+      && ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
+	  || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
+	  || (TREE_CODE (base0) == STRING_CST
+	      && TREE_CODE (base1) == STRING_CST
+	      && ioff0 >= 0 && ioff1 >= 0
+	      && ioff0 < TREE_STRING_LENGTH (base0)
+	      && ioff1 < TREE_STRING_LENGTH (base1)
+	      /* This is a too conservative test that the STRING_CSTs
+		 will not end up being string-merged.  */
+	      && strncmp (TREE_STRING_POINTER (base0) + ioff0,
+			  TREE_STRING_POINTER (base1) + ioff1,
+			  MIN (TREE_STRING_LENGTH (base0) - ioff0,
+			       TREE_STRING_LENGTH (base1) - ioff1)) != 0)))
     ;
-  else if (!DECL_P (base0) || !DECL_P (base1))
+  /* Punt on non-zero offsets from functions.  */
+  else if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
+	   || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
+    return 2;
+  else if ((!DECL_P (base0)
+  	    && (!folding_cxx_constexpr || TREE_CODE (base0) != STRING_CST))
+	   || (!DECL_P (base1)
+	       && (!folding_cxx_constexpr || TREE_CODE (base1) != STRING_CST)))
     return 2;
   /* If this is a pointer comparison, ignore for now even
      valid equalities where one pointer is the offset zero
      of one object and the other to one past end of another one.  */
-  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
+  else if (!folding_cxx_constexpr && !INTEGRAL_TYPE_P (type))
     ;
   /* Assume that automatic variables can't be adjacent to global
      variables.  */
-  else if (is_global_var (base0) != is_global_var (base1))
+  else if (!folding_cxx_constexpr
+	   && is_global_var (base0) != is_global_var (base1))
+    ;
+  /* For initializers, assume addresses of different functions are
+     different.  */
+  else if (folding_initializer
+	   && TREE_CODE (base0) == FUNCTION_DECL
+	   && TREE_CODE (base1) == FUNCTION_DECL)
     ;
   else
     {
-      tree sz0 = DECL_SIZE_UNIT (base0);
-      tree sz1 = DECL_SIZE_UNIT (base1);
-      /* If sizes are unknown, e.g. VLA or not representable, punt.  */
-      if (!tree_fits_poly_int64_p (sz0) || !tree_fits_poly_int64_p (sz1))
-	return 2;
+      poly_int64 size0, size1;
+      if (TREE_CODE (base0) == STRING_CST)
+	{
+	  if (ioff0 < 0
+	      || ioff0 > TREE_STRING_LENGTH (base0))
+	    return 2;
+	  size0 = TREE_STRING_LENGTH (base0);
+	}
+      /* For initializers, assume function decls don't overlap and have
+	 non-empty size.  */
+      else if (folding_initializer && TREE_CODE (base0) == FUNCTION_DECL)
+	size0 = 1;
+      else
+	{
+	  tree sz0 = DECL_SIZE_UNIT (base0);
+	  /* If sizes are unknown, e.g. VLA or not representable, punt.  */
+	  if (!tree_fits_poly_int64_p (sz0))
+	    return 2;
+
+	  size0 = tree_to_poly_int64 (sz0);
+	}
+
+      if (TREE_CODE (base1) == STRING_CST)
+	{
+	  if (ioff1 < 0
+	      || ioff1 > TREE_STRING_LENGTH (base1))
+	    return 2;
+	  size1 = TREE_STRING_LENGTH (base1);
+	}
+      /* For initializers, assume function decls don't overlap and have
+	 non-empty size.  */
+      else if (folding_initializer && TREE_CODE (base1) == FUNCTION_DECL)
+	size1 = 1;
+      else
+	{
+	  tree sz1 = DECL_SIZE_UNIT (base1);
+	  /* If sizes are unknown, e.g. VLA or not representable, punt.  */
+	  if (!tree_fits_poly_int64_p (sz1))
+	    return 2;
+
+	  size1 = tree_to_poly_int64 (sz1);
+	}
 
-      poly_int64 size0 = tree_to_poly_int64 (sz0);
-      poly_int64 size1 = tree_to_poly_int64 (sz1);
       /* If one offset is pointing (or could be) to the beginning of one
 	 object and the other is pointing to one past the last byte of the
 	 other object, punt.  */
@@ -16678,6 +16736,27 @@ address_compare (tree_code code, tree ty
 	  && (known_ne (off0, 0)
 	      || (known_ne (size0, 0) && known_ne (size1, 0))))
 	equal = 0;
+      if (equal == 0
+	  && TREE_CODE (base0) == STRING_CST
+	  && TREE_CODE (base1) == STRING_CST)
+	{
+	  /* If the bytes in the string literals starting at the pointers
+	     differ, the pointers need to be different.  */
+	  if (memcmp (TREE_STRING_POINTER (base0) + ioff0,
+		      TREE_STRING_POINTER (base1) + ioff1,
+		      MIN (TREE_STRING_LENGTH (base0) - ioff0,
+			   TREE_STRING_LENGTH (base1) - ioff1)) == 0)
+	    {
+	      HOST_WIDE_INT ioffmin = MIN (ioff0, ioff1);
+	      if (memcmp (TREE_STRING_POINTER (base0) + ioff0 - ioffmin,
+			  TREE_STRING_POINTER (base1) + ioff1 - ioffmin,
+			  ioffmin) == 0)
+		/* If even the bytes in the string literal before the
+		   pointers are the same, the string literals could be
+		   tail merged.  */
+		return 2;
+	    }
+	}
      }
   return equal;
 }
--- gcc/cp/constexpr.cc.jj	2022-01-26 19:40:22.957796808 +0100
+++ gcc/cp/constexpr.cc	2022-02-03 15:04:00.760558570 +0100
@@ -3413,7 +3413,10 @@ cxx_eval_binary_expression (const conste
       if (ctx->manifestly_const_eval
 	  && (flag_constexpr_fp_except
 	      || TREE_CODE (type) != REAL_TYPE))
-	r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
+	{
+	  auto ofcc = make_temp_override (folding_cxx_constexpr, true);
+	  r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
+	}
       else
 	r = fold_binary_loc (loc, code, type, lhs, rhs);
     }
--- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C.jj	2022-02-03 14:58:53.734901694 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C	2022-02-03 14:58:53.734901694 +0100
@@ -0,0 +1,132 @@
+// PR c++/89074
+// { dg-do compile { target c++14 } }
+
+int fn1 (void) { return 0; }
+int fn2 (void) { return 1; }
+
+constexpr bool
+f1 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  if (&a[1] == "foo")
+    return false;
+
+  if (&a[1] == &"foo"[4])
+    return false;
+
+  if (&"foo"[1] == &a[0])
+    return false;
+
+  if (&"foo"[3] == &a[4])
+    return false;
+
+  if (&a[0] == "foo")
+    return false;
+
+  // Pointer to start of one object (var) and end of another one (literal)
+  if (&a[0] == &"foo"[4])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f2 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  // Pointer to end of one object (var) and start of another one (literal)
+  if (&a[4] == "foo")		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+char v[] = { 1, 2, 3, 4 };
+
+constexpr bool
+f3 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  if (&a[1] == &v[1])
+    return false;
+
+  if (&a[0] == &v[3])
+    return false;
+
+  if (&a[2] == &v[4])
+    return false;
+
+  // Pointer to start of one object (automatic var) and end of another one (non-automagic var)
+  if (&a[0] == &v[4])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f4 ()
+{
+  char a[] = { 1, 2, 3, 4, 5 };
+
+  // Pointer to end of one object (automatic var) and start of another one (non-automagic var)
+  if (&a[5] == &v[0])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f5 ()
+{
+  if (fn1 != fn1)
+    return false;
+
+  if (fn1 == fn2)
+    return false;
+
+  if (&"abcde"[0] == &"edcba"[1])
+    return false;
+
+  if (&"abcde"[1] == &"edcba"[6])
+    return false;
+
+  // Pointer to start of one object (literal) and end of another one (literal)
+  if (&"abcde"[0] == &"edcba"[6])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f6 ()
+{
+  // Pointer to start of one object (literal) and end of another one (literal)
+  if (&"abcde"[6] == &"edcba"[0])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f7 ()
+{
+  if (&"abcde"[3] == &"fabcde"[3])
+    return false;
+
+  // These could be suffix merged, with &"abcde"[0] == &"fabcde"[1].
+  if (&"abcde"[3] == &"fabcde"[4])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool a = f1 ();
+constexpr bool b = f2 ();
+constexpr bool c = f3 ();
+constexpr bool d = f4 ();
+constexpr bool e = f5 ();
+constexpr bool f = f6 ();
+constexpr bool g = f7 ();


	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++, v2: Further address_compare fixes [PR89074]
  2022-02-03 15:52         ` [PATCH] c++, v2: " Jakub Jelinek
@ 2022-02-03 20:07           ` Jason Merrill
  2022-02-03 20:33             ` Jakub Jelinek
  0 siblings, 1 reply; 21+ messages in thread
From: Jason Merrill @ 2022-02-03 20:07 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Biener; +Cc: gcc-patches

On 2/3/22 10:52, Jakub Jelinek wrote:
> On Tue, Jan 18, 2022 at 05:40:48PM +0100, Jakub Jelinek via Gcc-patches wrote:
>>> About the rest of the patch, I thought I had seen richi comment on IRC (but
>>> can't find the comment now) that these cases where we could give a constant
>>> answer but decide not to because of C++ rules should be handled in the front
>>> end rather than generic code, which makes sense to me; that is, use
>>> folding_initializer only for giving more constant results, not for giving
>>> fewer constant results.  Maybe add another flag for limiting constant
>>> results if we think it's significantly easier to recognize these cases in
>>> fold?
>>
>> I'm afraid avoiding the match.pd & fold-const code here would be a lot of
>> work.
> 
> Sorry for dropping the ball on this.
> 
> Here is a patch that introduces folding_cxx_constexpr flag and documents
> that folding_initializer should be used for enabling more folding
> optimizations, while folding_cxx_constexpr should be used to prevent
> optimizations C++ doesn't allow in the constant expression evaluation.
> 
> Ok for trunk if it passes bootstrap/regtest?
> 
> 2022-02-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89074
> 	PR c++/104033
> 	* fold-const.h (folding_initializer): Adjust comment.
> 	(folding_cxx_constexpr): Declare.
> 	* fold-const.cc (folding_initializer): Adjust comment.
> 	(folding_cxx_constexpr): New variable.
> 	(address_compare): Restrict the decl vs. STRING_CST
> 	or vice versa or STRING_CST vs. STRING_CST or
> 	is_global_var != is_global_var optimizations to !folding_cxx_constexpr.
> 	Punt for FUNCTION_DECLs with non-zero offsets.  If folding_initializer,
> 	assume non-aliased functions have non-zero size and have different
> 	addresses.  For folding_cxx_constexpr, punt on comparisons of start
> 	of some object and end of another one, regardless whether it is a decl
> 	or string literal.  Also punt for folding_cxx_constexpr on
> 	STRING_CST vs. STRING_CST comparisons if the two literals could be
> 	overlapping.
> 
> 	* constexpr.cc (cxx_eval_binary_expression): Temporarily set
> 	folding_cxx_constexpr.
> 
> 	* g++.dg/cpp1y/constexpr-89074-3.C: New test.
> 
> --- gcc/fold-const.h.jj	2022-02-01 20:10:51.235856007 +0100
> +++ gcc/fold-const.h	2022-02-03 15:02:02.700228631 +0100
> @@ -20,9 +20,16 @@ along with GCC; see the file COPYING3.
>   #ifndef GCC_FOLD_CONST_H
>   #define GCC_FOLD_CONST_H
>   
> -/* Non-zero if we are folding constants inside an initializer; zero
> -   otherwise.  */
> +/* Nonzero if we are folding constants inside an initializer or a C++
> +   manifestly-constant-evaluated context; zero otherwise.
> +   Should be used when folding in initializer enables additional
> +   optimizations.  */
>   extern int folding_initializer;
> +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero

"Nonzero if"

> +   otherwise.
> +   Should be used when certain constructs shouldn't be optimized
> +   during folding in that context.  */
> +extern bool folding_cxx_constexpr;
>   
>   /* Convert between trees and native memory representation.  */
>   extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
> --- gcc/fold-const.cc.jj	2022-02-03 14:31:32.243129408 +0100
> +++ gcc/fold-const.cc	2022-02-03 16:25:27.541467112 +0100
> @@ -86,9 +86,17 @@ along with GCC; see the file COPYING3.
>   #include "gimple-range.h"
>   
>   /* Nonzero if we are folding constants inside an initializer or a C++
> -   manifestly-constant-evaluated context; zero otherwise.  */
> +   manifestly-constant-evaluated context; zero otherwise.
> +   Should be used when folding in initializer enables additional
> +   optimizations.  */
>   int folding_initializer = 0;
>   
> +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero

"Nonzero if"

> +   otherwise.
> +   Should be used when certain constructs shouldn't be optimized
> +   during folding in that context.  */
> +bool folding_cxx_constexpr = false;
> +
>   /* The following constants represent a bit based encoding of GCC's
>      comparison operators.  This encoding simplifies transformations
>      on relational comparison operators, such as AND and OR.  */
> @@ -16628,41 +16636,91 @@ address_compare (tree_code code, tree ty
>     HOST_WIDE_INT ioff0 = -1, ioff1 = -1;
>     off0.is_constant (&ioff0);
>     off1.is_constant (&ioff1);
> -  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> -       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> -       || (TREE_CODE (base0) == STRING_CST
> -	   && TREE_CODE (base1) == STRING_CST
> -	   && ioff0 >= 0 && ioff1 >= 0
> -	   && ioff0 < TREE_STRING_LENGTH (base0)
> -	   && ioff1 < TREE_STRING_LENGTH (base1)
> -	  /* This is a too conservative test that the STRING_CSTs
> -	     will not end up being string-merged.  */
> -	   && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> -		       TREE_STRING_POINTER (base1) + ioff1,
> -		       MIN (TREE_STRING_LENGTH (base0) - ioff0,
> -			    TREE_STRING_LENGTH (base1) - ioff1)) != 0))
> +  if (!folding_cxx_constexpr
> +      && ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> +	  || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> +	  || (TREE_CODE (base0) == STRING_CST
> +	      && TREE_CODE (base1) == STRING_CST
> +	      && ioff0 >= 0 && ioff1 >= 0
> +	      && ioff0 < TREE_STRING_LENGTH (base0)
> +	      && ioff1 < TREE_STRING_LENGTH (base1)
> +	      /* This is a too conservative test that the STRING_CSTs
> +		 will not end up being string-merged.  */
> +	      && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> +			  TREE_STRING_POINTER (base1) + ioff1,
> +			  MIN (TREE_STRING_LENGTH (base0) - ioff0,
> +			       TREE_STRING_LENGTH (base1) - ioff1)) != 0)))
>       ;
> -  else if (!DECL_P (base0) || !DECL_P (base1))
> +  /* Punt on non-zero offsets from functions.  */
> +  else if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
> +	   || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
> +    return 2;
> +  else if ((!DECL_P (base0)
> +  	    && (!folding_cxx_constexpr || TREE_CODE (base0) != STRING_CST))
> +	   || (!DECL_P (base1)
> +	       && (!folding_cxx_constexpr || TREE_CODE (base1) != STRING_CST)))

I think it would be clearer to leave the !DECL_P case alone and add

/* In C++ it is unspecified, and so non-constant, whether two
    equivalent strings have the same address.  */
else if (folding_cxx_constexpr
          && (TREE_CODE (base0) == STRING_CST
              || TREE_CODE (base1) == STRING_CST)

OK with that change and the above typo fixes.

>       return 2;
>     /* If this is a pointer comparison, ignore for now even
>        valid equalities where one pointer is the offset zero
>        of one object and the other to one past end of another one.  */
> -  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
> +  else if (!folding_cxx_constexpr && !INTEGRAL_TYPE_P (type))
>       ;
>     /* Assume that automatic variables can't be adjacent to global
>        variables.  */
> -  else if (is_global_var (base0) != is_global_var (base1))
> +  else if (!folding_cxx_constexpr
> +	   && is_global_var (base0) != is_global_var (base1))
> +    ;
> +  /* For initializers, assume addresses of different functions are
> +     different.  */
> +  else if (folding_initializer
> +	   && TREE_CODE (base0) == FUNCTION_DECL
> +	   && TREE_CODE (base1) == FUNCTION_DECL)
>       ;
>     else
>       {
> -      tree sz0 = DECL_SIZE_UNIT (base0);
> -      tree sz1 = DECL_SIZE_UNIT (base1);
> -      /* If sizes are unknown, e.g. VLA or not representable, punt.  */
> -      if (!tree_fits_poly_int64_p (sz0) || !tree_fits_poly_int64_p (sz1))
> -	return 2;
> +      poly_int64 size0, size1;
> +      if (TREE_CODE (base0) == STRING_CST)
> +	{
> +	  if (ioff0 < 0
> +	      || ioff0 > TREE_STRING_LENGTH (base0))
> +	    return 2;
> +	  size0 = TREE_STRING_LENGTH (base0);
> +	}
> +      /* For initializers, assume function decls don't overlap and have
> +	 non-empty size.  */
> +      else if (folding_initializer && TREE_CODE (base0) == FUNCTION_DECL)
> +	size0 = 1;
> +      else
> +	{
> +	  tree sz0 = DECL_SIZE_UNIT (base0);
> +	  /* If sizes are unknown, e.g. VLA or not representable, punt.  */
> +	  if (!tree_fits_poly_int64_p (sz0))
> +	    return 2;
> +
> +	  size0 = tree_to_poly_int64 (sz0);
> +	}
> +
> +      if (TREE_CODE (base1) == STRING_CST)
> +	{
> +	  if (ioff1 < 0
> +	      || ioff1 > TREE_STRING_LENGTH (base1))
> +	    return 2;
> +	  size1 = TREE_STRING_LENGTH (base1);
> +	}
> +      /* For initializers, assume function decls don't overlap and have
> +	 non-empty size.  */
> +      else if (folding_initializer && TREE_CODE (base1) == FUNCTION_DECL)
> +	size1 = 1;
> +      else
> +	{
> +	  tree sz1 = DECL_SIZE_UNIT (base1);
> +	  /* If sizes are unknown, e.g. VLA or not representable, punt.  */
> +	  if (!tree_fits_poly_int64_p (sz1))
> +	    return 2;
> +
> +	  size1 = tree_to_poly_int64 (sz1);
> +	}
>   
> -      poly_int64 size0 = tree_to_poly_int64 (sz0);
> -      poly_int64 size1 = tree_to_poly_int64 (sz1);
>         /* If one offset is pointing (or could be) to the beginning of one
>   	 object and the other is pointing to one past the last byte of the
>   	 other object, punt.  */
> @@ -16678,6 +16736,27 @@ address_compare (tree_code code, tree ty
>   	  && (known_ne (off0, 0)
>   	      || (known_ne (size0, 0) && known_ne (size1, 0))))
>   	equal = 0;
> +      if (equal == 0
> +	  && TREE_CODE (base0) == STRING_CST
> +	  && TREE_CODE (base1) == STRING_CST)
> +	{
> +	  /* If the bytes in the string literals starting at the pointers
> +	     differ, the pointers need to be different.  */
> +	  if (memcmp (TREE_STRING_POINTER (base0) + ioff0,
> +		      TREE_STRING_POINTER (base1) + ioff1,
> +		      MIN (TREE_STRING_LENGTH (base0) - ioff0,
> +			   TREE_STRING_LENGTH (base1) - ioff1)) == 0)
> +	    {
> +	      HOST_WIDE_INT ioffmin = MIN (ioff0, ioff1);
> +	      if (memcmp (TREE_STRING_POINTER (base0) + ioff0 - ioffmin,
> +			  TREE_STRING_POINTER (base1) + ioff1 - ioffmin,
> +			  ioffmin) == 0)
> +		/* If even the bytes in the string literal before the
> +		   pointers are the same, the string literals could be
> +		   tail merged.  */
> +		return 2;
> +	    }
> +	}
>        }
>     return equal;
>   }
> --- gcc/cp/constexpr.cc.jj	2022-01-26 19:40:22.957796808 +0100
> +++ gcc/cp/constexpr.cc	2022-02-03 15:04:00.760558570 +0100
> @@ -3413,7 +3413,10 @@ cxx_eval_binary_expression (const conste
>         if (ctx->manifestly_const_eval
>   	  && (flag_constexpr_fp_except
>   	      || TREE_CODE (type) != REAL_TYPE))
> -	r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
> +	{
> +	  auto ofcc = make_temp_override (folding_cxx_constexpr, true);
> +	  r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
> +	}
>         else
>   	r = fold_binary_loc (loc, code, type, lhs, rhs);
>       }
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C.jj	2022-02-03 14:58:53.734901694 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C	2022-02-03 14:58:53.734901694 +0100
> @@ -0,0 +1,132 @@
> +// PR c++/89074
> +// { dg-do compile { target c++14 } }
> +
> +int fn1 (void) { return 0; }
> +int fn2 (void) { return 1; }
> +
> +constexpr bool
> +f1 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  if (&a[1] == "foo")
> +    return false;
> +
> +  if (&a[1] == &"foo"[4])
> +    return false;
> +
> +  if (&"foo"[1] == &a[0])
> +    return false;
> +
> +  if (&"foo"[3] == &a[4])
> +    return false;
> +
> +  if (&a[0] == "foo")
> +    return false;
> +
> +  // Pointer to start of one object (var) and end of another one (literal)
> +  if (&a[0] == &"foo"[4])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f2 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  // Pointer to end of one object (var) and start of another one (literal)
> +  if (&a[4] == "foo")		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +char v[] = { 1, 2, 3, 4 };
> +
> +constexpr bool
> +f3 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  if (&a[1] == &v[1])
> +    return false;
> +
> +  if (&a[0] == &v[3])
> +    return false;
> +
> +  if (&a[2] == &v[4])
> +    return false;
> +
> +  // Pointer to start of one object (automatic var) and end of another one (non-automagic var)
> +  if (&a[0] == &v[4])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f4 ()
> +{
> +  char a[] = { 1, 2, 3, 4, 5 };
> +
> +  // Pointer to end of one object (automatic var) and start of another one (non-automagic var)
> +  if (&a[5] == &v[0])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f5 ()
> +{
> +  if (fn1 != fn1)
> +    return false;
> +
> +  if (fn1 == fn2)
> +    return false;
> +
> +  if (&"abcde"[0] == &"edcba"[1])
> +    return false;
> +
> +  if (&"abcde"[1] == &"edcba"[6])
> +    return false;
> +
> +  // Pointer to start of one object (literal) and end of another one (literal)
> +  if (&"abcde"[0] == &"edcba"[6])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f6 ()
> +{
> +  // Pointer to start of one object (literal) and end of another one (literal)
> +  if (&"abcde"[6] == &"edcba"[0])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f7 ()
> +{
> +  if (&"abcde"[3] == &"fabcde"[3])
> +    return false;
> +
> +  // These could be suffix merged, with &"abcde"[0] == &"fabcde"[1].
> +  if (&"abcde"[3] == &"fabcde"[4])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool a = f1 ();
> +constexpr bool b = f2 ();
> +constexpr bool c = f3 ();
> +constexpr bool d = f4 ();
> +constexpr bool e = f5 ();
> +constexpr bool f = f6 ();
> +constexpr bool g = f7 ();
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++, v2: Further address_compare fixes [PR89074]
  2022-02-03 20:07           ` Jason Merrill
@ 2022-02-03 20:33             ` Jakub Jelinek
  2022-02-03 21:04               ` Jason Merrill
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Jelinek @ 2022-02-03 20:33 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Richard Biener, gcc-patches

On Thu, Feb 03, 2022 at 03:07:03PM -0500, Jason Merrill wrote:
> > --- gcc/fold-const.h.jj	2022-02-01 20:10:51.235856007 +0100
> > +++ gcc/fold-const.h	2022-02-03 15:02:02.700228631 +0100
> > -/* Non-zero if we are folding constants inside an initializer; zero
> > -   otherwise.  */
> > +/* Nonzero if we are folding constants inside an initializer or a C++
> > +   manifestly-constant-evaluated context; zero otherwise.
> > +   Should be used when folding in initializer enables additional
> > +   optimizations.  */
> >   extern int folding_initializer;
> > +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero
> 
> "Nonzero if"

Oops, thanks for catching it.

> > +  if (!folding_cxx_constexpr
> > +      && ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> > +	  || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> > +	  || (TREE_CODE (base0) == STRING_CST
> > +	      && TREE_CODE (base1) == STRING_CST
> > +	      && ioff0 >= 0 && ioff1 >= 0
> > +	      && ioff0 < TREE_STRING_LENGTH (base0)
> > +	      && ioff1 < TREE_STRING_LENGTH (base1)
> > +	      /* This is a too conservative test that the STRING_CSTs
> > +		 will not end up being string-merged.  */
> > +	      && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> > +			  TREE_STRING_POINTER (base1) + ioff1,
> > +			  MIN (TREE_STRING_LENGTH (base0) - ioff0,
> > +			       TREE_STRING_LENGTH (base1) - ioff1)) != 0)))
> >       ;
> > -  else if (!DECL_P (base0) || !DECL_P (base1))
> > +  /* Punt on non-zero offsets from functions.  */
> > +  else if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
> > +	   || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
> > +    return 2;
> > +  else if ((!DECL_P (base0)
> > +  	    && (!folding_cxx_constexpr || TREE_CODE (base0) != STRING_CST))
> > +	   || (!DECL_P (base1)
> > +	       && (!folding_cxx_constexpr || TREE_CODE (base1) != STRING_CST)))
> 
> I think it would be clearer to leave the !DECL_P case alone and add
> 
> /* In C++ it is unspecified, and so non-constant, whether two
>    equivalent strings have the same address.  */
> else if (folding_cxx_constexpr
>          && (TREE_CODE (base0) == STRING_CST
>              || TREE_CODE (base1) == STRING_CST)

The point was to let the first if handle for
!folding_cxx_constexpr the cases with STRING_CST
as one or both operands and if that falls through, return 2.

But I guess it could be rewritten as:
  if (!DECL_P (base0) && TREE_CODE (base0) != STRING_CST)
    return 2;
  if (!DECL_P (base1) && TREE_CODE (base1) != STRING_CST)
    return 2;
  /* Punt on non-zero offsets from functions.  */
  if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
      || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
    return 2;
  if (!folding_cxx_constexpr
      && (TREE_CODE (base0) == STRING_CST
	  || TREE_CODE (base1) == STRING_CST))
    {
      if (DECL_P (base0)
	  || DECL_P (base1)
	  || (TREE_CODE (base0) == TREE_CODE (base1)
	      && ioff0 >= 0 && ioff1 >= 0
	      && ioff0 < TREE_STRING_LENGTH (base0)
	      && ioff1 < TREE_STRING_LENGTH (base1)
	      /* This is a too conservative test that the STRING_CSTs
		 will not end up being string-merged.  */
	      && strncmp (TREE_STRING_POINTER (base0) + ioff0,
			  TREE_STRING_POINTER (base1) + ioff1,
			  MIN (TREE_STRING_LENGTH (base0) - ioff0,
			       TREE_STRING_LENGTH (base1) - ioff1)) != 0))
	;
      else
	return 2;
    }
  else
    ...

	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++, v2: Further address_compare fixes [PR89074]
  2022-02-03 20:33             ` Jakub Jelinek
@ 2022-02-03 21:04               ` Jason Merrill
  2022-02-03 21:18                 ` Jakub Jelinek
  0 siblings, 1 reply; 21+ messages in thread
From: Jason Merrill @ 2022-02-03 21:04 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches

On 2/3/22 15:33, Jakub Jelinek wrote:
> On Thu, Feb 03, 2022 at 03:07:03PM -0500, Jason Merrill wrote:
>>> --- gcc/fold-const.h.jj	2022-02-01 20:10:51.235856007 +0100
>>> +++ gcc/fold-const.h	2022-02-03 15:02:02.700228631 +0100
>>> -/* Non-zero if we are folding constants inside an initializer; zero
>>> -   otherwise.  */
>>> +/* Nonzero if we are folding constants inside an initializer or a C++
>>> +   manifestly-constant-evaluated context; zero otherwise.
>>> +   Should be used when folding in initializer enables additional
>>> +   optimizations.  */
>>>    extern int folding_initializer;
>>> +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero
>>
>> "Nonzero if"
> 
> Oops, thanks for catching it.
> 
>>> +  if (!folding_cxx_constexpr
>>> +      && ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
>>> +	  || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
>>> +	  || (TREE_CODE (base0) == STRING_CST
>>> +	      && TREE_CODE (base1) == STRING_CST
>>> +	      && ioff0 >= 0 && ioff1 >= 0
>>> +	      && ioff0 < TREE_STRING_LENGTH (base0)
>>> +	      && ioff1 < TREE_STRING_LENGTH (base1)
>>> +	      /* This is a too conservative test that the STRING_CSTs
>>> +		 will not end up being string-merged.  */
>>> +	      && strncmp (TREE_STRING_POINTER (base0) + ioff0,
>>> +			  TREE_STRING_POINTER (base1) + ioff1,
>>> +			  MIN (TREE_STRING_LENGTH (base0) - ioff0,
>>> +			       TREE_STRING_LENGTH (base1) - ioff1)) != 0)))
>>>        ;
>>> -  else if (!DECL_P (base0) || !DECL_P (base1))
>>> +  /* Punt on non-zero offsets from functions.  */
>>> +  else if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
>>> +	   || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
>>> +    return 2;
>>> +  else if ((!DECL_P (base0)
>>> +  	    && (!folding_cxx_constexpr || TREE_CODE (base0) != STRING_CST))
>>> +	   || (!DECL_P (base1)
>>> +	       && (!folding_cxx_constexpr || TREE_CODE (base1) != STRING_CST)))
>>
>> I think it would be clearer to leave the !DECL_P case alone and add
>>
>> /* In C++ it is unspecified, and so non-constant, whether two
>>     equivalent strings have the same address.  */
>> else if (folding_cxx_constexpr
>>           && (TREE_CODE (base0) == STRING_CST
>>               || TREE_CODE (base1) == STRING_CST)
> 
> The point was to let the first if handle for
> !folding_cxx_constexpr the cases with STRING_CST
> as one or both operands and if that falls through, return 2.

Ah, I see.  And then for folding_cxx_constexpr you have your new code 
toward the bottom of the function that can say they're unequal in some 
cases.  Can you combine the STRING_CST handling for both values of 
folding_cxx_constexpr instead of having them so far apart?

Jason


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++, v2: Further address_compare fixes [PR89074]
  2022-02-03 21:04               ` Jason Merrill
@ 2022-02-03 21:18                 ` Jakub Jelinek
  2022-02-03 21:34                   ` Jason Merrill
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Jelinek @ 2022-02-03 21:18 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Richard Biener, gcc-patches

On Thu, Feb 03, 2022 at 04:04:57PM -0500, Jason Merrill wrote:
> > > I think it would be clearer to leave the !DECL_P case alone and add
> > > 
> > > /* In C++ it is unspecified, and so non-constant, whether two
> > >     equivalent strings have the same address.  */
> > > else if (folding_cxx_constexpr
> > >           && (TREE_CODE (base0) == STRING_CST
> > >               || TREE_CODE (base1) == STRING_CST)
> > 
> > The point was to let the first if handle for
> > !folding_cxx_constexpr the cases with STRING_CST
> > as one or both operands and if that falls through, return 2.
> 
> Ah, I see.  And then for folding_cxx_constexpr you have your new code toward
> the bottom of the function that can say they're unequal in some cases.  Can
> you combine the STRING_CST handling for both values of folding_cxx_constexpr
> instead of having them so far apart?

Not easily, because for the folding_cxx_constexpr case it primarily reuses
the code from the last else if - computing sizes of objects and checking
if one is at a start of one and another at the end of the other.

One further option would be to compute early flags like
  enum { OFF_POS_START, OFF_POS_MIDDLE, OFF_POS_END } pos0, pos1;
and then just use them or ignore them in the decisions later.

	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++, v2: Further address_compare fixes [PR89074]
  2022-02-03 21:18                 ` Jakub Jelinek
@ 2022-02-03 21:34                   ` Jason Merrill
  2022-02-04 13:41                     ` [PATCH] c++, v3: " Jakub Jelinek
  0 siblings, 1 reply; 21+ messages in thread
From: Jason Merrill @ 2022-02-03 21:34 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches

On 2/3/22 16:18, Jakub Jelinek wrote:
> On Thu, Feb 03, 2022 at 04:04:57PM -0500, Jason Merrill wrote:
>>>> I think it would be clearer to leave the !DECL_P case alone and add
>>>>
>>>> /* In C++ it is unspecified, and so non-constant, whether two
>>>>      equivalent strings have the same address.  */
>>>> else if (folding_cxx_constexpr
>>>>            && (TREE_CODE (base0) == STRING_CST
>>>>                || TREE_CODE (base1) == STRING_CST)
>>>
>>> The point was to let the first if handle for
>>> !folding_cxx_constexpr the cases with STRING_CST
>>> as one or both operands and if that falls through, return 2.
>>
>> Ah, I see.  And then for folding_cxx_constexpr you have your new code toward
>> the bottom of the function that can say they're unequal in some cases.  Can
>> you combine the STRING_CST handling for both values of folding_cxx_constexpr
>> instead of having them so far apart?
> 
> Not easily, because for the folding_cxx_constexpr case it primarily reuses
> the code from the last else if - computing sizes of objects and checking
> if one is at a start of one and another at the end of the other.

And the !folding_cxx_constexpr case shouldn't also use that code?

> One further option would be to compute early flags like
>    enum { OFF_POS_START, OFF_POS_MIDDLE, OFF_POS_END } pos0, pos1;
> and then just use them or ignore them in the decisions later.

If that helps to refactor a bit, sure.

Jason


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] c++, v3: Further address_compare fixes [PR89074]
  2022-02-03 21:34                   ` Jason Merrill
@ 2022-02-04 13:41                     ` Jakub Jelinek
  2022-02-04 21:42                       ` Jason Merrill
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Jelinek @ 2022-02-04 13:41 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Richard Biener, gcc-patches

On Thu, Feb 03, 2022 at 04:34:17PM -0500, Jason Merrill wrote:
> On 2/3/22 16:18, Jakub Jelinek wrote:
> > On Thu, Feb 03, 2022 at 04:04:57PM -0500, Jason Merrill wrote:
> > > > > I think it would be clearer to leave the !DECL_P case alone and add
> > > > > 
> > > > > /* In C++ it is unspecified, and so non-constant, whether two
> > > > >      equivalent strings have the same address.  */
> > > > > else if (folding_cxx_constexpr
> > > > >            && (TREE_CODE (base0) == STRING_CST
> > > > >                || TREE_CODE (base1) == STRING_CST)
> > > > 
> > > > The point was to let the first if handle for
> > > > !folding_cxx_constexpr the cases with STRING_CST
> > > > as one or both operands and if that falls through, return 2.
> > > 
> > > Ah, I see.  And then for folding_cxx_constexpr you have your new code toward
> > > the bottom of the function that can say they're unequal in some cases.  Can
> > > you combine the STRING_CST handling for both values of folding_cxx_constexpr
> > > instead of having them so far apart?
> > 
> > Not easily, because for the folding_cxx_constexpr case it primarily reuses
> > the code from the last else if - computing sizes of objects and checking
> > if one is at a start of one and another at the end of the other.
> 
> And the !folding_cxx_constexpr case shouldn't also use that code?
> 
> > One further option would be to compute early flags like
> >    enum { OFF_POS_START, OFF_POS_MIDDLE, OFF_POS_END } pos0, pos1;
> > and then just use them or ignore them in the decisions later.
> 
> If that helps to refactor a bit, sure.

Here it is, hopefully it makes the code more readable and understandable.

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

2022-02-04  Jakub Jelinek  <jakub@redhat.com>

	PR c++/89074
	PR c++/104033
	* fold-const.h (folding_initializer): Adjust comment.
	(folding_cxx_constexpr): Declare.
	* fold-const.cc (folding_initializer): Adjust comment.
	(folding_cxx_constexpr): New variable.
	(address_compare): Restrict the decl vs. STRING_CST
	or vice versa or STRING_CST vs. STRING_CST or
	is_global_var != is_global_var optimizations to !folding_cxx_constexpr.
	Punt for FUNCTION_DECLs with non-zero offsets.  If folding_initializer,
	assume non-aliased functions have non-zero size and have different
	addresses.  For folding_cxx_constexpr, punt on comparisons of start
	of some object and end of another one, regardless whether it is a decl
	or string literal.  Also punt for folding_cxx_constexpr on
	STRING_CST vs. STRING_CST comparisons if the two literals could be
	overlapping.

	* constexpr.cc (cxx_eval_binary_expression): Temporarily set
	folding_cxx_constexpr.

	* g++.dg/cpp1y/constexpr-89074-3.C: New test.

--- gcc/fold-const.h.jj	2022-02-01 20:10:51.235856007 +0100
+++ gcc/fold-const.h	2022-02-03 15:02:02.700228631 +0100
@@ -20,9 +20,16 @@ along with GCC; see the file COPYING3.
 #ifndef GCC_FOLD_CONST_H
 #define GCC_FOLD_CONST_H
 
-/* Non-zero if we are folding constants inside an initializer; zero
-   otherwise.  */
+/* Nonzero if we are folding constants inside an initializer or a C++
+   manifestly-constant-evaluated context; zero otherwise.
+   Should be used when folding in initializer enables additional
+   optimizations.  */
 extern int folding_initializer;
+/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero
+   otherwise.
+   Should be used when certain constructs shouldn't be optimized
+   during folding in that context.  */
+extern bool folding_cxx_constexpr;
 
 /* Convert between trees and native memory representation.  */
 extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
--- gcc/fold-const.cc.jj	2022-02-03 14:31:32.243129408 +0100
+++ gcc/fold-const.cc	2022-02-04 10:19:13.812784763 +0100
@@ -86,9 +86,17 @@ along with GCC; see the file COPYING3.
 #include "gimple-range.h"
 
 /* Nonzero if we are folding constants inside an initializer or a C++
-   manifestly-constant-evaluated context; zero otherwise.  */
+   manifestly-constant-evaluated context; zero otherwise.
+   Should be used when folding in initializer enables additional
+   optimizations.  */
 int folding_initializer = 0;
 
+/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero
+   otherwise.
+   Should be used when certain constructs shouldn't be optimized
+   during folding in that context.  */
+bool folding_cxx_constexpr = false;
+
 /* The following constants represent a bit based encoding of GCC's
    comparison operators.  This encoding simplifies transformations
    on relational comparison operators, such as AND and OR.  */
@@ -16628,41 +16636,55 @@ address_compare (tree_code code, tree ty
   HOST_WIDE_INT ioff0 = -1, ioff1 = -1;
   off0.is_constant (&ioff0);
   off1.is_constant (&ioff1);
-  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
-       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
-       || (TREE_CODE (base0) == STRING_CST
-	   && TREE_CODE (base1) == STRING_CST
-	   && ioff0 >= 0 && ioff1 >= 0
-	   && ioff0 < TREE_STRING_LENGTH (base0)
-	   && ioff1 < TREE_STRING_LENGTH (base1)
-	  /* This is a too conservative test that the STRING_CSTs
-	     will not end up being string-merged.  */
-	   && strncmp (TREE_STRING_POINTER (base0) + ioff0,
-		       TREE_STRING_POINTER (base1) + ioff1,
-		       MIN (TREE_STRING_LENGTH (base0) - ioff0,
-			    TREE_STRING_LENGTH (base1) - ioff1)) != 0))
-    ;
-  else if (!DECL_P (base0) || !DECL_P (base1))
+  /* Punt on non-zero offsets from functions.  */
+  if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
+      || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
     return 2;
-  /* If this is a pointer comparison, ignore for now even
-     valid equalities where one pointer is the offset zero
-     of one object and the other to one past end of another one.  */
-  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
-    ;
-  /* Assume that automatic variables can't be adjacent to global
-     variables.  */
-  else if (is_global_var (base0) != is_global_var (base1))
-    ;
+  /* Or if the bases are neither decls nor string literals.  */
+  if (!DECL_P (base0) && TREE_CODE (base0) != STRING_CST)
+    return 2;
+  if (!DECL_P (base1) && TREE_CODE (base1) != STRING_CST)
+    return 2;
+
+  /* Compute whether one address points to the start of one
+     object and another one to the end of another one.  */
+  poly_int64 size0 = 0, size1 = 0;
+  if (TREE_CODE (base0) == STRING_CST)
+    {
+      if (ioff0 < 0 || ioff0 > TREE_STRING_LENGTH (base0))
+	equal = 2;
+      else
+	size0 = TREE_STRING_LENGTH (base0);
+    }
+  else if (TREE_CODE (base0) == FUNCTION_DECL)
+    size0 = 1;
   else
     {
       tree sz0 = DECL_SIZE_UNIT (base0);
+      if (!tree_fits_poly_int64_p (sz0))
+	equal = 2;
+      else
+	size0 = tree_to_poly_int64 (sz0);
+    }
+  if (TREE_CODE (base1) == STRING_CST)
+    {
+      if (ioff1 < 0 || ioff1 > TREE_STRING_LENGTH (base1))
+	equal = 2;
+      else
+	size1 = TREE_STRING_LENGTH (base1);
+    }
+  else if (TREE_CODE (base1) == FUNCTION_DECL)
+    size1 = 1;
+  else
+    {
       tree sz1 = DECL_SIZE_UNIT (base1);
-      /* If sizes are unknown, e.g. VLA or not representable, punt.  */
-      if (!tree_fits_poly_int64_p (sz0) || !tree_fits_poly_int64_p (sz1))
-	return 2;
-
-      poly_int64 size0 = tree_to_poly_int64 (sz0);
-      poly_int64 size1 = tree_to_poly_int64 (sz1);
+      if (!tree_fits_poly_int64_p (sz1))
+	equal = 2;
+      else
+	size1 = tree_to_poly_int64 (sz1);
+    }
+  if (equal == 0)
+    {
       /* If one offset is pointing (or could be) to the beginning of one
 	 object and the other is pointing to one past the last byte of the
 	 other object, punt.  */
@@ -16678,7 +16700,60 @@ address_compare (tree_code code, tree ty
 	  && (known_ne (off0, 0)
 	      || (known_ne (size0, 0) && known_ne (size1, 0))))
 	equal = 0;
-     }
+    }
+
+  if (TREE_CODE (base0) == STRING_CST || TREE_CODE (base1) == STRING_CST)
+    {
+      if (TREE_CODE (base0) != TREE_CODE (base1))
+	return folding_cxx_constexpr ? equal : 0;
+
+      /* STRING_CST vs. STRING_CST.  */
+      if (folding_cxx_constexpr && equal)
+	return equal;
+
+      if (ioff0 < 0
+	  || ioff1 < 0
+	  || ioff0 > TREE_STRING_LENGTH (base0)
+	  || ioff1 > TREE_STRING_LENGTH (base1))
+	return 2;
+
+      /* If the bytes in the string literals starting at the pointers
+	 differ, the pointers need to be different.  */
+      if (memcmp (TREE_STRING_POINTER (base0) + ioff0,
+		  TREE_STRING_POINTER (base1) + ioff1,
+		  MIN (TREE_STRING_LENGTH (base0) - ioff0,
+		       TREE_STRING_LENGTH (base1) - ioff1)) == 0)
+	{
+	  HOST_WIDE_INT ioffmin = MIN (ioff0, ioff1);
+	  if (memcmp (TREE_STRING_POINTER (base0) + ioff0 - ioffmin,
+		      TREE_STRING_POINTER (base1) + ioff1 - ioffmin,
+		      ioffmin) == 0)
+	    /* If even the bytes in the string literal before the
+	       pointers are the same, the string literals could be
+	       tail merged.  */
+	    return 2;
+	}
+      return 0;
+    }
+
+  /* If this is a pointer comparison, ignore for now even
+     valid equalities where one pointer is the offset zero
+     of one object and the other to one past end of another one.  */
+  if (!folding_cxx_constexpr && !INTEGRAL_TYPE_P (type))
+    return 0;
+
+  /* Assume that automatic variables can't be adjacent to global
+     variables.  */
+  if (!folding_cxx_constexpr && is_global_var (base0) != is_global_var (base1))
+    return 0;
+
+  /* For initializers, assume addresses of different functions are
+     different.  */
+  else if (folding_initializer
+	   && TREE_CODE (base0) == FUNCTION_DECL
+	   && TREE_CODE (base1) == FUNCTION_DECL)
+    return 0;
+
   return equal;
 }
 
--- gcc/cp/constexpr.cc.jj	2022-01-26 19:40:22.957796808 +0100
+++ gcc/cp/constexpr.cc	2022-02-03 15:04:00.760558570 +0100
@@ -3413,7 +3413,10 @@ cxx_eval_binary_expression (const conste
       if (ctx->manifestly_const_eval
 	  && (flag_constexpr_fp_except
 	      || TREE_CODE (type) != REAL_TYPE))
-	r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
+	{
+	  auto ofcc = make_temp_override (folding_cxx_constexpr, true);
+	  r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
+	}
       else
 	r = fold_binary_loc (loc, code, type, lhs, rhs);
     }
--- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C.jj	2022-02-03 14:58:53.734901694 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C	2022-02-03 14:58:53.734901694 +0100
@@ -0,0 +1,132 @@
+// PR c++/89074
+// { dg-do compile { target c++14 } }
+
+int fn1 (void) { return 0; }
+int fn2 (void) { return 1; }
+
+constexpr bool
+f1 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  if (&a[1] == "foo")
+    return false;
+
+  if (&a[1] == &"foo"[4])
+    return false;
+
+  if (&"foo"[1] == &a[0])
+    return false;
+
+  if (&"foo"[3] == &a[4])
+    return false;
+
+  if (&a[0] == "foo")
+    return false;
+
+  // Pointer to start of one object (var) and end of another one (literal)
+  if (&a[0] == &"foo"[4])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f2 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  // Pointer to end of one object (var) and start of another one (literal)
+  if (&a[4] == "foo")		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+char v[] = { 1, 2, 3, 4 };
+
+constexpr bool
+f3 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  if (&a[1] == &v[1])
+    return false;
+
+  if (&a[0] == &v[3])
+    return false;
+
+  if (&a[2] == &v[4])
+    return false;
+
+  // Pointer to start of one object (automatic var) and end of another one (non-automagic var)
+  if (&a[0] == &v[4])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f4 ()
+{
+  char a[] = { 1, 2, 3, 4, 5 };
+
+  // Pointer to end of one object (automatic var) and start of another one (non-automagic var)
+  if (&a[5] == &v[0])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f5 ()
+{
+  if (fn1 != fn1)
+    return false;
+
+  if (fn1 == fn2)
+    return false;
+
+  if (&"abcde"[0] == &"edcba"[1])
+    return false;
+
+  if (&"abcde"[1] == &"edcba"[6])
+    return false;
+
+  // Pointer to start of one object (literal) and end of another one (literal)
+  if (&"abcde"[0] == &"edcba"[6])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f6 ()
+{
+  // Pointer to start of one object (literal) and end of another one (literal)
+  if (&"abcde"[6] == &"edcba"[0])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f7 ()
+{
+  if (&"abcde"[3] == &"fabcde"[3])
+    return false;
+
+  // These could be suffix merged, with &"abcde"[0] == &"fabcde"[1].
+  if (&"abcde"[3] == &"fabcde"[4])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool a = f1 ();
+constexpr bool b = f2 ();
+constexpr bool c = f3 ();
+constexpr bool d = f4 ();
+constexpr bool e = f5 ();
+constexpr bool f = f6 ();
+constexpr bool g = f7 ();


	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++, v3: Further address_compare fixes [PR89074]
  2022-02-04 13:41                     ` [PATCH] c++, v3: " Jakub Jelinek
@ 2022-02-04 21:42                       ` Jason Merrill
  2022-02-04 23:02                         ` Jakub Jelinek
  0 siblings, 1 reply; 21+ messages in thread
From: Jason Merrill @ 2022-02-04 21:42 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches

On 2/4/22 08:41, Jakub Jelinek wrote:
> On Thu, Feb 03, 2022 at 04:34:17PM -0500, Jason Merrill wrote:
>> On 2/3/22 16:18, Jakub Jelinek wrote:
>>> On Thu, Feb 03, 2022 at 04:04:57PM -0500, Jason Merrill wrote:
>>>>>> I think it would be clearer to leave the !DECL_P case alone and add
>>>>>>
>>>>>> /* In C++ it is unspecified, and so non-constant, whether two
>>>>>>       equivalent strings have the same address.  */
>>>>>> else if (folding_cxx_constexpr
>>>>>>             && (TREE_CODE (base0) == STRING_CST
>>>>>>                 || TREE_CODE (base1) == STRING_CST)
>>>>>
>>>>> The point was to let the first if handle for
>>>>> !folding_cxx_constexpr the cases with STRING_CST
>>>>> as one or both operands and if that falls through, return 2.
>>>>
>>>> Ah, I see.  And then for folding_cxx_constexpr you have your new code toward
>>>> the bottom of the function that can say they're unequal in some cases.  Can
>>>> you combine the STRING_CST handling for both values of folding_cxx_constexpr
>>>> instead of having them so far apart?
>>>
>>> Not easily, because for the folding_cxx_constexpr case it primarily reuses
>>> the code from the last else if - computing sizes of objects and checking
>>> if one is at a start of one and another at the end of the other.
>>
>> And the !folding_cxx_constexpr case shouldn't also use that code?
>>
>>> One further option would be to compute early flags like
>>>     enum { OFF_POS_START, OFF_POS_MIDDLE, OFF_POS_END } pos0, pos1;
>>> and then just use them or ignore them in the decisions later.
>>
>> If that helps to refactor a bit, sure.
> 
> Here it is, hopefully it makes the code more readable and understandable.

Much more readable, thanks!

> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
> 
> 2022-02-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89074
> 	PR c++/104033
> 	* fold-const.h (folding_initializer): Adjust comment.
> 	(folding_cxx_constexpr): Declare.
> 	* fold-const.cc (folding_initializer): Adjust comment.
> 	(folding_cxx_constexpr): New variable.
> 	(address_compare): Restrict the decl vs. STRING_CST
> 	or vice versa or STRING_CST vs. STRING_CST or
> 	is_global_var != is_global_var optimizations to !folding_cxx_constexpr.
> 	Punt for FUNCTION_DECLs with non-zero offsets.  If folding_initializer,
> 	assume non-aliased functions have non-zero size and have different
> 	addresses.  For folding_cxx_constexpr, punt on comparisons of start
> 	of some object and end of another one, regardless whether it is a decl
> 	or string literal.  Also punt for folding_cxx_constexpr on
> 	STRING_CST vs. STRING_CST comparisons if the two literals could be
> 	overlapping.
> 
> 	* constexpr.cc (cxx_eval_binary_expression): Temporarily set
> 	folding_cxx_constexpr.
> 
> 	* g++.dg/cpp1y/constexpr-89074-3.C: New test.
> 
> --- gcc/fold-const.h.jj	2022-02-01 20:10:51.235856007 +0100
> +++ gcc/fold-const.h	2022-02-03 15:02:02.700228631 +0100
> @@ -20,9 +20,16 @@ along with GCC; see the file COPYING3.
>   #ifndef GCC_FOLD_CONST_H
>   #define GCC_FOLD_CONST_H
>   
> -/* Non-zero if we are folding constants inside an initializer; zero
> -   otherwise.  */
> +/* Nonzero if we are folding constants inside an initializer or a C++
> +   manifestly-constant-evaluated context; zero otherwise.
> +   Should be used when folding in initializer enables additional
> +   optimizations.  */
>   extern int folding_initializer;
> +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero

Still need to fix this typo.

> +   otherwise.
> +   Should be used when certain constructs shouldn't be optimized
> +   during folding in that context.  */
> +extern bool folding_cxx_constexpr;
>   
>   /* Convert between trees and native memory representation.  */
>   extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
> --- gcc/fold-const.cc.jj	2022-02-03 14:31:32.243129408 +0100
> +++ gcc/fold-const.cc	2022-02-04 10:19:13.812784763 +0100
> @@ -86,9 +86,17 @@ along with GCC; see the file COPYING3.
>   #include "gimple-range.h"
>   
>   /* Nonzero if we are folding constants inside an initializer or a C++
> -   manifestly-constant-evaluated context; zero otherwise.  */
> +   manifestly-constant-evaluated context; zero otherwise.
> +   Should be used when folding in initializer enables additional
> +   optimizations.  */
>   int folding_initializer = 0;
>   
> +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero

And here.

> +   otherwise.
> +   Should be used when certain constructs shouldn't be optimized
> +   during folding in that context.  */
> +bool folding_cxx_constexpr = false;
> +
>   /* The following constants represent a bit based encoding of GCC's
>      comparison operators.  This encoding simplifies transformations
>      on relational comparison operators, such as AND and OR.  */
> @@ -16628,41 +16636,55 @@ address_compare (tree_code code, tree ty

Incidentally, the function comment needs to document TYPE.

>     HOST_WIDE_INT ioff0 = -1, ioff1 = -1;
>     off0.is_constant (&ioff0);
>     off1.is_constant (&ioff1);
> -  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> -       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> -       || (TREE_CODE (base0) == STRING_CST
> -	   && TREE_CODE (base1) == STRING_CST
> -	   && ioff0 >= 0 && ioff1 >= 0
> -	   && ioff0 < TREE_STRING_LENGTH (base0)
> -	   && ioff1 < TREE_STRING_LENGTH (base1)
> -	  /* This is a too conservative test that the STRING_CSTs
> -	     will not end up being string-merged.  */
> -	   && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> -		       TREE_STRING_POINTER (base1) + ioff1,
> -		       MIN (TREE_STRING_LENGTH (base0) - ioff0,
> -			    TREE_STRING_LENGTH (base1) - ioff1)) != 0))
> -    ;
> -  else if (!DECL_P (base0) || !DECL_P (base1))
> +  /* Punt on non-zero offsets from functions.  */
> +  if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
> +      || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
>       return 2;
> -  /* If this is a pointer comparison, ignore for now even
> -     valid equalities where one pointer is the offset zero
> -     of one object and the other to one past end of another one.  */
> -  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
> -    ;
> -  /* Assume that automatic variables can't be adjacent to global
> -     variables.  */
> -  else if (is_global_var (base0) != is_global_var (base1))
> -    ;
> +  /* Or if the bases are neither decls nor string literals.  */
> +  if (!DECL_P (base0) && TREE_CODE (base0) != STRING_CST)
> +    return 2;
> +  if (!DECL_P (base1) && TREE_CODE (base1) != STRING_CST)
> +    return 2;
> +
> +  /* Compute whether one address points to the start of one
> +     object and another one to the end of another one.  */
> +  poly_int64 size0 = 0, size1 = 0;
> +  if (TREE_CODE (base0) == STRING_CST)
> +    {
> +      if (ioff0 < 0 || ioff0 > TREE_STRING_LENGTH (base0))
> +	equal = 2;
> +      else
> +	size0 = TREE_STRING_LENGTH (base0);
> +    }
> +  else if (TREE_CODE (base0) == FUNCTION_DECL)
> +    size0 = 1;
>     else
>       {
>         tree sz0 = DECL_SIZE_UNIT (base0);
> +      if (!tree_fits_poly_int64_p (sz0))
> +	equal = 2;
> +      else
> +	size0 = tree_to_poly_int64 (sz0);
> +    }
> +  if (TREE_CODE (base1) == STRING_CST)
> +    {
> +      if (ioff1 < 0 || ioff1 > TREE_STRING_LENGTH (base1))
> +	equal = 2;
> +      else
> +	size1 = TREE_STRING_LENGTH (base1);
> +    }
> +  else if (TREE_CODE (base1) == FUNCTION_DECL)
> +    size1 = 1;
> +  else
> +    {
>         tree sz1 = DECL_SIZE_UNIT (base1);
> -      /* If sizes are unknown, e.g. VLA or not representable, punt.  */
> -      if (!tree_fits_poly_int64_p (sz0) || !tree_fits_poly_int64_p (sz1))
> -	return 2;
> -
> -      poly_int64 size0 = tree_to_poly_int64 (sz0);
> -      poly_int64 size1 = tree_to_poly_int64 (sz1);
> +      if (!tree_fits_poly_int64_p (sz1))
> +	equal = 2;
> +      else
> +	size1 = tree_to_poly_int64 (sz1);
> +    }
> +  if (equal == 0)
> +    {
>         /* If one offset is pointing (or could be) to the beginning of one
>   	 object and the other is pointing to one past the last byte of the
>   	 other object, punt.  */
> @@ -16678,7 +16700,60 @@ address_compare (tree_code code, tree ty
>   	  && (known_ne (off0, 0)
>   	      || (known_ne (size0, 0) && known_ne (size1, 0))))
>   	equal = 0;
> -     }
> +    }
> +

So at this point equal can be 0 or 2, the latter because either the 
offset is out of bounds, or because we're comparing offset 0 to 
one-past-the-end.

In the out-of-bounds case, we're into undefined behavior, so I'd be 
inclined to return 2 immediately rather than continue, so the code below 
only needs to worry about possibly overlapping/contiguous objects.

In the code below, in !constexpr mode we decide to return 0 even though 
equal == 2 in three cases which need more commentary, either together or 
separately:

1) One is a string and the other a decl.  Do we know that we can't 
layout a string and a global variable next to each other?  This overlaps 
a lot with...

2) We're comparing as pointers (rather than integers), so we return 
unequal even if they could be equal in practice if the objects are 
contiguous.  The comment says this but still needs a rationale; it 
doesn't seem useful to me for the limited cases that could reach here 
with equal == 2.

3) We're comparing a local variable and a global, so they really can't 
be equal unless the offset is far out of bounds.  This is currently 
last; we might move it first and treat strings like globals?

I know the above is existing behavior, but I'd like to at least improve 
the comments.

> +  if (TREE_CODE (base0) == STRING_CST || TREE_CODE (base1) == STRING_CST)
> +    {
> +      if (TREE_CODE (base0) != TREE_CODE (base1))
> +	return folding_cxx_constexpr ? equal : 0;
> +
> +      /* STRING_CST vs. STRING_CST.  */
> +      if (folding_cxx_constexpr && equal)
> +	return equal;
> +
> +      if (ioff0 < 0
> +	  || ioff1 < 0
> +	  || ioff0 > TREE_STRING_LENGTH (base0)
> +	  || ioff1 > TREE_STRING_LENGTH (base1))
> +	return 2;
> +
> +      /* If the bytes in the string literals starting at the pointers
> +	 differ, the pointers need to be different.  */
> +      if (memcmp (TREE_STRING_POINTER (base0) + ioff0,
> +		  TREE_STRING_POINTER (base1) + ioff1,
> +		  MIN (TREE_STRING_LENGTH (base0) - ioff0,
> +		       TREE_STRING_LENGTH (base1) - ioff1)) == 0)
> +	{
> +	  HOST_WIDE_INT ioffmin = MIN (ioff0, ioff1);
> +	  if (memcmp (TREE_STRING_POINTER (base0) + ioff0 - ioffmin,
> +		      TREE_STRING_POINTER (base1) + ioff1 - ioffmin,
> +		      ioffmin) == 0)
> +	    /* If even the bytes in the string literal before the
> +	       pointers are the same, the string literals could be
> +	       tail merged.  */
> +	    return 2;
> +	}
> +      return 0;
> +    }
> +
> +  /* If this is a pointer comparison, ignore for now even
> +     valid equalities where one pointer is the offset zero
> +     of one object and the other to one past end of another one.  */
> +  if (!folding_cxx_constexpr && !INTEGRAL_TYPE_P (type))
> +    return 0;
> +
> +  /* Assume that automatic variables can't be adjacent to global
> +     variables.  */
> +  if (!folding_cxx_constexpr && is_global_var (base0) != is_global_var (base1))
> +    return 0;
> +
> +  /* For initializers, assume addresses of different functions are
> +     different.  */
> +  else if (folding_initializer
> +	   && TREE_CODE (base0) == FUNCTION_DECL
> +	   && TREE_CODE (base1) == FUNCTION_DECL)
> +    return 0;
> +
>     return equal;
>   }
>   
> --- gcc/cp/constexpr.cc.jj	2022-01-26 19:40:22.957796808 +0100
> +++ gcc/cp/constexpr.cc	2022-02-03 15:04:00.760558570 +0100
> @@ -3413,7 +3413,10 @@ cxx_eval_binary_expression (const conste
>         if (ctx->manifestly_const_eval
>   	  && (flag_constexpr_fp_except
>   	      || TREE_CODE (type) != REAL_TYPE))
> -	r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
> +	{
> +	  auto ofcc = make_temp_override (folding_cxx_constexpr, true);
> +	  r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
> +	}
>         else
>   	r = fold_binary_loc (loc, code, type, lhs, rhs);
>       }
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C.jj	2022-02-03 14:58:53.734901694 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C	2022-02-03 14:58:53.734901694 +0100
> @@ -0,0 +1,132 @@
> +// PR c++/89074
> +// { dg-do compile { target c++14 } }
> +
> +int fn1 (void) { return 0; }
> +int fn2 (void) { return 1; }
> +
> +constexpr bool
> +f1 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  if (&a[1] == "foo")
> +    return false;
> +
> +  if (&a[1] == &"foo"[4])
> +    return false;
> +
> +  if (&"foo"[1] == &a[0])
> +    return false;
> +
> +  if (&"foo"[3] == &a[4])
> +    return false;
> +
> +  if (&a[0] == "foo")
> +    return false;
> +
> +  // Pointer to start of one object (var) and end of another one (literal)
> +  if (&a[0] == &"foo"[4])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f2 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  // Pointer to end of one object (var) and start of another one (literal)
> +  if (&a[4] == "foo")		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +char v[] = { 1, 2, 3, 4 };
> +
> +constexpr bool
> +f3 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  if (&a[1] == &v[1])
> +    return false;
> +
> +  if (&a[0] == &v[3])
> +    return false;
> +
> +  if (&a[2] == &v[4])
> +    return false;
> +
> +  // Pointer to start of one object (automatic var) and end of another one (non-automagic var)
> +  if (&a[0] == &v[4])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f4 ()
> +{
> +  char a[] = { 1, 2, 3, 4, 5 };
> +
> +  // Pointer to end of one object (automatic var) and start of another one (non-automagic var)
> +  if (&a[5] == &v[0])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f5 ()
> +{
> +  if (fn1 != fn1)
> +    return false;
> +
> +  if (fn1 == fn2)
> +    return false;
> +
> +  if (&"abcde"[0] == &"edcba"[1])
> +    return false;
> +
> +  if (&"abcde"[1] == &"edcba"[6])
> +    return false;
> +
> +  // Pointer to start of one object (literal) and end of another one (literal)
> +  if (&"abcde"[0] == &"edcba"[6])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f6 ()
> +{
> +  // Pointer to start of one object (literal) and end of another one (literal)
> +  if (&"abcde"[6] == &"edcba"[0])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f7 ()
> +{
> +  if (&"abcde"[3] == &"fabcde"[3])
> +    return false;
> +
> +  // These could be suffix merged, with &"abcde"[0] == &"fabcde"[1].
> +  if (&"abcde"[3] == &"fabcde"[4])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool a = f1 ();
> +constexpr bool b = f2 ();
> +constexpr bool c = f3 ();
> +constexpr bool d = f4 ();
> +constexpr bool e = f5 ();
> +constexpr bool f = f6 ();
> +constexpr bool g = f7 ();
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++, v3: Further address_compare fixes [PR89074]
  2022-02-04 21:42                       ` Jason Merrill
@ 2022-02-04 23:02                         ` Jakub Jelinek
  2022-02-05 12:17                           ` [PATCH] c++, v4: " Jakub Jelinek
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Jelinek @ 2022-02-04 23:02 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Richard Biener, gcc-patches

On Fri, Feb 04, 2022 at 04:42:41PM -0500, Jason Merrill wrote:
> > @@ -20,9 +20,16 @@ along with GCC; see the file COPYING3.
> >   #ifndef GCC_FOLD_CONST_H
> >   #define GCC_FOLD_CONST_H
> > -/* Non-zero if we are folding constants inside an initializer; zero
> > -   otherwise.  */
> > +/* Nonzero if we are folding constants inside an initializer or a C++
> > +   manifestly-constant-evaluated context; zero otherwise.
> > +   Should be used when folding in initializer enables additional
> > +   optimizations.  */
> >   extern int folding_initializer;
> > +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero
> 
> Still need to fix this typo.

Sorry, finally now fixed in my copy.

> > +   otherwise.
> > +   Should be used when certain constructs shouldn't be optimized
> > +   during folding in that context.  */
> > +bool folding_cxx_constexpr = false;
> > +
> >   /* The following constants represent a bit based encoding of GCC's
> >      comparison operators.  This encoding simplifies transformations
> >      on relational comparison operators, such as AND and OR.  */
> > @@ -16628,41 +16636,55 @@ address_compare (tree_code code, tree ty
> 
> Incidentally, the function comment needs to document TYPE.

Will do.

> So at this point equal can be 0 or 2, the latter because either the offset
> is out of bounds, or because we're comparing offset 0 to one-past-the-end.

Yes.

> In the out-of-bounds case, we're into undefined behavior, so I'd be inclined
> to return 2 immediately rather than continue, so the code below only needs
> to worry about possibly overlapping/contiguous objects.

You mean for folding_cxx_constexpr ?  The code does that basically, with one
exception, the folding_initializer FUNCTION_DECL cmp FUNCTION_DECL case.
We don't track sizes of functions, so the size of 1 is just a hack to
pretend functions don't have zero size.  Some functions can have zero size
if they contain just __builtin_unreachable, but it is very rare.
But I guess I could move that
  if (folding_initializer
      && TREE_CODE (base0) == FUNCTION_DECL
      && TREE_CODE (base1) == FUNCTION_DECL)
    return 0;
above the size checking block and then indeed right after that do
  if (folding_cxx_constexpr && equal)
    return equal;
with a comment.

> In the code below, in !constexpr mode we decide to return 0 even though
> equal == 2 in three cases which need more commentary, either together or
> separately:
> 
> 1) One is a string and the other a decl.  Do we know that we can't layout a
> string and a global variable next to each other?  This overlaps a lot
> with...
> 
> 2) We're comparing as pointers (rather than integers), so we return unequal
> even if they could be equal in practice if the objects are contiguous.  The
> comment says this but still needs a rationale; it doesn't seem useful to me
> for the limited cases that could reach here with equal == 2.

For the pointer comparisons we just exploit the undefined behavior and
pretend they can't be adjacent even if they actually sometimes can be,
and we've been doing that intentionally for years.
If one does (uintptr_t) &x == (uintptr_t) &y, we try to be more
conservative.

> 3) We're comparing a local variable and a global, so they really can't be
> equal unless the offset is far out of bounds.  This is currently last; we
> might move it first and treat strings like globals?

For the automatic vs. global or strings, it is very unlikely they'd be
adjacent, with typical memory layouts there couldn't be any heap and stack
would need to grow until it reaches end of data or bss section on a page
boundary.  Strings perhaps could be adjacent in .rodata, but again it is
fairly rare.
And sure, I can try to improve comments.

	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] c++, v4: Further address_compare fixes [PR89074]
  2022-02-04 23:02                         ` Jakub Jelinek
@ 2022-02-05 12:17                           ` Jakub Jelinek
  2022-02-05 13:54                             ` Jason Merrill
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Jelinek @ 2022-02-05 12:17 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Richard Biener, gcc-patches

On Sat, Feb 05, 2022 at 12:02:14AM +0100, Jakub Jelinek via Gcc-patches wrote:
> You mean for folding_cxx_constexpr ?  The code does that basically, with one
> exception, the folding_initializer FUNCTION_DECL cmp FUNCTION_DECL case.
> We don't track sizes of functions, so the size of 1 is just a hack to
> pretend functions don't have zero size.  Some functions can have zero size
> if they contain just __builtin_unreachable, but it is very rare.
> But I guess I could move that
>   if (folding_initializer
>       && TREE_CODE (base0) == FUNCTION_DECL
>       && TREE_CODE (base1) == FUNCTION_DECL)
>     return 0;
> above the size checking block and then indeed right after that do
>   if (folding_cxx_constexpr && equal)
>     return equal;
> with a comment.

Here is an updated patch:

2022-02-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/89074
	PR c++/104033
	* fold-const.h (folding_initializer): Adjust comment.
	(folding_cxx_constexpr): Declare.
	* fold-const.cc (folding_initializer): Adjust comment.
	(folding_cxx_constexpr): New variable.
	(address_compare): Restrict the decl vs. STRING_CST
	or vice versa or STRING_CST vs. STRING_CST or
	is_global_var != is_global_var optimizations to !folding_cxx_constexpr.
	Punt for FUNCTION_DECLs with non-zero offsets.  If folding_initializer,
	assume non-aliased functions have non-zero size and have different
	addresses.  For folding_cxx_constexpr, punt on comparisons of start
	of some object and end of another one, regardless whether it is a decl
	or string literal.  Also punt for folding_cxx_constexpr on
	STRING_CST vs. STRING_CST comparisons if the two literals could be
	overlapping.

	* constexpr.cc (cxx_eval_binary_expression): Temporarily set
	folding_cxx_constexpr.

	* g++.dg/cpp1y/constexpr-89074-3.C: New test.

--- gcc/fold-const.h.jj	2022-02-04 18:30:34.695003975 +0100
+++ gcc/fold-const.h	2022-02-05 12:47:54.935664258 +0100
@@ -20,9 +20,16 @@ along with GCC; see the file COPYING3.
 #ifndef GCC_FOLD_CONST_H
 #define GCC_FOLD_CONST_H
 
-/* Non-zero if we are folding constants inside an initializer; zero
-   otherwise.  */
+/* Nonzero if we are folding constants inside an initializer or a C++
+   manifestly-constant-evaluated context; zero otherwise.
+   Should be used when folding in initializer enables additional
+   optimizations.  */
 extern int folding_initializer;
+/* Nonzero if we are folding C++ manifestly-constant-evaluated context; zero
+   otherwise.
+   Should be used when certain constructs shouldn't be optimized
+   during folding in that context.  */
+extern bool folding_cxx_constexpr;
 
 /* Convert between trees and native memory representation.  */
 extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
--- gcc/fold-const.cc.jj	2022-02-04 18:30:34.695003975 +0100
+++ gcc/fold-const.cc	2022-02-05 13:07:53.801996609 +0100
@@ -86,9 +86,17 @@ along with GCC; see the file COPYING3.
 #include "gimple-range.h"
 
 /* Nonzero if we are folding constants inside an initializer or a C++
-   manifestly-constant-evaluated context; zero otherwise.  */
+   manifestly-constant-evaluated context; zero otherwise.
+   Should be used when folding in initializer enables additional
+   optimizations.  */
 int folding_initializer = 0;
 
+/* Nonzero if we are folding C++ manifestly-constant-evaluated context; zero
+   otherwise.
+   Should be used when certain constructs shouldn't be optimized
+   during folding in that context.  */
+bool folding_cxx_constexpr = false;
+
 /* The following constants represent a bit based encoding of GCC's
    comparison operators.  This encoding simplifies transformations
    on relational comparison operators, such as AND and OR.  */
@@ -16572,6 +16580,7 @@ tree_nonzero_bits (const_tree t)
 
 /* Helper function for address compare simplifications in match.pd.
    OP0 and OP1 are ADDR_EXPR operands being compared by CODE.
+   TYPE is the type of comparison operands.
    BASE0, BASE1, OFF0 and OFF1 are set by the function.
    GENERIC is true if GENERIC folding and false for GIMPLE folding.
    Returns 0 if OP0 is known to be unequal to OP1 regardless of OFF{0,1},
@@ -16648,44 +16657,66 @@ address_compare (tree_code code, tree ty
   if (code != EQ_EXPR && code != NE_EXPR)
     return 2;
 
+  /* At this point we know (or assume) the two pointers point at
+     different objects.  */
   HOST_WIDE_INT ioff0 = -1, ioff1 = -1;
   off0.is_constant (&ioff0);
   off1.is_constant (&ioff1);
-  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
-       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
-       || (TREE_CODE (base0) == STRING_CST
-	   && TREE_CODE (base1) == STRING_CST
-	   && ioff0 >= 0 && ioff1 >= 0
-	   && ioff0 < TREE_STRING_LENGTH (base0)
-	   && ioff1 < TREE_STRING_LENGTH (base1)
-	  /* This is a too conservative test that the STRING_CSTs
-	     will not end up being string-merged.  */
-	   && strncmp (TREE_STRING_POINTER (base0) + ioff0,
-		       TREE_STRING_POINTER (base1) + ioff1,
-		       MIN (TREE_STRING_LENGTH (base0) - ioff0,
-			    TREE_STRING_LENGTH (base1) - ioff1)) != 0))
-    ;
-  else if (!DECL_P (base0) || !DECL_P (base1))
+  /* Punt on non-zero offsets from functions.  */
+  if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
+      || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
     return 2;
-  /* If this is a pointer comparison, ignore for now even
-     valid equalities where one pointer is the offset zero
-     of one object and the other to one past end of another one.  */
-  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
-    ;
-  /* Assume that automatic variables can't be adjacent to global
-     variables.  */
-  else if (is_global_var (base0) != is_global_var (base1))
-    ;
+  /* Or if the bases are neither decls nor string literals.  */
+  if (!DECL_P (base0) && TREE_CODE (base0) != STRING_CST)
+    return 2;
+  if (!DECL_P (base1) && TREE_CODE (base1) != STRING_CST)
+    return 2;
+  /* For initializers, assume addresses of different functions are
+     different.  */
+  if (folding_initializer
+      && TREE_CODE (base0) == FUNCTION_DECL
+      && TREE_CODE (base1) == FUNCTION_DECL)
+    return 0;
+
+  /* Compute whether one address points to the start of one
+     object and another one to the end of another one.  */
+  poly_int64 size0 = 0, size1 = 0;
+  if (TREE_CODE (base0) == STRING_CST)
+    {
+      if (ioff0 < 0 || ioff0 > TREE_STRING_LENGTH (base0))
+	equal = 2;
+      else
+	size0 = TREE_STRING_LENGTH (base0);
+    }
+  else if (TREE_CODE (base0) == FUNCTION_DECL)
+    size0 = 1;
   else
     {
       tree sz0 = DECL_SIZE_UNIT (base0);
+      if (!tree_fits_poly_int64_p (sz0))
+	equal = 2;
+      else
+	size0 = tree_to_poly_int64 (sz0);
+    }
+  if (TREE_CODE (base1) == STRING_CST)
+    {
+      if (ioff1 < 0 || ioff1 > TREE_STRING_LENGTH (base1))
+	equal = 2;
+      else
+	size1 = TREE_STRING_LENGTH (base1);
+    }
+  else if (TREE_CODE (base1) == FUNCTION_DECL)
+    size1 = 1;
+  else
+    {
       tree sz1 = DECL_SIZE_UNIT (base1);
-      /* If sizes are unknown, e.g. VLA or not representable, punt.  */
-      if (!tree_fits_poly_int64_p (sz0) || !tree_fits_poly_int64_p (sz1))
-	return 2;
-
-      poly_int64 size0 = tree_to_poly_int64 (sz0);
-      poly_int64 size1 = tree_to_poly_int64 (sz1);
+      if (!tree_fits_poly_int64_p (sz1))
+	equal = 2;
+      else
+	size1 = tree_to_poly_int64 (sz1);
+    }
+  if (equal == 0)
+    {
       /* If one offset is pointing (or could be) to the beginning of one
 	 object and the other is pointing to one past the last byte of the
 	 other object, punt.  */
@@ -16701,7 +16732,63 @@ address_compare (tree_code code, tree ty
 	  && (known_ne (off0, 0)
 	      || (known_ne (size0, 0) && known_ne (size1, 0))))
 	equal = 0;
-     }
+    }
+
+  /* At this point, equal is 2 if either one or both pointers are out of
+     bounds of their object, or one points to start of its object and the
+     other points to end of its object.  This is unspecified behavior
+     e.g. in C++.  Otherwise equal is 0.  */
+  if (folding_cxx_constexpr && equal)
+    return equal;
+
+  /* When both pointers point to string literals, even when equal is 0,
+     due to tail merging of string literals the pointers might be the same.  */
+  if (TREE_CODE (base0) == STRING_CST && TREE_CODE (base1) == STRING_CST)
+    {
+      if (ioff0 < 0
+	  || ioff1 < 0
+	  || ioff0 > TREE_STRING_LENGTH (base0)
+	  || ioff1 > TREE_STRING_LENGTH (base1))
+	return 2;
+
+      /* If the bytes in the string literals starting at the pointers
+	 differ, the pointers need to be different.  */
+      if (memcmp (TREE_STRING_POINTER (base0) + ioff0,
+		  TREE_STRING_POINTER (base1) + ioff1,
+		  MIN (TREE_STRING_LENGTH (base0) - ioff0,
+		       TREE_STRING_LENGTH (base1) - ioff1)) == 0)
+	{
+	  HOST_WIDE_INT ioffmin = MIN (ioff0, ioff1);
+	  if (memcmp (TREE_STRING_POINTER (base0) + ioff0 - ioffmin,
+		      TREE_STRING_POINTER (base1) + ioff1 - ioffmin,
+		      ioffmin) == 0)
+	    /* If even the bytes in the string literal before the
+	       pointers are the same, the string literals could be
+	       tail merged.  */
+	    return 2;
+	}
+      return 0;
+    }
+
+  if (folding_cxx_constexpr)
+    return 0;
+
+  /* If this is a pointer comparison, ignore for now even
+     valid equalities where one pointer is the offset zero
+     of one object and the other to one past end of another one.  */
+  if (!INTEGRAL_TYPE_P (type))
+    return 0;
+
+  /* Assume that string literals can't be adjacent to variables
+     (automatic or global).  */
+  if (TREE_CODE (base0) == STRING_CST || TREE_CODE (base1) == STRING_CST)
+    return 0;
+
+  /* Assume that automatic variables can't be adjacent to global
+     variables.  */
+  if (is_global_var (base0) != is_global_var (base1))
+    return 0;
+
   return equal;
 }
 
--- gcc/cp/constexpr.cc.jj	2022-02-04 14:36:54.597610997 +0100
+++ gcc/cp/constexpr.cc	2022-02-05 12:47:54.939664202 +0100
@@ -3413,7 +3413,10 @@ cxx_eval_binary_expression (const conste
       if (ctx->manifestly_const_eval
 	  && (flag_constexpr_fp_except
 	      || TREE_CODE (type) != REAL_TYPE))
-	r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
+	{
+	  auto ofcc = make_temp_override (folding_cxx_constexpr, true);
+	  r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
+	}
       else
 	r = fold_binary_loc (loc, code, type, lhs, rhs);
     }
--- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C.jj	2022-02-05 12:47:54.939664202 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C	2022-02-05 12:47:54.939664202 +0100
@@ -0,0 +1,132 @@
+// PR c++/89074
+// { dg-do compile { target c++14 } }
+
+int fn1 (void) { return 0; }
+int fn2 (void) { return 1; }
+
+constexpr bool
+f1 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  if (&a[1] == "foo")
+    return false;
+
+  if (&a[1] == &"foo"[4])
+    return false;
+
+  if (&"foo"[1] == &a[0])
+    return false;
+
+  if (&"foo"[3] == &a[4])
+    return false;
+
+  if (&a[0] == "foo")
+    return false;
+
+  // Pointer to start of one object (var) and end of another one (literal)
+  if (&a[0] == &"foo"[4])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f2 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  // Pointer to end of one object (var) and start of another one (literal)
+  if (&a[4] == "foo")		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+char v[] = { 1, 2, 3, 4 };
+
+constexpr bool
+f3 ()
+{
+  char a[] = { 1, 2, 3, 4 };
+
+  if (&a[1] == &v[1])
+    return false;
+
+  if (&a[0] == &v[3])
+    return false;
+
+  if (&a[2] == &v[4])
+    return false;
+
+  // Pointer to start of one object (automatic var) and end of another one (non-automagic var)
+  if (&a[0] == &v[4])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f4 ()
+{
+  char a[] = { 1, 2, 3, 4, 5 };
+
+  // Pointer to end of one object (automatic var) and start of another one (non-automagic var)
+  if (&a[5] == &v[0])		// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f5 ()
+{
+  if (fn1 != fn1)
+    return false;
+
+  if (fn1 == fn2)
+    return false;
+
+  if (&"abcde"[0] == &"edcba"[1])
+    return false;
+
+  if (&"abcde"[1] == &"edcba"[6])
+    return false;
+
+  // Pointer to start of one object (literal) and end of another one (literal)
+  if (&"abcde"[0] == &"edcba"[6])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f6 ()
+{
+  // Pointer to start of one object (literal) and end of another one (literal)
+  if (&"abcde"[6] == &"edcba"[0])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool
+f7 ()
+{
+  if (&"abcde"[3] == &"fabcde"[3])
+    return false;
+
+  // These could be suffix merged, with &"abcde"[0] == &"fabcde"[1].
+  if (&"abcde"[3] == &"fabcde"[4])	// { dg-error "is not a constant expression" }
+    return false;
+
+  return true;
+}
+
+constexpr bool a = f1 ();
+constexpr bool b = f2 ();
+constexpr bool c = f3 ();
+constexpr bool d = f4 ();
+constexpr bool e = f5 ();
+constexpr bool f = f6 ();
+constexpr bool g = f7 ();


	Jakub


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] c++, v4: Further address_compare fixes [PR89074]
  2022-02-05 12:17                           ` [PATCH] c++, v4: " Jakub Jelinek
@ 2022-02-05 13:54                             ` Jason Merrill
  0 siblings, 0 replies; 21+ messages in thread
From: Jason Merrill @ 2022-02-05 13:54 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches

On 2/5/22 07:17, Jakub Jelinek wrote:
> On Sat, Feb 05, 2022 at 12:02:14AM +0100, Jakub Jelinek via Gcc-patches wrote:
>> You mean for folding_cxx_constexpr ?  The code does that basically, with one
>> exception, the folding_initializer FUNCTION_DECL cmp FUNCTION_DECL case.
>> We don't track sizes of functions, so the size of 1 is just a hack to
>> pretend functions don't have zero size.  Some functions can have zero size
>> if they contain just __builtin_unreachable, but it is very rare.
>> But I guess I could move that
>>    if (folding_initializer
>>        && TREE_CODE (base0) == FUNCTION_DECL
>>        && TREE_CODE (base1) == FUNCTION_DECL)
>>      return 0;
>> above the size checking block and then indeed right after that do
>>    if (folding_cxx_constexpr && equal)
>>      return equal;
>> with a comment.
> 
> Here is an updated patch:

OK, thanks.

> 2022-02-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/89074
> 	PR c++/104033
> 	* fold-const.h (folding_initializer): Adjust comment.
> 	(folding_cxx_constexpr): Declare.
> 	* fold-const.cc (folding_initializer): Adjust comment.
> 	(folding_cxx_constexpr): New variable.
> 	(address_compare): Restrict the decl vs. STRING_CST
> 	or vice versa or STRING_CST vs. STRING_CST or
> 	is_global_var != is_global_var optimizations to !folding_cxx_constexpr.
> 	Punt for FUNCTION_DECLs with non-zero offsets.  If folding_initializer,
> 	assume non-aliased functions have non-zero size and have different
> 	addresses.  For folding_cxx_constexpr, punt on comparisons of start
> 	of some object and end of another one, regardless whether it is a decl
> 	or string literal.  Also punt for folding_cxx_constexpr on
> 	STRING_CST vs. STRING_CST comparisons if the two literals could be
> 	overlapping.
> 
> 	* constexpr.cc (cxx_eval_binary_expression): Temporarily set
> 	folding_cxx_constexpr.
> 
> 	* g++.dg/cpp1y/constexpr-89074-3.C: New test.
> 
> --- gcc/fold-const.h.jj	2022-02-04 18:30:34.695003975 +0100
> +++ gcc/fold-const.h	2022-02-05 12:47:54.935664258 +0100
> @@ -20,9 +20,16 @@ along with GCC; see the file COPYING3.
>   #ifndef GCC_FOLD_CONST_H
>   #define GCC_FOLD_CONST_H
>   
> -/* Non-zero if we are folding constants inside an initializer; zero
> -   otherwise.  */
> +/* Nonzero if we are folding constants inside an initializer or a C++
> +   manifestly-constant-evaluated context; zero otherwise.
> +   Should be used when folding in initializer enables additional
> +   optimizations.  */
>   extern int folding_initializer;
> +/* Nonzero if we are folding C++ manifestly-constant-evaluated context; zero
> +   otherwise.
> +   Should be used when certain constructs shouldn't be optimized
> +   during folding in that context.  */
> +extern bool folding_cxx_constexpr;
>   
>   /* Convert between trees and native memory representation.  */
>   extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
> --- gcc/fold-const.cc.jj	2022-02-04 18:30:34.695003975 +0100
> +++ gcc/fold-const.cc	2022-02-05 13:07:53.801996609 +0100
> @@ -86,9 +86,17 @@ along with GCC; see the file COPYING3.
>   #include "gimple-range.h"
>   
>   /* Nonzero if we are folding constants inside an initializer or a C++
> -   manifestly-constant-evaluated context; zero otherwise.  */
> +   manifestly-constant-evaluated context; zero otherwise.
> +   Should be used when folding in initializer enables additional
> +   optimizations.  */
>   int folding_initializer = 0;
>   
> +/* Nonzero if we are folding C++ manifestly-constant-evaluated context; zero
> +   otherwise.
> +   Should be used when certain constructs shouldn't be optimized
> +   during folding in that context.  */
> +bool folding_cxx_constexpr = false;
> +
>   /* The following constants represent a bit based encoding of GCC's
>      comparison operators.  This encoding simplifies transformations
>      on relational comparison operators, such as AND and OR.  */
> @@ -16572,6 +16580,7 @@ tree_nonzero_bits (const_tree t)
>   
>   /* Helper function for address compare simplifications in match.pd.
>      OP0 and OP1 are ADDR_EXPR operands being compared by CODE.
> +   TYPE is the type of comparison operands.
>      BASE0, BASE1, OFF0 and OFF1 are set by the function.
>      GENERIC is true if GENERIC folding and false for GIMPLE folding.
>      Returns 0 if OP0 is known to be unequal to OP1 regardless of OFF{0,1},
> @@ -16648,44 +16657,66 @@ address_compare (tree_code code, tree ty
>     if (code != EQ_EXPR && code != NE_EXPR)
>       return 2;
>   
> +  /* At this point we know (or assume) the two pointers point at
> +     different objects.  */
>     HOST_WIDE_INT ioff0 = -1, ioff1 = -1;
>     off0.is_constant (&ioff0);
>     off1.is_constant (&ioff1);
> -  if ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
> -       || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
> -       || (TREE_CODE (base0) == STRING_CST
> -	   && TREE_CODE (base1) == STRING_CST
> -	   && ioff0 >= 0 && ioff1 >= 0
> -	   && ioff0 < TREE_STRING_LENGTH (base0)
> -	   && ioff1 < TREE_STRING_LENGTH (base1)
> -	  /* This is a too conservative test that the STRING_CSTs
> -	     will not end up being string-merged.  */
> -	   && strncmp (TREE_STRING_POINTER (base0) + ioff0,
> -		       TREE_STRING_POINTER (base1) + ioff1,
> -		       MIN (TREE_STRING_LENGTH (base0) - ioff0,
> -			    TREE_STRING_LENGTH (base1) - ioff1)) != 0))
> -    ;
> -  else if (!DECL_P (base0) || !DECL_P (base1))
> +  /* Punt on non-zero offsets from functions.  */
> +  if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
> +      || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
>       return 2;
> -  /* If this is a pointer comparison, ignore for now even
> -     valid equalities where one pointer is the offset zero
> -     of one object and the other to one past end of another one.  */
> -  else if (!folding_initializer && !INTEGRAL_TYPE_P (type))
> -    ;
> -  /* Assume that automatic variables can't be adjacent to global
> -     variables.  */
> -  else if (is_global_var (base0) != is_global_var (base1))
> -    ;
> +  /* Or if the bases are neither decls nor string literals.  */
> +  if (!DECL_P (base0) && TREE_CODE (base0) != STRING_CST)
> +    return 2;
> +  if (!DECL_P (base1) && TREE_CODE (base1) != STRING_CST)
> +    return 2;
> +  /* For initializers, assume addresses of different functions are
> +     different.  */
> +  if (folding_initializer
> +      && TREE_CODE (base0) == FUNCTION_DECL
> +      && TREE_CODE (base1) == FUNCTION_DECL)
> +    return 0;
> +
> +  /* Compute whether one address points to the start of one
> +     object and another one to the end of another one.  */
> +  poly_int64 size0 = 0, size1 = 0;
> +  if (TREE_CODE (base0) == STRING_CST)
> +    {
> +      if (ioff0 < 0 || ioff0 > TREE_STRING_LENGTH (base0))
> +	equal = 2;
> +      else
> +	size0 = TREE_STRING_LENGTH (base0);
> +    }
> +  else if (TREE_CODE (base0) == FUNCTION_DECL)
> +    size0 = 1;
>     else
>       {
>         tree sz0 = DECL_SIZE_UNIT (base0);
> +      if (!tree_fits_poly_int64_p (sz0))
> +	equal = 2;
> +      else
> +	size0 = tree_to_poly_int64 (sz0);
> +    }
> +  if (TREE_CODE (base1) == STRING_CST)
> +    {
> +      if (ioff1 < 0 || ioff1 > TREE_STRING_LENGTH (base1))
> +	equal = 2;
> +      else
> +	size1 = TREE_STRING_LENGTH (base1);
> +    }
> +  else if (TREE_CODE (base1) == FUNCTION_DECL)
> +    size1 = 1;
> +  else
> +    {
>         tree sz1 = DECL_SIZE_UNIT (base1);
> -      /* If sizes are unknown, e.g. VLA or not representable, punt.  */
> -      if (!tree_fits_poly_int64_p (sz0) || !tree_fits_poly_int64_p (sz1))
> -	return 2;
> -
> -      poly_int64 size0 = tree_to_poly_int64 (sz0);
> -      poly_int64 size1 = tree_to_poly_int64 (sz1);
> +      if (!tree_fits_poly_int64_p (sz1))
> +	equal = 2;
> +      else
> +	size1 = tree_to_poly_int64 (sz1);
> +    }
> +  if (equal == 0)
> +    {
>         /* If one offset is pointing (or could be) to the beginning of one
>   	 object and the other is pointing to one past the last byte of the
>   	 other object, punt.  */
> @@ -16701,7 +16732,63 @@ address_compare (tree_code code, tree ty
>   	  && (known_ne (off0, 0)
>   	      || (known_ne (size0, 0) && known_ne (size1, 0))))
>   	equal = 0;
> -     }
> +    }
> +
> +  /* At this point, equal is 2 if either one or both pointers are out of
> +     bounds of their object, or one points to start of its object and the
> +     other points to end of its object.  This is unspecified behavior
> +     e.g. in C++.  Otherwise equal is 0.  */
> +  if (folding_cxx_constexpr && equal)
> +    return equal;
> +
> +  /* When both pointers point to string literals, even when equal is 0,
> +     due to tail merging of string literals the pointers might be the same.  */
> +  if (TREE_CODE (base0) == STRING_CST && TREE_CODE (base1) == STRING_CST)
> +    {
> +      if (ioff0 < 0
> +	  || ioff1 < 0
> +	  || ioff0 > TREE_STRING_LENGTH (base0)
> +	  || ioff1 > TREE_STRING_LENGTH (base1))
> +	return 2;
> +
> +      /* If the bytes in the string literals starting at the pointers
> +	 differ, the pointers need to be different.  */
> +      if (memcmp (TREE_STRING_POINTER (base0) + ioff0,
> +		  TREE_STRING_POINTER (base1) + ioff1,
> +		  MIN (TREE_STRING_LENGTH (base0) - ioff0,
> +		       TREE_STRING_LENGTH (base1) - ioff1)) == 0)
> +	{
> +	  HOST_WIDE_INT ioffmin = MIN (ioff0, ioff1);
> +	  if (memcmp (TREE_STRING_POINTER (base0) + ioff0 - ioffmin,
> +		      TREE_STRING_POINTER (base1) + ioff1 - ioffmin,
> +		      ioffmin) == 0)
> +	    /* If even the bytes in the string literal before the
> +	       pointers are the same, the string literals could be
> +	       tail merged.  */
> +	    return 2;
> +	}
> +      return 0;
> +    }
> +
> +  if (folding_cxx_constexpr)
> +    return 0;
> +
> +  /* If this is a pointer comparison, ignore for now even
> +     valid equalities where one pointer is the offset zero
> +     of one object and the other to one past end of another one.  */
> +  if (!INTEGRAL_TYPE_P (type))
> +    return 0;
> +
> +  /* Assume that string literals can't be adjacent to variables
> +     (automatic or global).  */
> +  if (TREE_CODE (base0) == STRING_CST || TREE_CODE (base1) == STRING_CST)
> +    return 0;
> +
> +  /* Assume that automatic variables can't be adjacent to global
> +     variables.  */
> +  if (is_global_var (base0) != is_global_var (base1))
> +    return 0;
> +
>     return equal;
>   }
>   
> --- gcc/cp/constexpr.cc.jj	2022-02-04 14:36:54.597610997 +0100
> +++ gcc/cp/constexpr.cc	2022-02-05 12:47:54.939664202 +0100
> @@ -3413,7 +3413,10 @@ cxx_eval_binary_expression (const conste
>         if (ctx->manifestly_const_eval
>   	  && (flag_constexpr_fp_except
>   	      || TREE_CODE (type) != REAL_TYPE))
> -	r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
> +	{
> +	  auto ofcc = make_temp_override (folding_cxx_constexpr, true);
> +	  r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
> +	}
>         else
>   	r = fold_binary_loc (loc, code, type, lhs, rhs);
>       }
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C.jj	2022-02-05 12:47:54.939664202 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-89074-3.C	2022-02-05 12:47:54.939664202 +0100
> @@ -0,0 +1,132 @@
> +// PR c++/89074
> +// { dg-do compile { target c++14 } }
> +
> +int fn1 (void) { return 0; }
> +int fn2 (void) { return 1; }
> +
> +constexpr bool
> +f1 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  if (&a[1] == "foo")
> +    return false;
> +
> +  if (&a[1] == &"foo"[4])
> +    return false;
> +
> +  if (&"foo"[1] == &a[0])
> +    return false;
> +
> +  if (&"foo"[3] == &a[4])
> +    return false;
> +
> +  if (&a[0] == "foo")
> +    return false;
> +
> +  // Pointer to start of one object (var) and end of another one (literal)
> +  if (&a[0] == &"foo"[4])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f2 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  // Pointer to end of one object (var) and start of another one (literal)
> +  if (&a[4] == "foo")		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +char v[] = { 1, 2, 3, 4 };
> +
> +constexpr bool
> +f3 ()
> +{
> +  char a[] = { 1, 2, 3, 4 };
> +
> +  if (&a[1] == &v[1])
> +    return false;
> +
> +  if (&a[0] == &v[3])
> +    return false;
> +
> +  if (&a[2] == &v[4])
> +    return false;
> +
> +  // Pointer to start of one object (automatic var) and end of another one (non-automagic var)
> +  if (&a[0] == &v[4])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f4 ()
> +{
> +  char a[] = { 1, 2, 3, 4, 5 };
> +
> +  // Pointer to end of one object (automatic var) and start of another one (non-automagic var)
> +  if (&a[5] == &v[0])		// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f5 ()
> +{
> +  if (fn1 != fn1)
> +    return false;
> +
> +  if (fn1 == fn2)
> +    return false;
> +
> +  if (&"abcde"[0] == &"edcba"[1])
> +    return false;
> +
> +  if (&"abcde"[1] == &"edcba"[6])
> +    return false;
> +
> +  // Pointer to start of one object (literal) and end of another one (literal)
> +  if (&"abcde"[0] == &"edcba"[6])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f6 ()
> +{
> +  // Pointer to start of one object (literal) and end of another one (literal)
> +  if (&"abcde"[6] == &"edcba"[0])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool
> +f7 ()
> +{
> +  if (&"abcde"[3] == &"fabcde"[3])
> +    return false;
> +
> +  // These could be suffix merged, with &"abcde"[0] == &"fabcde"[1].
> +  if (&"abcde"[3] == &"fabcde"[4])	// { dg-error "is not a constant expression" }
> +    return false;
> +
> +  return true;
> +}
> +
> +constexpr bool a = f1 ();
> +constexpr bool b = f2 ();
> +constexpr bool c = f3 ();
> +constexpr bool d = f4 ();
> +constexpr bool e = f5 ();
> +constexpr bool f = f6 ();
> +constexpr bool g = f7 ();
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-02-05 13:55 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-06  9:24 [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074] Jakub Jelinek
2022-01-10 14:10 ` Richard Biener
2022-01-11  3:24   ` Andrew Pinski
2022-01-13 17:35 ` Patch ping (Re: [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074]) Jakub Jelinek
2022-01-13 21:18 ` [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074] Jason Merrill
2022-01-18 10:17   ` [PATCH] c++: Further address_compare fixes [PR89074] Jakub Jelinek
2022-01-18 12:30     ` Jakub Jelinek
2022-01-18 16:25     ` Jason Merrill
2022-01-18 16:40       ` Jakub Jelinek
2022-01-18 16:56         ` Jason Merrill
2022-02-03 15:52         ` [PATCH] c++, v2: " Jakub Jelinek
2022-02-03 20:07           ` Jason Merrill
2022-02-03 20:33             ` Jakub Jelinek
2022-02-03 21:04               ` Jason Merrill
2022-02-03 21:18                 ` Jakub Jelinek
2022-02-03 21:34                   ` Jason Merrill
2022-02-04 13:41                     ` [PATCH] c++, v3: " Jakub Jelinek
2022-02-04 21:42                       ` Jason Merrill
2022-02-04 23:02                         ` Jakub Jelinek
2022-02-05 12:17                           ` [PATCH] c++, v4: " Jakub Jelinek
2022-02-05 13:54                             ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).