Author: Mike Crowe Date: 2024-06-03T16:22:24+02:00 New Revision: 0e62d5cf55479981da5e05e406bbca4afb3cdc4f
URL: https://github.com/llvm/llvm-project/commit/0e62d5cf55479981da5e05e406bbca4afb3cdc4f DIFF: https://github.com/llvm/llvm-project/commit/0e62d5cf55479981da5e05e406bbca4afb3cdc4f.diff LOG: [clang-tidy] Fix assert in modernize-use-std-format/print (#94104) Ensure that FormatStringConverter's constructor fails with a sensible error message rather than asserting if the format string is not a narrow string literal. Also, ensure that we don't even get that far in modernize-use-std-print and modernize-use-std-format by checking that the format string parameter is a char pointer. Fixes #92896 Added: Modified: clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp clang-tools-extra/clang-tidy/utils/Matchers.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp index 6cef21f1318a2..d082faa786b37 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp @@ -47,13 +47,15 @@ void UseStdFormatCheck::registerPPCallbacks(const SourceManager &SM, } void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) { + auto CharPointerType = + hasType(pointerType(pointee(matchers::isSimpleChar()))); Finder->addMatcher( - callExpr(argumentCountAtLeast(1), - hasArgument(0, stringLiteral(isOrdinary())), - callee(functionDecl(unless(cxxMethodDecl()), - matchers::matchesAnyListedName( - StrFormatLikeFunctions)) - .bind("func_decl"))) + callExpr( + argumentCountAtLeast(1), hasArgument(0, stringLiteral(isOrdinary())), + callee(functionDecl( + unless(cxxMethodDecl()), hasParameter(0, CharPointerType), + matchers::matchesAnyListedName(StrFormatLikeFunctions)) + .bind("func_decl"))) .bind("strformat"), this); } diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp index ff990feadc0c1..1ea170c3cd310 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp @@ -95,12 +95,15 @@ unusedReturnValue(clang::ast_matchers::StatementMatcher MatchedCallExpr) { } void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) { + auto CharPointerType = + hasType(pointerType(pointee(matchers::isSimpleChar()))); if (!PrintfLikeFunctions.empty()) Finder->addMatcher( unusedReturnValue( callExpr(argumentCountAtLeast(1), hasArgument(0, stringLiteral(isOrdinary())), callee(functionDecl(unless(cxxMethodDecl()), + hasParameter(0, CharPointerType), matchers::matchesAnyListedName( PrintfLikeFunctions)) .bind("func_decl"))) @@ -113,6 +116,7 @@ void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) { callExpr(argumentCountAtLeast(2), hasArgument(1, stringLiteral(isOrdinary())), callee(functionDecl(unless(cxxMethodDecl()), + hasParameter(1, CharPointerType), matchers::matchesAnyListedName( FprintfLikeFunctions)) .bind("func_decl"))) diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp index 845e71c5003b8..33f3ea47df1e3 100644 --- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp +++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp @@ -208,9 +208,11 @@ FormatStringConverter::FormatStringConverter(ASTContext *ContextIn, assert(ArgsOffset <= NumArgs); FormatExpr = llvm::dyn_cast<StringLiteral>( Args[FormatArgOffset]->IgnoreImplicitAsWritten()); - assert(FormatExpr); - if (!FormatExpr->isOrdinary()) - return; // No wide string support yet + if (!FormatExpr || !FormatExpr->isOrdinary()) { + // Function must have a narrow string literal as its first argument. + conversionNotPossible("first argument is not a narrow string literal"); + return; + } PrintfFormatString = FormatExpr->getString(); // Assume that the output will be approximately the same size as the input, diff --git a/clang-tools-extra/clang-tidy/utils/Matchers.h b/clang-tools-extra/clang-tidy/utils/Matchers.h index 045e3ffbb6a8b..5fd98db967870 100644 --- a/clang-tools-extra/clang-tidy/utils/Matchers.h +++ b/clang-tools-extra/clang-tidy/utils/Matchers.h @@ -49,6 +49,14 @@ AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isPointerToConst) { return pointerType(pointee(qualType(isConstQualified()))); } +// Returns QualType matcher for target char type only. +AST_MATCHER(QualType, isSimpleChar) { + const auto ActualType = Node.getTypePtr(); + return ActualType && + (ActualType->isSpecificBuiltinType(BuiltinType::Char_S) || + ActualType->isSpecificBuiltinType(BuiltinType::Char_U)); +} + AST_MATCHER(Expr, hasUnevaluatedContext) { if (isa<CXXNoexceptExpr>(Node) || isa<RequiresExpr>(Node)) return true; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 4f674d1a4d556..33b65caf2b02c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -351,6 +351,11 @@ Changes in existing checks <clang-tidy/checks/modernize/use-starts-ends-with>` check to also handle calls to ``compare`` method. +- Improved :doc:`modernize-use-std-print + <clang-tidy/checks/modernize/use-std-print>` check to not crash if the + format string parameter of the function to be replaced is not of the + expected type. + - Improved :doc:`modernize-use-using <clang-tidy/checks/modernize/use-using>` check by adding support for detection of typedefs declared on function level. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp index 815e22b291551..c025113055cce 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp @@ -2,7 +2,7 @@ // RUN: -std=c++20 %s modernize-use-std-format %t -- \ // RUN: -config="{CheckOptions: { \ // RUN: modernize-use-std-format.StrictMode: true, \ -// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2', \ +// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2; bad_format_type_strprintf', \ // RUN: modernize-use-std-format.ReplacementFormatFunction: 'fmt::format', \ // RUN: modernize-use-std-format.FormatHeader: '<fmt/core.h>' \ // RUN: }}" \ @@ -10,7 +10,7 @@ // RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \ // RUN: -std=c++20 %s modernize-use-std-format %t -- \ // RUN: -config="{CheckOptions: { \ -// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2', \ +// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2; bad_format_type_strprintf', \ // RUN: modernize-use-std-format.ReplacementFormatFunction: 'fmt::format', \ // RUN: modernize-use-std-format.FormatHeader: '<fmt/core.h>' \ // RUN: }}" \ @@ -50,3 +50,17 @@ std::string A(const std::string &in) { return "_" + in; } + +// Issue #92896: Ensure that the check doesn't assert if the argument is +// promoted to something that isn't a string. +struct S { + S(...); +}; +std::string bad_format_type_strprintf(const S &, ...); + +std::string unsupported_format_parameter_type() +{ + // No fixes here because the format parameter of the function called is not a + // string. + return bad_format_type_strprintf(""); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp index 8466217b765a8..09720001ab837 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp @@ -1,8 +1,8 @@ // RUN: %check_clang_tidy -std=c++23 %s modernize-use-std-print %t -- \ // RUN: -config="{CheckOptions: \ // RUN: { \ -// RUN: modernize-use-std-print.PrintfLikeFunctions: 'unqualified_printf;::myprintf; mynamespace::myprintf2', \ -// RUN: modernize-use-std-print.FprintfLikeFunctions: '::myfprintf; mynamespace::myfprintf2' \ +// RUN: modernize-use-std-print.PrintfLikeFunctions: 'unqualified_printf;::myprintf; mynamespace::myprintf2; bad_format_type_printf', \ +// RUN: modernize-use-std-print.FprintfLikeFunctions: '::myfprintf; mynamespace::myfprintf2; bad_format_type_fprintf' \ // RUN: } \ // RUN: }" \ // RUN: -- -isystem %clang_tidy_headers @@ -86,3 +86,25 @@ void no_name(const std::string &in) { "A" + in; } + +int myprintf(const wchar_t *, ...); + +void wide_string_not_supported() { + myprintf(L"wide string %s", L"string"); +} + +// Issue #92896: Ensure that the check doesn't assert if the argument is +// promoted to something that isn't a string. +struct S { + S(...) {} +}; +int bad_format_type_printf(const S &, ...); +int bad_format_type_fprintf(FILE *, const S &, ...); + +void unsupported_format_parameter_type() +{ + // No fixes here because the format parameter of the function called is not a + // string. + bad_format_type_printf("Hello %s", "world"); + bad_format_type_fprintf(stderr, "Hello %s", "world"); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
