[clang] [Sema] Suggest missing format attributes (PR #166738)

[Vladimir Vuksanovic via cfe-commits]

================
@@ -7001,6 +7005,85 @@ bool Sema::CheckFormatString(const FormatMatchesAttr *Format,
   return false;
 }
 
+static void CheckMissingFormatAttributes(Sema *S, FormatStringType FormatType,
+                                         unsigned FormatIdx, unsigned FirstArg,
+                                         ArrayRef<const Expr *> Args,
+                                         Sema::FormatArgumentPassingKind APK,
+                                         unsigned CallerParamIdx,
+                                         SourceLocation Loc) {
+  const FunctionDecl *Caller = S->getCurFunctionDecl();
+  if (!Caller)
+    return;
+
+  // Find the offset to convert between attribute and parameter indexes.
+  unsigned CallerArgumentIndexOffset =
+      hasImplicitObjectParameter(Caller) ? 2 : 1;
+
+  unsigned FirstArgumentIndex = -1;
+  switch (APK) {
+  case Sema::FormatArgumentPassingKind::FAPK_Fixed:
+  case Sema::FormatArgumentPassingKind::FAPK_Variadic: {
+    // As an extension, clang allows the format attribute on non-variadic
+    // functions.
+    // Caller must have fixed arguments to pass them to a fixed or variadic
+    // function. Try to match caller and callee arguments. If successful, then
+    // emit a diag with the caller idx, otherwise we can't determine the callee
+    // arguments.
+    unsigned NumCalleeArgs = Args.size() - FirstArg;
+    if (NumCalleeArgs == 0 || Caller->getNumParams() < NumCalleeArgs) {
+      // There aren't enough arugments in the caller to pass to callee.
+      return;
+    }
+    for (unsigned CalleeIdx = Args.size() - 1,
+                  CallerIdx = Caller->getNumParams() - 1;
+         CalleeIdx >= FirstArg; --CalleeIdx, --CallerIdx) {
+      const auto *Arg =
+          dyn_cast<DeclRefExpr>(Args[CalleeIdx]->IgnoreParenCasts());
+      if (!Arg)
+        return;
+      const auto *Param = dyn_cast<ParmVarDecl>(Arg->getDecl());
+      if (!Param || Param->getFunctionScopeIndex() != CallerIdx)
+        return;
+    }
+    FirstArgumentIndex =
+        Caller->getNumParams() + CallerArgumentIndexOffset - NumCalleeArgs;
+    break;
+  }
+  case Sema::FormatArgumentPassingKind::FAPK_VAList:
+    // Caller arguments are either variadic or a va_list.
+    FirstArgumentIndex =
+        Caller->isVariadic()
+            ? (Caller->getNumParams() + CallerArgumentIndexOffset)
+            : 0;
+    break;
+  case Sema::FormatArgumentPassingKind::FAPK_Elsewhere:
+    // Args are not passed to the callee.
+    return;
+  }
+
+  // Emit the diagnostic and fixit.
----------------
vvuksanovic wrote:

That changes some behavior:

```c
void foo(const char *fmt, va_list args) {
  vprintf(fmt, args);
  vscanf(fmt, args);
}
```

Previously, this would emit two diagnostics to suggest a printf and scanf format attribute. If we add an implicit attr, then when looking at `vscanf` we would get `passing 'printf' format string where 'scanf' format string is expected`. Also, this would eliminate duplicate diagnostics created by multiple format function calls of the same type.

I'll see if there are any other differences before updating the PR, but these changes make sense to me.

https://github.com/llvm/llvm-project/pull/166738