From c6368a23feae261bfce4beaf0b4fca4c5c56677e Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Wed, 16 Nov 2022 09:44:29 -0500 Subject: clang-tidy module: look for sizeof string literal in cmStrLen() check --- .../Tests/cmake-use-cmstrlen-fixit.cxx | 5 +++ .../Tests/cmake-use-cmstrlen-stdout.txt | 32 +++++++++++++ .../ClangTidyModule/Tests/cmake-use-cmstrlen.cxx | 5 +++ Utilities/ClangTidyModule/UseCmstrlenCheck.cxx | 52 ++++++++++++++++++++-- 4 files changed, 90 insertions(+), 4 deletions(-) diff --git a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx index c93d557..cde0086 100644 --- a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx +++ b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx @@ -27,11 +27,16 @@ int main() (void)cmStrLen("Goodbye"); (void)cmStrLen("Hola"); (void)cmStrLen("Bonjour"); + (void)(cmStrLen("Hallo")); + (void)(4 + cmStrLen("Hallo")); + (void)(cmStrLen("Hallo")); + (void)(4 + cmStrLen("Hallo")); // No correction needed (void)ns2::strlen("Salve"); (void)cmStrLen("Konnichiwa"); (void)strlen(s0); + (void)(sizeof("Hallo") - 2); return 0; } diff --git a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-stdout.txt b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-stdout.txt index 6c52ad5..d18822a 100644 --- a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-stdout.txt +++ b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-stdout.txt @@ -18,3 +18,35 @@ cmake-use-cmstrlen.cxx:29:9: warning: use cmStrLen() for string literals [cmake- ^~~~~~~~~~~ cmStrLen cmake-use-cmstrlen.cxx:29:9: note: FIX-IT applied suggested code changes +cmake-use-cmstrlen.cxx:30:10: warning: use cmStrLen() for string literals [cmake-use-cmstrlen] + (void)(sizeof("Hallo") - 1); + ^~~~~~ ~~~ + cmStrLen +cmake-use-cmstrlen.cxx:30:10: note: FIX-IT applied suggested code changes +cmake-use-cmstrlen.cxx:30:26: note: FIX-IT applied suggested code changes + (void)(sizeof("Hallo") - 1); + ^ +cmake-use-cmstrlen.cxx:31:14: warning: use cmStrLen() for string literals [cmake-use-cmstrlen] + (void)(4 + sizeof("Hallo") - 1); + ^~~~~~ ~~~ + cmStrLen +cmake-use-cmstrlen.cxx:31:14: note: FIX-IT applied suggested code changes +cmake-use-cmstrlen.cxx:31:30: note: FIX-IT applied suggested code changes + (void)(4 + sizeof("Hallo") - 1); + ^ +cmake-use-cmstrlen.cxx:32:10: warning: use cmStrLen() for string literals [cmake-use-cmstrlen] + (void)(sizeof "Hallo" - 1); + ^~~~~~ ~~~ + cmStrLen( ) +cmake-use-cmstrlen.cxx:32:10: note: FIX-IT applied suggested code changes +cmake-use-cmstrlen.cxx:32:25: note: FIX-IT applied suggested code changes + (void)(sizeof "Hallo" - 1); + ^ +cmake-use-cmstrlen.cxx:33:14: warning: use cmStrLen() for string literals [cmake-use-cmstrlen] + (void)(4 + sizeof "Hallo" - 1); + ^~~~~~ ~~~ + cmStrLen( ) +cmake-use-cmstrlen.cxx:33:14: note: FIX-IT applied suggested code changes +cmake-use-cmstrlen.cxx:33:29: note: FIX-IT applied suggested code changes + (void)(4 + sizeof "Hallo" - 1); + ^ diff --git a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen.cxx b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen.cxx index f36262b..205bc9c 100644 --- a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen.cxx +++ b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen.cxx @@ -27,11 +27,16 @@ int main() (void)::strlen("Goodbye"); (void)std::strlen("Hola"); (void)ns1::strlen("Bonjour"); + (void)(sizeof("Hallo") - 1); + (void)(4 + sizeof("Hallo") - 1); + (void)(sizeof "Hallo" - 1); + (void)(4 + sizeof "Hallo" - 1); // No correction needed (void)ns2::strlen("Salve"); (void)cmStrLen("Konnichiwa"); (void)strlen(s0); + (void)(sizeof("Hallo") - 2); return 0; } diff --git a/Utilities/ClangTidyModule/UseCmstrlenCheck.cxx b/Utilities/ClangTidyModule/UseCmstrlenCheck.cxx index 590d260..d4bae1f 100644 --- a/Utilities/ClangTidyModule/UseCmstrlenCheck.cxx +++ b/Utilities/ClangTidyModule/UseCmstrlenCheck.cxx @@ -17,17 +17,61 @@ UseCmstrlenCheck::UseCmstrlenCheck(StringRef Name, ClangTidyContext* Context) void UseCmstrlenCheck::registerMatchers(MatchFinder* Finder) { Finder->addMatcher(callExpr(callee(functionDecl(hasName("::strlen"))), - callee(expr().bind("callee")), + callee(expr().bind("strlen")), hasArgument(0, stringLiteral())), this); + + auto IsSizeOfStringLiteral = + unaryExprOrTypeTraitExpr( + ofKind(UETT_SizeOf), + anyOf(has(parenExpr(has(stringLiteral())).bind("paren")), + has(stringLiteral()))) + .bind("sizeOf"); + Finder->addMatcher( + binaryOperator( + hasOperatorName("-"), + hasLHS(anyOf( + binaryOperator(hasOperatorName("+"), hasRHS(IsSizeOfStringLiteral)), + IsSizeOfStringLiteral)), + hasRHS(implicitCastExpr(has(integerLiteral(equals(1)).bind("literal"))))) + .bind("sizeOfMinus"), + this); } void UseCmstrlenCheck::check(const MatchFinder::MatchResult& Result) { - const Expr* Node = Result.Nodes.getNodeAs("callee"); + const Expr* Strlen = Result.Nodes.getNodeAs("strlen"); + const BinaryOperator* SizeOfMinus = + Result.Nodes.getNodeAs("sizeOfMinus"); + + if (Strlen) { + this->diag(Strlen->getBeginLoc(), "use cmStrLen() for string literals") + << FixItHint::CreateReplacement(Strlen->getSourceRange(), "cmStrLen"); + } + + if (SizeOfMinus) { + const ParenExpr* Paren = Result.Nodes.getNodeAs("paren"); + const UnaryExprOrTypeTraitExpr* SizeOf = + Result.Nodes.getNodeAs("sizeOf"); + const IntegerLiteral* Literal = + Result.Nodes.getNodeAs("literal"); - this->diag(Node->getBeginLoc(), "use cmStrLen() for string literals") - << FixItHint::CreateReplacement(Node->getSourceRange(), "cmStrLen"); + std::vector FixIts; + if (Paren) { + FixIts.push_back( + FixItHint::CreateReplacement(SizeOf->getOperatorLoc(), "cmStrLen")); + FixIts.push_back(FixItHint::CreateRemoval( + SourceRange(SizeOfMinus->getOperatorLoc(), Literal->getLocation()))); + } else { + FixIts.push_back( + FixItHint::CreateReplacement(SizeOf->getOperatorLoc(), "cmStrLen(")); + FixIts.push_back(FixItHint::CreateReplacement( + SourceRange(SizeOfMinus->getOperatorLoc(), Literal->getLocation()), + ")")); + } + this->diag(SizeOf->getOperatorLoc(), "use cmStrLen() for string literals") + << FixIts; + } } } } -- cgit v0.12 From 830eed374d540ebd643cd9385546838804c07160 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Wed, 16 Nov 2022 09:57:43 -0500 Subject: CMake: fix sizeof string literal violations --- Source/CPack/cmCPackDragNDropGenerator.cxx | 2 +- Source/cmExportFileGenerator.cxx | 7 +++---- Source/cmGeneratorExpression.cxx | 2 +- Source/cmGeneratorTarget.cxx | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Source/CPack/cmCPackDragNDropGenerator.cxx b/Source/CPack/cmCPackDragNDropGenerator.cxx index 0579066..68e7ba3 100644 --- a/Source/CPack/cmCPackDragNDropGenerator.cxx +++ b/Source/CPack/cmCPackDragNDropGenerator.cxx @@ -451,7 +451,7 @@ int cmCPackDragNDropGenerator::CreateDMG(const std::string& src_dir, mountpoint_regex.find(attach_output.c_str()); std::string const temp_mount = mountpoint_regex.match(1); std::string const temp_mount_name = - temp_mount.substr(sizeof("/Volumes/") - 1); + temp_mount.substr(cmStrLen("/Volumes/")); // Remove dummy padding file so we have enough space on RW image ... std::ostringstream dummy_padding; diff --git a/Source/cmExportFileGenerator.cxx b/Source/cmExportFileGenerator.cxx index bd2b6af..c8e2cb8 100644 --- a/Source/cmExportFileGenerator.cxx +++ b/Source/cmExportFileGenerator.cxx @@ -669,8 +669,7 @@ void cmExportFileGenerator::ResolveTargetsInGeneratorExpression( while ((pos = input.find("$', nameStartPos); std::string::size_type commaPos = input.find(',', nameStartPos); std::string::size_type nextOpenPos = input.find("$<", nameStartPos); @@ -696,7 +695,7 @@ void cmExportFileGenerator::ResolveTargetsInGeneratorExpression( pos = 0; lastPos = pos; while ((pos = input.find("$', nameStartPos); if (endPos == std::string::npos) { errorString = "$ expression incomplete"; @@ -721,7 +720,7 @@ void cmExportFileGenerator::ResolveTargetsInGeneratorExpression( lastPos = pos; while (errorString.empty() && (pos = input.find("$', nameStartPos); if (endPos == std::string::npos) { errorString = "$ expression incomplete"; diff --git a/Source/cmGeneratorExpression.cxx b/Source/cmGeneratorExpression.cxx index b605350..c5ae31b 100644 --- a/Source/cmGeneratorExpression.cxx +++ b/Source/cmGeneratorExpression.cxx @@ -406,7 +406,7 @@ void cmGeneratorExpression::ReplaceInstallPrefix( while ((pos = input.find("$", lastPos)) != std::string::npos) { - std::string::size_type endPos = pos + sizeof("$") - 1; + std::string::size_type endPos = pos + cmStrLen("$"); input.replace(pos, endPos - pos, replacement); lastPos = endPos; } diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 6065285..fae6d54 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -6718,7 +6718,7 @@ bool cmGeneratorTarget::IsLinkLookupScope(std::string const& n, cmLocalGenerator const*& lg) const { if (cmHasLiteralPrefix(n, CMAKE_DIRECTORY_ID_SEP)) { - cmDirectoryId const dirId = n.substr(sizeof(CMAKE_DIRECTORY_ID_SEP) - 1); + cmDirectoryId const dirId = n.substr(cmStrLen(CMAKE_DIRECTORY_ID_SEP)); if (dirId.String.empty()) { lg = this->LocalGenerator; return true; -- cgit v0.12