[-Wunsafe-buffer-usage] Fix a bug in "warning libc functions (llvm#10…

…1583)"

The commit d7dd2c4 crashes for such
an example:
```
void printf() { printf(); }
```
Because it assumes `printf` must have arguments. This commit fixes
this issue.

(rdar://117182250)

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -14,6 +14,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/Type.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/SourceLocation.h"
@@ -763,32 +764,27 @@ AST_MATCHER(FunctionDecl, isNormalPrintfFunc) {
 AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
               clang::ast_matchers::internal::Matcher<Expr>,
               UnsafeStringArgMatcher) {
-  // Determine what printf it is:
-  const Expr *FirstArg = Node.getArg(0);
-  ASTContext &Ctx = Finder->getASTContext();
+  // Determine what printf it is by examining formal parameters:
+  const FunctionDecl *FD = Node.getDirectCallee();
 
-  if (isa<StringLiteral>(FirstArg->IgnoreParenImpCasts())) {
-    // It is a printf/kprintf. And, the format is a string literal:
-    bool isKprintf = false;
-    const Expr *UnsafeArg;
+  assert(FD && "It should have been checked that FD is non-null.");
 
-    if (auto *Callee = Node.getDirectCallee())
-      if (auto *II = Callee->getIdentifier())
-        isKprintf = II->getName() == "kprintf";
-    if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 0, Ctx, isKprintf))
-      return UnsafeStringArgMatcher.matches(*UnsafeArg, Finder, Builder);
-    return false;
-  }
+  unsigned NumParms = FD->getNumParams();
+
+  if (NumParms < 1)
+    return false; // possibly some user-defined printf function
 
-  QualType PtrTy = FirstArg->getType();
+  ASTContext &Ctx = Finder->getASTContext();
+  QualType FristParmTy = FD->getParamDecl(0)->getType();
 
-  assert(PtrTy->isPointerType());
+  if (!FristParmTy->isPointerType())
+    return false; // possibly some user-defined printf function
 
-  QualType PteTy = (cast<PointerType>(PtrTy))->getPointeeType();
+  QualType FirstPteTy = (cast<PointerType>(FristParmTy))->getPointeeType();
 
-  if (!Ctx.getFILEType().isNull() /* If `FILE *` is not ever in the ASTContext,
-                                     there can't be any file pointer then */
-      && PteTy.getCanonicalType() == Ctx.getFILEType().getCanonicalType()) {
+  if (!Ctx.getFILEType()
+           .isNull() && //`FILE *` must be in the context if it is fprintf
+      FirstPteTy.getCanonicalType() == Ctx.getFILEType().getCanonicalType()) {
     // It is a fprintf:
     const Expr *UnsafeArg;
 
@@ -797,17 +793,32 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
     return false;
   }
 
-  const Expr *SecondArg = Node.getArg(1);
-
-  if (SecondArg->getType()->isIntegerType()) {
-    // It is a snprintf:
+  if (FirstPteTy.isConstQualified()) {
+    // If the first parameter is a `const char *`, it is a printf/kprintf:
+    bool isKprintf = false;
     const Expr *UnsafeArg;
 
-    if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 2, Ctx, false))
+    if (auto *II = FD->getIdentifier())
+      isKprintf = II->getName() == "kprintf";
+    if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 0, Ctx, isKprintf))
       return UnsafeStringArgMatcher.matches(*UnsafeArg, Finder, Builder);
     return false;
   }
-  // It is printf but the format string is passed by pointer. The only thing we
+
+  if (NumParms > 2) {
+    QualType SecondParmTy = FD->getParamDecl(1)->getType();
+
+    if (!FirstPteTy.isConstQualified() && SecondParmTy->isIntegerType()) {
+      // If the first parameter type is non-const qualified `char *` and the
+      // second is an integer, it is a snprintf:
+      const Expr *UnsafeArg;
+
+      if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 2, Ctx, false))
+        return UnsafeStringArgMatcher.matches(*UnsafeArg, Finder, Builder);
+      return false;
+    }
+  }
+  // We don't really recognize this "normal" printf, the only thing we
   // can do is to require all pointers to be null-terminated:
   for (auto Arg : Node.arguments())
     if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg))
@@ -826,12 +837,23 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
 //    size:= DRE.size()/DRE.size_bytes()
 // And DRE is a hardened container or view.
 AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
-  if (Node.getNumArgs() < 3)
-    return false; // not an snprintf call
+  const FunctionDecl *FD = Node.getDirectCallee();
+
+  assert(FD && "It should have been checked that FD is non-null.");
+
+  if (FD->getNumParams() < 3)
+    return false; // Not an snprint
+
+  QualType FirstParmTy = FD->getParamDecl(0)->getType();
+
+  if (!FirstParmTy->isPointerType())
+    return false; // Not an snprint
 
+  QualType FirstPteTy = cast<PointerType>(FirstParmTy)->getPointeeType();
   const Expr *Buf = Node.getArg(0), *Size = Node.getArg(1);
 
-  if (!Buf->getType()->isPointerType() || !Size->getType()->isIntegerType())
+  if (FirstPteTy.isConstQualified() || !Buf->getType()->isPointerType() ||
+      !Size->getType()->isIntegerType())
     return false; // not an snprintf call
 
   static StringRef SizedObjs[] = {"span", "array", "vector",

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

@@ -20,6 +20,7 @@ int snwprintf_s( char* buffer, unsigned buf_size, const char* format, ... );
 int vsnprintf( char* buffer, unsigned buf_size, const char* format, ... );
 int sscanf_s(const char * buffer, const char * format, ...);
 int sscanf(const char * buffer, const char * format, ... );
+int __asan_printf();
 
 namespace std {
   template< class InputIt, class OutputIt >
@@ -64,7 +65,6 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
   strcpy_s();                 // expected-warning{{function 'strcpy_s' is unsafe}}
   wcscpy_s();                 // expected-warning{{function 'wcscpy_s' is unsafe}}
 
-
   /* Test printfs */
   fprintf((FILE*)p, "%s%d", p, *p);  // expected-warning{{function 'fprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
   printf("%s%d", // expected-warning{{function 'printf' is unsafe}}
@@ -90,6 +90,7 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
   snwprintf(s.data(), s.size_bytes(), "%s%d", __PRETTY_FUNCTION__, *p); // no warn
   snwprintf_s(s.data(), s.size_bytes(), "%s%d", __PRETTY_FUNCTION__, *p); // no warn
   strlen("hello");// no warn
+  __asan_printf();// a printf but no argument, so no warn
 }
 
 void v(std::string s1, int *p) {