Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][bytecode] Handle __builtin_memcmp #119544

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

tbaederr
Copy link
Contributor

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

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

2 Files Affected:

  • (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+30-3)
  • (modified) clang/test/AST/ByteCode/builtin-functions.cpp (+21)
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index a0de193ec32a2f..a567a955bb536f 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -190,6 +190,12 @@ static bool interp__builtin_is_constant_evaluated(InterpState &S, CodePtr OpPC,
   return true;
 }
 
+/// Determine if T is a character type for which we guarantee that
+/// sizeof(T) == 1.
+static bool isOneByteCharacterType(QualType T) {
+  return T->isCharType() || T->isChar8Type();
+}
+
 static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
                                    const InterpFrame *Frame,
                                    const Function *Func, const CallExpr *Call) {
@@ -197,11 +203,13 @@ static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
   const Pointer &A = getParam<Pointer>(Frame, 0);
   const Pointer &B = getParam<Pointer>(Frame, 1);
 
-  if (ID == Builtin::BIstrcmp || ID == Builtin::BIstrncmp)
+  if (ID == Builtin::BIstrcmp || ID == Builtin::BIstrncmp ||
+      ID == Builtin::BImemcmp)
     diagnoseNonConstexprBuiltin(S, OpPC, ID);
 
   uint64_t Limit = ~static_cast<uint64_t>(0);
-  if (ID == Builtin::BIstrncmp || ID == Builtin::BI__builtin_strncmp)
+  if (ID == Builtin::BIstrncmp || ID == Builtin::BI__builtin_strncmp ||
+      ID == Builtin::BI__builtin_memcmp || ID == Builtin::BImemcmp)
     Limit = peekToAPSInt(S.Stk, *S.getContext().classify(Call->getArg(2)))
                 .getZExtValue();
 
@@ -219,6 +227,23 @@ static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
   assert(A.getFieldDesc()->isPrimitiveArray());
   assert(B.getFieldDesc()->isPrimitiveArray());
 
+  bool StopAtZero =
+      (ID == Builtin::BIstrcmp || ID == Builtin::BI__builtin_strcmp ||
+       ID == Builtin::BIstrncmp || ID == Builtin::BI__builtin_strncmp);
+  bool IsRawByte = ID == Builtin::BImemcmp || ID == Builtin::BI__builtin_memcmp;
+
+  if (IsRawByte &&
+      (!isOneByteCharacterType(A.getFieldDesc()->getElemQualType()) ||
+       !isOneByteCharacterType(B.getFieldDesc()->getElemQualType()))) {
+    QualType CharTy1 = A.getFieldDesc()->getElemQualType();
+    QualType CharTy2 = B.getFieldDesc()->getElemQualType();
+    S.FFDiag(S.Current->getSource(OpPC),
+             diag::note_constexpr_memcmp_unsupported)
+        << ("'" + S.getASTContext().BuiltinInfo.getName(ID) + "'").str()
+        << CharTy1 << CharTy2;
+    return false;
+  }
+
   unsigned IndexA = A.getIndex();
   unsigned IndexB = B.getIndex();
   int32_t Result = 0;
@@ -243,7 +268,7 @@ static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
       Result = -1;
       break;
     }
-    if (CA == 0 || CB == 0)
+    if (StopAtZero && (CA == 0 || CB == 0))
       break;
   }
 
@@ -1904,6 +1929,8 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
   case Builtin::BIstrcmp:
   case Builtin::BI__builtin_strncmp:
   case Builtin::BIstrncmp:
+  case Builtin::BI__builtin_memcmp:
+  case Builtin::BImemcmp:
     if (!interp__builtin_strcmp(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 7dd08cb5fa1c35..485652867a44ec 100644
--- a/clang/test/AST/ByteCode/builtin-functions.cpp
+++ b/clang/test/AST/ByteCode/builtin-functions.cpp
@@ -1208,3 +1208,24 @@ namespace BuiltinMemcpy {
   static_assert(memcpyTypeRem() == 12); // both-error {{not an integral constant expression}} \
                                         // both-note {{in call to}}
 }
+
+namespace Memcmp {
+  constexpr unsigned char ku00fe00[] = {0x00, 0xfe, 0x00};
+  constexpr unsigned char ku00feff[] = {0x00, 0xfe, 0xff};
+  constexpr signed char ks00fe00[] = {0, -2, 0};
+  constexpr signed char ks00feff[] = {0, -2, -1};
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ku00fe00, ks00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ku00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 99) == -1);
+
+  struct Bool3Tuple { bool bb[3]; };
+  constexpr Bool3Tuple kb000100 = {{false, true, false}};
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100.bb, 1) == 0); // both-error {{constant}} \
+                                                                                        // both-note {{not supported}}
+}

@tbaederr tbaederr force-pushed the builtin-bit-cast22 branch 4 times, most recently from d4518ea to 3e5ca2b Compare December 12, 2024 06:55
@tbaederr tbaederr merged commit 8713914 into llvm:main Dec 12, 2024
8 checks passed
@aaronpuchert
Copy link
Member

Check out https://lab.llvm.org/buildbot/#/builders/13/builds/4041:

RUN: at line 1: /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/stage1/bin/clang -cc1 -internal-isystem /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/stage1/lib/clang/20/include -nostdsysteminc -Wno-string-plus-int -fexperimental-new-constant-interpreter /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/llvm/clang/test/AST/ByteCode/builtin-functions.cpp -verify=expected,both
+ /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/stage1/bin/clang -cc1 -internal-isystem /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/stage1/lib/clang/20/include -nostdsysteminc -Wno-string-plus-int -fexperimental-new-constant-interpreter /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/llvm/clang/test/AST/ByteCode/builtin-functions.cpp -verify=expected,both
error: 'both-error' diagnostics seen but not expected: 
  File /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/llvm/clang/test/AST/ByteCode/builtin-functions.cpp Line 1232: static assertion failed due to requirement '__builtin_memcmp(ku00feff, ks00fe00, 2) == 0'
  File /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/llvm/clang/test/AST/ByteCode/builtin-functions.cpp Line 1235: static assertion failed due to requirement '__builtin_memcmp(ks00feff, ku00fe00, 2) == 0'
  File /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/llvm/clang/test/AST/ByteCode/builtin-functions.cpp Line 1238: static assertion failed due to requirement '__builtin_memcmp(ks00fe00, ks00feff, 2) == 0'
error: 'both-note' diagnostics seen but not expected: 
  File /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/llvm/clang/test/AST/ByteCode/builtin-functions.cpp Line 1232: expression evaluates to '1 == 0'
  File /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/llvm/clang/test/AST/ByteCode/builtin-functions.cpp Line 1235: expression evaluates to '1 == 0'
  File /opt/llvm-buildbot/home/solaris11-sparcv9/clang-solaris11-sparcv9/llvm/clang/test/AST/ByteCode/builtin-functions.cpp Line 1238: expression evaluates to '-1 == 0'
8 errors generated.

@nikic
Copy link
Contributor

nikic commented Dec 13, 2024

We're seeing the same issue on s390x. Big endian not handled correctly?

@tbaederr
Copy link
Contributor Author

Yup, see #119851

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants