Re: [PATCH] c++, v4: Further address_compare fixes [PR89074]

[Jason Merrill <jason@redhat.com>]

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
>