[clang] [clang][bytecode] Handle __builtin_bcmp (PR #119678)

[Timm Baeder via cfe-commits]

https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/119678

... the same as `__builtin_memcmp`. Also fix a bug we still had when we couldn't find a difference in the two inputs after `Size` bytes.

>From 3a480676781925badff8b88c3833f083fdc3dcbd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 12 Dec 2024 10:17:45 +0100
Subject: [PATCH] [clang][bytecode] Handle __builtin_bcmp

... the same as `__builtin_memcmp`. Also fix a bug we still had when we
couldn't find a difference in the two inputs after `Size` bytes.
---
 clang/lib/AST/ByteCode/InterpBuiltin.cpp      | 37 +++++++++++++++----
 clang/test/AST/ByteCode/builtin-functions.cpp | 15 ++++++++
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index a343280b5ce50f..21baedf832eeac 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -1917,7 +1917,7 @@ static bool interp__builtin_memcmp(InterpState &S, CodePtr OpPC,
   const APSInt &Size =
       peekToAPSInt(S.Stk, *S.getContext().classify(Call->getArg(2)));
 
-  if (ID == Builtin::BImemcmp)
+  if (ID == Builtin::BImemcmp || ID == Builtin::BIbcmp)
     diagnoseNonConstexprBuiltin(S, OpPC, ID);
 
   if (Size.isZero()) {
@@ -1952,15 +1952,34 @@ static bool interp__builtin_memcmp(InterpState &S, CodePtr OpPC,
                                   BufferB.byteSize().getQuantity());
   size_t CmpSize =
       std::min(MinBufferSize, static_cast<size_t>(Size.getZExtValue()));
-  int Result = std::memcmp(BufferA.Data.get(), BufferB.Data.get(), CmpSize);
-  if (Result == 0)
+
+  for (size_t I = 0; I != CmpSize; ++I) {
+    std::byte A = BufferA.Data[I];
+    std::byte B = BufferB.Data[I];
+
+    if (A < B) {
+      pushInteger(S, -1, Call->getType());
+      return true;
+    } else if (A > B) {
+      pushInteger(S, 1, Call->getType());
+      return true;
+    }
+  }
+
+  // We compared CmpSize bytes above. If the limiting factor was the Size
+  // passed, we're done and the result is equality (0).
+  if (Size.getZExtValue() <= CmpSize) {
     pushInteger(S, 0, Call->getType());
-  else if (Result < 0)
-    pushInteger(S, -1, Call->getType());
-  else
-    pushInteger(S, 1, Call->getType());
+    return true;
+  }
 
-  return true;
+  // However, if we read all the available bytes but were instructed to read
+  // even more, diagnose this as a "read of dereferenced one-past-the-end
+  // pointer". This is what would happen if we called CheckRead() on every array
+  // element.
+  S.FFDiag(S.Current->getSource(OpPC), diag::note_constexpr_access_past_end)
+      << AK_Read << S.Current->getRange(OpPC);
+  return false;
 }
 
 bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
@@ -2438,6 +2457,8 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
 
   case Builtin::BI__builtin_memcmp:
   case Builtin::BImemcmp:
+  case Builtin::BI__builtin_bcmp:
+  case Builtin::BIbcmp:
     if (!interp__builtin_memcmp(S, OpPC, Frame, F, Call))
       return false;
     break;
diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp
index 83caa1d03df3a3..4ee24646286fa8 100644
--- a/clang/test/AST/ByteCode/builtin-functions.cpp
+++ b/clang/test/AST/ByteCode/builtin-functions.cpp
@@ -1255,4 +1255,19 @@ namespace Memcmp {
                                                               // both-note {{not supported}}
   static_assert(__builtin_memcmp("", &incomplete, 1u) == 42); // both-error {{not an integral constant}} \
                                                               // both-note {{not supported}}
+
+  static_assert(__builtin_memcmp(u8"abab\0banana", u8"abab\0banana", 100) == 0); // both-error {{not an integral constant}} \
+                                                                                 // both-note {{dereferenced one-past-the-end}}
+
+  static_assert(__builtin_bcmp("abaa", "abba", 3) != 0);
+  static_assert(__builtin_bcmp("abaa", "abba", 2) == 0);
+  static_assert(__builtin_bcmp("a\203", "a", 2) != 0);
+  static_assert(__builtin_bcmp("a\203", "a\003", 2) != 0);
+  static_assert(__builtin_bcmp(0, 0, 0) == 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0banana", 100) == 0); // both-error {{not an integral constant}}\
+                                                                           // both-note {{dereferenced one-past-the-end}}
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 100) != 0); // FIXME: Should we reject this?
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 7) != 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 6) != 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 5) == 0);
 }