public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Jason Merrill <jason@redhat.com>, Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] c++, v2: Further address_compare fixes [PR89074]
Date: Thu, 3 Feb 2022 16:52:37 +0100	[thread overview]
Message-ID: <20220203155237.GR2646553@tucnak> (raw)
In-Reply-To: <20220118164048.GD2646553@tucnak>

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


  parent reply	other threads:[~2022-02-03 15:52 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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         ` Jakub Jelinek [this message]
2022-02-03 20:07           ` [PATCH] c++, v2: " 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220203155237.GR2646553@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).