https://github.com/mikecrowe updated https://github.com/llvm/llvm-project/pull/94104
>From 2972062997ca582100b5797cd548c4dc2f80c69a Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Fri, 31 May 2024 21:27:03 +0100 Subject: [PATCH] [clang-tidy] Fix assert in modernize-use-std-format/print 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 --- .../modernize/UseStdFormatCheck.cpp | 18 ++++++++----- .../clang-tidy/modernize/UseStdPrintCheck.cpp | 8 ++++++ .../utils/FormatStringConverter.cpp | 8 +++--- .../modernize/use-std-format-custom.cpp | 18 +++++++++++-- .../modernize/use-std-print-custom.cpp | 26 +++++++++++++++++-- 5 files changed, 65 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp index 6cef21f1318a2..5c72f8f22dec7 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp @@ -20,6 +20,11 @@ namespace clang::tidy::modernize { namespace { AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); } +AST_MATCHER(QualType, isSimpleChar) { + const auto ActualType = Node.getTypePtr(); + return ActualType->isSpecificBuiltinType(BuiltinType::Char_S) || + ActualType->isSpecificBuiltinType(BuiltinType::Char_U); +} } // namespace UseStdFormatCheck::UseStdFormatCheck(StringRef Name, ClangTidyContext *Context) @@ -47,13 +52,14 @@ void UseStdFormatCheck::registerPPCallbacks(const SourceManager &SM, } void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) { + auto CharPointerType = hasType(pointerType(pointee(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..6a4497eaf7f60 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp @@ -20,6 +20,11 @@ namespace clang::tidy::modernize { namespace { AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); } +AST_MATCHER(QualType, isSimpleChar) { + const auto ActualType = Node.getTypePtr(); + return ActualType->isSpecificBuiltinType(BuiltinType::Char_S) || + ActualType->isSpecificBuiltinType(BuiltinType::Char_U); +} } // namespace UseStdPrintCheck::UseStdPrintCheck(StringRef Name, ClangTidyContext *Context) @@ -95,12 +100,14 @@ unusedReturnValue(clang::ast_matchers::StatementMatcher MatchedCallExpr) { } void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) { + auto CharPointerType = hasType(pointerType(pointee(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 +120,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/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