[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

Author: mikecrowe

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);
 }

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")))

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,

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;

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.
 

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

@@ -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:            }}"                                                  \
@@ -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("");
+}

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");
+}