https://github.com/cavdarahmet created https://github.com/llvm/llvm-project/pull/211403
The header <ctype.h> declares character classification and conversion functions (isalpha, toupper, etc.) that take their argument as an int, which must be representable as an unsigned char or equal EOF. Passing a plain (signed) char holding a non-ASCII character is undefined behavior, since it is implicitly converted to a negative int value. Diagnose this the same way as the existing signed-char-to-integer cases already caught by this check. Since hasArgument() strips implicit casts off the matched argument, this case is matched directly on the (uncast) argument's type instead of going through charCastExpression(), which relies on an explicit cast node being present. Fixes #63280 >From 170cd86e94ad764a32287220672a39a47b9446bf Mon Sep 17 00:00:00 2001 From: Ahmet <[email protected]> Date: Thu, 23 Jul 2026 01:31:54 +0300 Subject: [PATCH] [clang-tidy] Catch <cctype> function args in bugprone-signed-char-misuse The header <ctype.h> declares character classification and conversion functions (isalpha, toupper, etc.) that take their argument as an int, which must be representable as an unsigned char or equal EOF. Passing a plain (signed) char holding a non-ASCII character is undefined behavior, since it is implicitly converted to a negative int value. Diagnose this the same way as the existing signed-char-to-integer cases already caught by this check. Since hasArgument() strips implicit casts off the matched argument, this case is matched directly on the (uncast) argument's type instead of going through charCastExpression(), which relies on an explicit cast node being present. Fixes #63280 --- .../bugprone/SignedCharMisuseCheck.cpp | 37 +++++++++++++++++-- clang-tools-extra/docs/ReleaseNotes.rst | 6 +++ .../checks/bugprone/signed-char-misuse.rst | 25 +++++++++++++ .../checkers/bugprone/signed-char-misuse.cpp | 33 +++++++++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp index 88a2614b266b9..6884d11812064 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp @@ -125,11 +125,38 @@ void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) { .bind("arraySubscript"); Finder->addMatcher(STDArraySubscript, this); + + // Catch signed char values passed to a <cctype>/<ctype.h> classification + // or conversion function; any value other than EOF or one representable + // as unsigned char is undefined behavior. Unlike the matchers above, this + // matches the uncast argument directly instead of going through + // charCastExpression(), because hasArgument() strips implicit casts off + // the argument before matching it. + const auto IntTypedef = qualType(hasDeclaration(typedefDecl( + hasAnyName(utils::options::parseStringList(CharTypedefsToIgnoreList))))); + const auto CctypeFunctionArgument = + callExpr( + callee(functionDecl( + hasAnyName("isalnum", "std::isalnum", "isalpha", "std::isalpha", + "isblank", "std::isblank", "iscntrl", "std::iscntrl", + "isdigit", "std::isdigit", "isgraph", "std::isgraph", + "islower", "std::islower", "isprint", "std::isprint", + "ispunct", "std::ispunct", "isspace", "std::isspace", + "isupper", "std::isupper", "isxdigit", "std::isxdigit", + "tolower", "std::tolower", "toupper", "std::toupper"), + hasParameter(0, parmVarDecl(hasType(IntegerType))))), + hasArgument(0, + expr(hasType(qualType(isAnyCharacter(), isSignedInteger(), + unless(IntTypedef)))) + .bind("signedCastExpression"))) + .bind("cctypeFunctionArgument"); + + Finder->addMatcher(CctypeFunctionArgument, this); } void SignedCharMisuseCheck::check(const MatchFinder::MatchResult &Result) { const auto *SignedCastExpression = - Result.Nodes.getNodeAs<ImplicitCastExpr>("signedCastExpression"); + Result.Nodes.getNodeAs<Expr>("signedCastExpression"); const auto *IntegerType = Result.Nodes.getNodeAs<QualType>("integerType"); assert(SignedCastExpression); assert(IntegerType); @@ -138,8 +165,7 @@ void SignedCharMisuseCheck::check(const MatchFinder::MatchResult &Result) { // The potential misinterpretation happens for negative values only. Expr::EvalResult EVResult; if (!SignedCastExpression->isValueDependent() && - SignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult, - *Result.Context)) { + SignedCastExpression->EvaluateAsInt(EVResult, *Result.Context)) { const llvm::APSInt Value = EVResult.Val.getInt(); if (Value.isNonNegative()) return; @@ -166,6 +192,11 @@ void SignedCharMisuseCheck::check(const MatchFinder::MatchResult &Result) { "'signed char' to %0 conversion in array subscript; " "consider casting to 'unsigned char' first.") << *IntegerType; + } else if (Result.Nodes.getNodeAs<Expr>("cctypeFunctionArgument")) { + diag(SignedCastExpression->getBeginLoc(), + "'signed char' to %0 conversion passed to a <cctype> function; " + "consider casting to 'unsigned char' first.") + << *IntegerType; } else { diag(SignedCastExpression->getBeginLoc(), "'signed char' to %0 conversion; " diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 69c3bcf67b8db..7a07a009bbb02 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -103,6 +103,12 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`bugprone-signed-char-misuse + <clang-tidy/checks/bugprone/signed-char-misuse>` check by adding a + diagnostic for ``signed char`` values passed as an argument to a + ``<cctype>``/``<ctype.h>`` character classification or conversion + function. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/signed-char-misuse.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/signed-char-misuse.rst index 2a728d1093f6f..cdf33a9e72890 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/signed-char-misuse.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/signed-char-misuse.rst @@ -41,6 +41,11 @@ Currently, this check warns in the following cases: - ``signed char`` and ``unsigned char`` are compared with equality/inequality operator - ``signed char`` is converted to an integer in the array subscript +- ``signed char`` is passed as an argument to a ``<cctype>``/``<ctype.h>`` + character classification or conversion function (e.g. ``isalpha``, + ``toupper``). These functions require their argument to be representable + as an ``unsigned char`` or equal to ``EOF``; any other value is undefined + behavior. See also: `STR34-C. Cast characters to unsigned char before converting to larger @@ -108,6 +113,26 @@ so both arguments will have the same type. return false; } +Another use case is passing a ``signed char`` to a ``<cctype>``/``<ctype.h>`` +function. These functions interpret their argument as an ``unsigned char``, +so a ``signed char`` holding a non-ASCII character is passed as a negative +value, which is undefined behavior. + +.. code-block:: c++ + + bool isValid(signed char SChar) { + return std::isalpha(SChar); + } + +The fix is the same as above: cast the ``signed char`` to ``unsigned char`` +before passing it to the function. + +.. code-block:: c++ + + bool isValid(signed char SChar) { + return std::isalpha(static_cast<unsigned char>(SChar)); + } + Options ------- diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse.cpp index a1cc2ae489915..6bd84cf1d38af 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse.cpp @@ -13,6 +13,13 @@ struct array { }; } // namespace std +int isalpha(int); +int toupper(int); +namespace std { +using ::isalpha; +using ::toupper; +} // namespace std + int SimpleVarDeclaration() { signed char CCharacter = -5; int NCharacter = CCharacter; @@ -110,6 +117,16 @@ int SignedCharSTDArraySubscript(std::array<int, 3> Array, signed char SCharacter return Array[static_cast<unsigned int>(SCharacter)]; // CHECK-MESSAGES: [[@LINE]]:42: warning: 'signed char' to 'unsigned int' conversion in array subscript; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] } +int CctypeFunctionArgument(signed char SCharacter) { + return isalpha(SCharacter); + // CHECK-MESSAGES: [[@LINE-1]]:18: warning: 'signed char' to 'int' conversion passed to a <cctype> function; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] +} + +int CctypeFunctionArgumentStdNamespace(signed char SCharacter) { + return std::toupper(SCharacter); + // CHECK-MESSAGES: [[@LINE-1]]:23: warning: 'signed char' to 'int' conversion passed to a <cctype> function; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] +} + /////////////////////////////////////////////////////////////////// /// Test cases correctly ignored by the check. @@ -247,3 +264,19 @@ int UnsignedCharSTDArraySubscript(std::array<int, 3> Array, unsigned char USChar int CastedSTDArraySubscript(std::array<int, 3> Array, signed char SCharacter) { return Array[static_cast<unsigned char>(SCharacter)]; } + +int CctypeFunctionArgumentUnsignedChar(unsigned char USCharacter) { + return isalpha(USCharacter); +} + +int CctypeFunctionArgumentCasted(signed char SCharacter) { + return isalpha(static_cast<unsigned char>(SCharacter)); +} + +int CctypeFunctionArgumentAsciiLiteral() { + return isalpha('a'); +} + +int CctypeFunctionArgumentAlreadyInt(int ICharacter) { + return isalpha(ICharacter); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
