Eval string one past end reland

[hnrklssn]

Relands #137078 after updating clang/test/AST/ByteCode/cxx20.cpp to account for diagnostic outputs that differ between Linux and macOS.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Henrik G. Olsson (hnrklssn)

Changes

Relands #137078 after updating clang/test/AST/ByteCode/cxx20.cpp to account for diagnostic outputs that differ between Linux and macOS.

---

Full diff: https://github.com/llvm/llvm-project/pull/137091.diff

3 Files Affected:

• (modified) clang/lib/AST/ExprConstant.cpp (+7-2)
• (modified) clang/test/AST/ByteCode/cxx20.cpp (+8)
• (modified) clang/test/SemaCXX/constant-expression-cxx11.cpp (+2)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index f598ef5929aa4..7c933f47bf7f0 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2232,10 +2232,15 @@ static bool ArePotentiallyOverlappingStringLiterals(const EvalInfo &Info,
   // within RHS. We don't need to look at the characters of one string that
   // would appear before the start of the other string if they were merged.
   CharUnits Offset = RHS.Offset - LHS.Offset;
-  if (Offset.isNegative())
+  if (Offset.isNegative()) {
+    if (LHSString.Bytes.size() < (size_t)-Offset.getQuantity())
+      return false;
     LHSString.Bytes = LHSString.Bytes.drop_front(-Offset.getQuantity());
-  else
+  } else {
+    if (RHSString.Bytes.size() < (size_t)Offset.getQuantity())
+      return false;
     RHSString.Bytes = RHSString.Bytes.drop_front(Offset.getQuantity());
+  }
 
   bool LHSIsLonger = LHSString.Bytes.size() > RHSString.Bytes.size();
   StringRef Longer = LHSIsLonger ? LHSString.Bytes : RHSString.Bytes;
diff --git a/clang/test/AST/ByteCode/cxx20.cpp b/clang/test/AST/ByteCode/cxx20.cpp
index 42e6ae33e92e4..69a53a567fa41 100644
--- a/clang/test/AST/ByteCode/cxx20.cpp
+++ b/clang/test/AST/ByteCode/cxx20.cpp
@@ -119,6 +119,14 @@ constexpr auto b3 = name1() == name1(); // ref-error {{must be initialized by a
 constexpr auto b4 = name1() == name2();
 static_assert(!b4);
 
+constexpr auto bar(const char *p) { return p + __builtin_strlen(p); }
+constexpr auto b5 = bar(p1) == p1;
+static_assert(!b5);
+constexpr auto b6 = bar(p1) == ""; // ref-error {{must be initialized by a constant expression}} \
+                                   // ref-note {{comparison of addresses of potentially overlapping literals}}
+constexpr auto b7 = bar(p1) + 1 == ""; // both-error {{must be initialized by a constant expression}} \
+                                       // both-note-re {{comparison against pointer '&"test1"[6]{{( \+ 1)?}}' that points past the end of a complete object has unspecified value}}
+
 namespace UninitializedFields {
   class A {
   public:
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 28016da925ef9..dc8f4bf1666ee 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2203,6 +2203,8 @@ namespace BuiltinStrlen {
   static_assert(__builtin_strlen("foo") == 3, "");
   static_assert(__builtin_strlen("foo\0quux") == 3, "");
   static_assert(__builtin_strlen("foo\0quux" + 4) == 4, "");
+  static_assert(__builtin_strlen("foo") + 1 + "foo" == "foo", ""); // expected-error {{static assertion expression is not an integral constant expression}}
+  // expected-note@-1 {{comparison against pointer '&"foo"[4]' that points past the end of a complete object has unspecified value}}
 
   constexpr bool check(const char *p) {
     return __builtin_strlen(p) == 3 &&

[zygoloid]

Do you know where that + 1 is coming from? It looks wrong. I assume it's this, but the IsOnePastTheEnd flag is never supposed to be set for an lvalue whose path ends in an array indexing.

Is this maybe a difference between the constant evaluator and the bytecode interpreter? (If so, we should probably track that with different ref-note and expected-note expectations.)

[hnrklssn]

My original patch had different ref-note and expected-note diagnostics, because that is the behaviour I get locally on macOS. The Linux CI does not seem to get the + 1 for the bytecode interpreter though, which is what caused me to revert. I'll dig a bit deeper and see if I can figure out what's going on on my machine.

[zygoloid]

Ah, I bet you don't have this fix locally: 6196b4e

I think you can just remove the + 1.

[hnrklssn]

Ah yeah, that's probably it