Reduce false positives in warnings about printfs

- Predefined name macros such as `__PRETTY_FUNCTION__` are considered safe.
- We need to distinguish between `%s` and `%p` as both accept pointers
  but the latter is not a buffer operation.  This leaves us no choice
  other than parsing the format string.  Fortunately, the building
  blocks of format parsing have already existed and are quite handy.

Author: ziqingluo-90

clang/include/clang/AST/FormatString.h

@@ -783,6 +783,18 @@ bool ParsePrintfString(FormatStringHandler &H,
 bool ParseFormatStringHasSArg(const char *beg, const char *end,
                               const LangOptions &LO, const TargetInfo &Target);
 
+/// Parse C format string and return index (relative to `ArgIndex`) of the first
+/// found `s` specifier.  Return 0 if not found.
+/// \param I The start of the C format string; Updated to the first unparsed
+/// position upon return.
+/// \param E The end of the C format string;
+/// \param ArgIndex The argument index of the last found `s` specifier; Or the
+/// argument index of the formatter in initial case.
+unsigned ParseFormatStringFirstSArgIndex(const char *&I, const char *E,
+                                         unsigned ArgIndex,
+                                         const LangOptions &LO,
+                                         const TargetInfo &Target);
+
 bool ParseScanfString(FormatStringHandler &H,
                       const char *beg, const char *end, const LangOptions &LO,
                       const TargetInfo &Target);

clang/lib/AST/PrintfFormatString.cpp

@@ -483,6 +483,34 @@ bool clang::analyze_format_string::ParseFormatStringHasSArg(const char *I,
   return false;
 }
 
+unsigned clang::analyze_format_string::ParseFormatStringFirstSArgIndex(
+    const char *&I, const char *E, unsigned ArgIndex, const LangOptions &LO,
+    const TargetInfo &Target) {
+  unsigned argIndex = ArgIndex;
+
+  // Keep looking for a %s format specifier until we have exhausted the string.
+  FormatStringHandler H;
+  while (I != E) {
+    const PrintfSpecifierResult &FSR =
+        ParsePrintfSpecifier(H, I, E, argIndex, LO, Target, false, false);
+    // Did a fail-stop error of any kind occur when parsing the specifier?
+    // If so, don't do any more processing.
+    if (FSR.shouldStop())
+      return false;
+    // Did we exhaust the string or encounter an error that
+    // we can recover from?
+    if (!FSR.hasValue())
+      continue;
+    const analyze_printf::PrintfSpecifier &FS = FSR.getValue();
+    // Return true if this a %s format specifier.
+    if (FS.getConversionSpecifier().getKind() ==
+        ConversionSpecifier::Kind::sArg) {
+      return FS.getPositionalArgIndex();
+    }
+  }
+  return false;
+}
+
 bool clang::analyze_format_string::parseFormatStringHasFormattingSpecifiers(
     const char *Begin, const char *End, const LangOptions &LO,
     const TargetInfo &Target) {

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -12,6 +12,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/FormatString.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtVisitor.h"
@@ -611,6 +612,38 @@ static bool isNullTermPointer(const Expr *Ptr) {
   return false;
 }
 
+// Return true iff at least one of following cases holds:
+//  1. Format string is a literal and there is an unsafe pointer argument
+//     corresponding to an `s` specifier;
+//  2. Format string is not a literal and there is least an unsafe pointer
+//     argument (including the formatter argument).
+static bool hasUnsafeFormatOrSArg(const Expr *Fmt, unsigned FmtArgIdx,
+                                  const CallExpr *Call, ASTContext &Ctx) {
+  if (auto *SL = dyn_cast<StringLiteral>(Fmt->IgnoreParenImpCasts())) {
+    StringRef FmtStr = SL->getString();
+    auto I = FmtStr.begin();
+    auto E = FmtStr.end();
+    unsigned ArgIdx = FmtArgIdx;
+
+    do {
+      ArgIdx = analyze_format_string::ParseFormatStringFirstSArgIndex(
+          I, E, ArgIdx, Ctx.getLangOpts(), Ctx.getTargetInfo());
+      if (ArgIdx && Call->getNumArgs() > ArgIdx &&
+          !isNullTermPointer(Call->getArg(ArgIdx)))
+        return true;
+    } while (ArgIdx);
+    return false;
+  }
+  // If format is not a string literal, we cannot analyze the format string.
+  // In this case, this call is considered unsafe if at least one argument
+  // (including the format argument) is unsafe pointer.
+  return llvm::any_of(
+      llvm::make_range(Call->arg_begin() + FmtArgIdx, Call->arg_end()),
+      [](const Expr *Arg) {
+        return Arg->getType()->isPointerType() && !isNullTermPointer(Arg);
+      });
+}
+
 // Matches a call to one of the `-printf" functions (excluding the ones with
 // va_list, or `-sprintf`s) that taking pointer-to-char-as-string arguments but
 // fail to guarantee their null-termination.  In other words, these calls are
@@ -626,19 +659,18 @@ AST_MATCHER_P(CallExpr, unsafeStringInPrintfs, StringRef, CoreName) {
   if (Prefix.ends_with("w"))
     Prefix = Prefix.drop_back(1);
 
-  auto AnyUnsafeStrPtr = [](const Expr *Arg) -> bool {
-    return Arg->getType()->isPointerType() && !isNullTermPointer(Arg);
-  };
-
   if (Prefix.empty() ||
       Prefix == "k") // printf: all pointer args should be null-terminated
-    return any_of(Node.arguments(), AnyUnsafeStrPtr);
-  if (Prefix == "f" && Node.getNumArgs() > 1)
-    return any_of(llvm::make_range(Node.arg_begin() + 1, Node.arg_end()),
-                  AnyUnsafeStrPtr);
-  if (Prefix == "sn" && Node.getNumArgs() > 2) {
-    return any_of(llvm::make_range(Node.arg_begin() + 2, Node.arg_end()),
-                  AnyUnsafeStrPtr);
+    return hasUnsafeFormatOrSArg(Node.getArg(0), 0, &Node,
+                                 Finder->getASTContext());
+  if (Prefix == "f")
+    return hasUnsafeFormatOrSArg(Node.getArg(1), 1, &Node,
+                                 Finder->getASTContext());
+  if (Prefix == "sn") {
+    // The first two arguments need to be in safe patterns, which is checked
+    // by `isSafeSizedby`:
+    return hasUnsafeFormatOrSArg(Node.getArg(2), 2, &Node,
+                                 Finder->getASTContext());
   }
   return false; // A call to a "-printf" falls into another category.
 }
@@ -775,7 +807,7 @@ AST_MATCHER_P(CallExpr, ignoreLibcPrefixAndSuffix,
 
   StringRef CoreName =
       TheLittleParser.matchName(II->getName(), FD->getBuiltinID());
-      
+
   return InnerMatcher(CoreName).matches(Node, Finder, Builder);
 }
 } // namespace clang::ast_matchers

clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp

@@ -70,13 +70,20 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
   sscanf(p, "%s%d", "hello", *p);    // expected-warning{{function sscanf introduces unsafe buffer access}}
   sscanf_s(p, "%s%d", "hello", *p);  // expected-warning{{function sscanf_s introduces unsafe buffer access}}
   fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, p); // expected-warning{{function fprintf introduces unsafe buffer access}} expected-note{{use 'std::string::c_str' or string literal as string pointer to guarantee null-termination}}
+  fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, "hello"); // no warn
   printf("%s%d", "hello", *p); // no warn
   snprintf(s.data(), s.size_bytes(), "%s%d", "hello", *p); // no warn
+  snprintf(s.data(), s.size_bytes(), "%s%d", __PRETTY_FUNCTION__, *p); // no warn
   strlen("hello");// no warn
 }
 
-void v(std::string s1) {
-  snprintf(s1.data(), s1.size_bytes(), "%s%d", s1.c_str(), 0); // no warn
+void v(std::string s1, int *p) {
+  snprintf(s1.data(), s1.size_bytes(), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
+  snprintf(s1.data(), s1.size_bytes(), s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str());      // no warn
+  printf("%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str());              // no warn
+  printf(s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str());                   // no warn
+  fprintf((FILE*)0, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str());   // no warn
+  fprintf((FILE*)0, s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str());        // no warn
 }