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>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] c++: Reject in constant evaluation address comparisons of start of one var and end of another [PR89074]
Date: Thu, 6 Jan 2022 10:24:16 +0100	[thread overview]
Message-ID: <20220106092416.GX2646553@tucnak> (raw)

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


             reply	other threads:[~2022-01-06  9:24 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-06  9:24 Jakub Jelinek [this message]
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

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=20220106092416.GX2646553@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /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).