llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Vedant Singhal (Ved235) <details> <summary>Changes</summary> 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`. --- Full diff: https://github.com/llvm/llvm-project/pull/230869.diff 4 Files Affected: - (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (-16) - (modified) clang/lib/AST/ExprConstant.cpp (-7) - (modified) clang/test/AST/ByteCode/builtin-functions.cpp (+8-4) - (modified) clang/test/SemaCXX/constexpr-string.cpp (+8-4) ``````````diff 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}} `````````` </details> https://github.com/llvm/llvm-project/pull/230869 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
