Skip to content

Commit 2972062

Browse files
mikecrowemikecrowe
committed
[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
1 parent 16397e8 commit 2972062

File tree

5 files changed

+65
-13
lines changed

5 files changed

+65
-13
lines changed

‎clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ namespace clang::tidy::modernize {
2020

2121
namespace {
2222
AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); }
23+
AST_MATCHER(QualType, isSimpleChar) {
24+
const auto ActualType = Node.getTypePtr();
25+
return ActualType->isSpecificBuiltinType(BuiltinType::Char_S) ||
26+
ActualType->isSpecificBuiltinType(BuiltinType::Char_U);
27+
}
2328
} // namespace
2429

2530
UseStdFormatCheck::UseStdFormatCheck(StringRef Name, ClangTidyContext *Context)
@@ -47,13 +52,14 @@ void UseStdFormatCheck::registerPPCallbacks(const SourceManager &SM,
4752
}
4853

4954
void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) {
55+
auto CharPointerType = hasType(pointerType(pointee(isSimpleChar())));
5056
Finder->addMatcher(
51-
callExpr(argumentCountAtLeast(1),
52-
hasArgument(0, stringLiteral(isOrdinary())),
53-
callee(functionDecl(unless(cxxMethodDecl()),
54-
matchers::matchesAnyListedName(
55-
StrFormatLikeFunctions))
56-
.bind("func_decl")))
57+
callExpr(
58+
argumentCountAtLeast(1), hasArgument(0, stringLiteral(isOrdinary())),
59+
callee(functionDecl(
60+
unless(cxxMethodDecl()), hasParameter(0, CharPointerType),
61+
matchers::matchesAnyListedName(StrFormatLikeFunctions))
62+
.bind("func_decl")))
5763
.bind("strformat"),
5864
this);
5965
}

‎clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ namespace clang::tidy::modernize {
2020

2121
namespace {
2222
AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); }
23+
AST_MATCHER(QualType, isSimpleChar) {
24+
const auto ActualType = Node.getTypePtr();
25+
return ActualType->isSpecificBuiltinType(BuiltinType::Char_S) ||
26+
ActualType->isSpecificBuiltinType(BuiltinType::Char_U);
27+
}
2328
} // namespace
2429

2530
UseStdPrintCheck::UseStdPrintCheck(StringRef Name, ClangTidyContext *Context)
@@ -95,12 +100,14 @@ unusedReturnValue(clang::ast_matchers::StatementMatcher MatchedCallExpr) {
95100
}
96101

97102
void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) {
103+
auto CharPointerType = hasType(pointerType(pointee(isSimpleChar())));
98104
if (!PrintfLikeFunctions.empty())
99105
Finder->addMatcher(
100106
unusedReturnValue(
101107
callExpr(argumentCountAtLeast(1),
102108
hasArgument(0, stringLiteral(isOrdinary())),
103109
callee(functionDecl(unless(cxxMethodDecl()),
110+
hasParameter(0, CharPointerType),
104111
matchers::matchesAnyListedName(
105112
PrintfLikeFunctions))
106113
.bind("func_decl")))
@@ -113,6 +120,7 @@ void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) {
113120
callExpr(argumentCountAtLeast(2),
114121
hasArgument(1, stringLiteral(isOrdinary())),
115122
callee(functionDecl(unless(cxxMethodDecl()),
123+
hasParameter(1, CharPointerType),
116124
matchers::matchesAnyListedName(
117125
FprintfLikeFunctions))
118126
.bind("func_decl")))

‎clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,11 @@ FormatStringConverter::FormatStringConverter(ASTContext *ContextIn,
208208
assert(ArgsOffset <= NumArgs);
209209
FormatExpr = llvm::dyn_cast<StringLiteral>(
210210
Args[FormatArgOffset]->IgnoreImplicitAsWritten());
211-
assert(FormatExpr);
212-
if (!FormatExpr->isOrdinary())
213-
return; // No wide string support yet
211+
if (!FormatExpr || !FormatExpr->isOrdinary()) {
212+
// Function must have a narrow string literal as its first argument.
213+
conversionNotPossible("first argument is not a narrow string literal");
214+
return;
215+
}
214216
PrintfFormatString = FormatExpr->getString();
215217

216218
// Assume that the output will be approximately the same size as the input,

‎clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22
// RUN: -std=c++20 %s modernize-use-std-format %t -- \
33
// RUN: -config="{CheckOptions: { \
44
// RUN: modernize-use-std-format.StrictMode: true, \
5-
// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2', \
5+
// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2; bad_format_type_strprintf', \
66
// RUN: modernize-use-std-format.ReplacementFormatFunction: 'fmt::format', \
77
// RUN: modernize-use-std-format.FormatHeader: '<fmt/core.h>' \
88
// RUN: }}" \
99
// RUN: -- -isystem %clang_tidy_headers
1010
// RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \
1111
// RUN: -std=c++20 %s modernize-use-std-format %t -- \
1212
// RUN: -config="{CheckOptions: { \
13-
// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2', \
13+
// RUN: modernize-use-std-format.StrFormatLikeFunctions: '::strprintf; mynamespace::strprintf2; bad_format_type_strprintf', \
1414
// RUN: modernize-use-std-format.ReplacementFormatFunction: 'fmt::format', \
1515
// RUN: modernize-use-std-format.FormatHeader: '<fmt/core.h>' \
1616
// RUN: }}" \
@@ -50,3 +50,17 @@ std::string A(const std::string &in)
5050
{
5151
return "_" + in;
5252
}
53+
54+
// Issue #92896: Ensure that the check doesn't assert if the argument is
55+
// promoted to something that isn't a string.
56+
struct S {
57+
S(...);
58+
};
59+
std::string bad_format_type_strprintf(const S &, ...);
60+
61+
std::string unsupported_format_parameter_type()
62+
{
63+
// No fixes here because the format parameter of the function called is not a
64+
// string.
65+
return bad_format_type_strprintf("");
66+
}

‎clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// RUN: %check_clang_tidy -std=c++23 %s modernize-use-std-print %t -- \
22
// RUN: -config="{CheckOptions: \
33
// RUN: { \
4-
// RUN: modernize-use-std-print.PrintfLikeFunctions: 'unqualified_printf;::myprintf; mynamespace::myprintf2', \
5-
// RUN: modernize-use-std-print.FprintfLikeFunctions: '::myfprintf; mynamespace::myfprintf2' \
4+
// RUN: modernize-use-std-print.PrintfLikeFunctions: 'unqualified_printf;::myprintf; mynamespace::myprintf2; bad_format_type_printf', \
5+
// RUN: modernize-use-std-print.FprintfLikeFunctions: '::myfprintf; mynamespace::myfprintf2; bad_format_type_fprintf' \
66
// RUN: } \
77
// RUN: }" \
88
// RUN: -- -isystem %clang_tidy_headers
@@ -86,3 +86,25 @@ void no_name(const std::string &in)
8686
{
8787
"A" + in;
8888
}
89+
90+
int myprintf(const wchar_t *, ...);
91+
92+
void wide_string_not_supported() {
93+
myprintf(L"wide string %s", L"string");
94+
}
95+
96+
// Issue #92896: Ensure that the check doesn't assert if the argument is
97+
// promoted to something that isn't a string.
98+
struct S {
99+
S(...) {}
100+
};
101+
int bad_format_type_printf(const S &, ...);
102+
int bad_format_type_fprintf(FILE *, const S &, ...);
103+
104+
void unsupported_format_parameter_type()
105+
{
106+
// No fixes here because the format parameter of the function called is not a
107+
// string.
108+
bad_format_type_printf("Hello %s", "world");
109+
bad_format_type_fprintf(stderr, "Hello %s", "world");
110+
}

0 commit comments

Comments
 (0)