https://github.com/Ved235 created https://github.com/llvm/llvm-project/pull/230869
Closes #229810 ### Description Modified both `ExprConstant.cpp` and `InterpBuiltin.cpp` to remove the casting check. The check evaluated whether the input value provided to `strchr()` changed its value or not after being converted to a char This approach isn't correct as if it is a signed char of 8 bits, an input of 128 would interpreted as -128 which would result in the check being false. However, the byte representation of both values is still the same (0x80) which is what should be searched for. (The standard says to search for the value after converting the int to a char) ### Testing Added new tests in `constexpr-string.cpp` and `builtin-functions.cpp`. >From 3ae1762f92c0dba16b54c337fe8341084858c34d Mon Sep 17 00:00:00 2001 From: Vedant <[email protected]> Date: Sat, 10 Oct 2026 23:29:16 +0100 Subject: [PATCH] Fix type conversion in constant evaluation of strchr --- clang/lib/AST/ByteCode/InterpBuiltin.cpp | 16 ---------------- clang/lib/AST/ExprConstant.cpp | 7 ------- clang/test/AST/ByteCode/builtin-functions.cpp | 12 ++++++++---- clang/test/SemaCXX/constexpr-string.cpp | 12 ++++++++---- 4 files changed, 16 insertions(+), 31 deletions(-) diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp index 4a5c691d390a7..53d543928efa3 100644 --- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp +++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp @@ -2378,22 +2378,6 @@ static bool interp__builtin_memchr(InterpState &S, CodePtr OpPC, if (!isReadable(Ptr)) return false; - if (ID == Builtin::BIstrchr || ID == Builtin::BI__builtin_strchr) { - int64_t DesiredTrunc; - if (S.getASTContext().CharTy->isSignedIntegerType()) - DesiredTrunc = - Desired.trunc(S.getASTContext().getCharWidth()).getSExtValue(); - else - DesiredTrunc = - Desired.trunc(S.getASTContext().getCharWidth()).getZExtValue(); - // strchr compares directly to the passed integer, and therefore - // always fails if given an int that is not a char. - if (Desired != DesiredTrunc) { - S.Stk.push<Pointer>(); - return true; - } - } - uint64_t DesiredVal; if (ID == Builtin::BIwmemchr || ID == Builtin::BI__builtin_wmemchr || ID == Builtin::BIwcschr || ID == Builtin::BI__builtin_wcschr) { diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 8551f4c9ae279..f5b562a4eab67 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -10877,13 +10877,6 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, switch (BuiltinOp) { case Builtin::BIstrchr: case Builtin::BI__builtin_strchr: - // strchr compares directly to the passed integer, and therefore - // always fails if given an int that is not a char. - if (!APSInt::isSameValue(HandleIntToIntCast(Info, E, CharTy, - E->getArg(1)->getType(), - Desired), - Desired)) - return ZeroInitialization(E); StopAtNull = true; [[fallthrough]]; case Builtin::BImemchr: diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp index 211c3e98d161c..ae07bae277962 100644 --- a/clang/test/AST/ByteCode/builtin-functions.cpp +++ b/clang/test/AST/ByteCode/builtin-functions.cpp @@ -1825,11 +1825,15 @@ namespace Strchr { static_assert(__builtin_strchr(kStr, 'd') == nullptr); static_assert(__builtin_strchr(kStr, 'e') == nullptr); static_assert(__builtin_strchr(kStr, '\0') == kStr + 5); - static_assert(__builtin_strchr(kStr, 'a' + 256) == nullptr); - static_assert(__builtin_strchr(kStr, 'a' - 256) == nullptr); + static_assert(__builtin_strchr(kStr, 256) == kStr + 5); + static_assert(__builtin_strchr(kStr, -256) == kStr + 5); + static_assert(__builtin_strchr(kStr, 'a' + 256) == kStr); + static_assert(__builtin_strchr(kStr, 'a' - 256) == kStr); static_assert(__builtin_strchr(kStr, '\xff') == kStr + 4); - static_assert(__builtin_strchr(kStr, '\xff' + 256) == nullptr); - static_assert(__builtin_strchr(kStr, '\xff' - 256) == nullptr); + static_assert(__builtin_strchr(kStr, '\xff' + 256) == kStr + 4); + static_assert(__builtin_strchr(kStr, '\xff' - 256) == kStr + 4); + constexpr char kStr2[] = "\x80\x41"; + static_assert(__builtin_strchr(kStr2, 128) == kStr2); static_assert(__builtin_strchr(kFoo, 'o') == kFoo + 1); static_assert(__builtin_strchr(kFoo, 'x') == nullptr); // both-error {{not an integral constant}} \ // both-note {{dereferenced one-past-the-end}} diff --git a/clang/test/SemaCXX/constexpr-string.cpp b/clang/test/SemaCXX/constexpr-string.cpp index b49979bbc8ca0..b55118ffc9e2a 100644 --- a/clang/test/SemaCXX/constexpr-string.cpp +++ b/clang/test/SemaCXX/constexpr-string.cpp @@ -369,11 +369,15 @@ namespace StrchrEtc { static_assert(__builtin_strchr(kStr, 'd') == nullptr); static_assert(__builtin_strchr(kStr, 'e') == nullptr); static_assert(__builtin_strchr(kStr, '\0') == kStr + 5); - static_assert(__builtin_strchr(kStr, 'a' + 256) == nullptr); - static_assert(__builtin_strchr(kStr, 'a' - 256) == nullptr); + static_assert(__builtin_strchr(kStr, 256) == kStr + 5); + static_assert(__builtin_strchr(kStr, -256) == kStr + 5); + static_assert(__builtin_strchr(kStr, 'a' + 256) == kStr); + static_assert(__builtin_strchr(kStr, 'a' - 256) == kStr); static_assert(__builtin_strchr(kStr, '\xff') == kStr + 4); - static_assert(__builtin_strchr(kStr, '\xff' + 256) == nullptr); - static_assert(__builtin_strchr(kStr, '\xff' - 256) == nullptr); + static_assert(__builtin_strchr(kStr, '\xff' + 256) == kStr + 4); + static_assert(__builtin_strchr(kStr, '\xff' - 256) == kStr + 4); + constexpr char kStr2[] = "\x80\x41"; + static_assert(__builtin_strchr(kStr2, 128) == kStr2); static_assert(__builtin_strchr(kFoo, 'o') == kFoo + 1); static_assert(__builtin_strchr(kFoo, 'x') == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}} static_assert(__builtin_strchr(nullptr, 'x') == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced null}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
