Skip to content

[clang-tidy] Fix assert in modernize-use-std-format/print #94104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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")))
Expand All @@ -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")))
Expand Down
8 changes: 5 additions & 3 deletions clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions clang-tools-extra/clang-tidy/utils/Matchers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
// 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: }}" \
// RUN: -- -isystem %clang_tidy_headers
// 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: }}" \
Expand Down Expand Up @@ -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("");
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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");
}
Loading