[Sema] Consider all format_arg attributes.

[Meinersbur]

Summary

If a function has multiple format_arg attributes, clang only considers the first it finds (because AttributeLists are in reverse order, it is textual last) and ignores all others.

Loop over all FormatArgAttr to print warnings for all declared format_arg attributes.

For instance, libintl's ngettext (select plural or singular version of format string) has two format_arg attributes.

[Meinersbur]

lib/Sema/SemaChecking.cpp

5534

Not sure what should be returned here; To minimize surprises, this returns what the current version would have returned.

[aaron.ballman]

lib/Sema/SemaChecking.cpp

5523

You can use const auto *FA here instead; the type is spelled out in the initializer.

5536

const auto *

test/Sema/attr-format_arg.c

17

Can you add tests for the other permutations as well? It would also be useful to put the string literals on their own line so that the expected-warning designation matches the specific argument that's being diagnosed. e.g.,

h(
  "%d",
  ""
)
h(
  "", 
  "%d"
)
h(
  "%d",
  "%d"
)

[Meinersbur]

• Address Aaron's remarks

[Meinersbur]

• Reinsert test for the change in this patch

[aaron.ballman]

LGTM!

lib/Sema/SemaChecking.cpp

case Stmt::DeclRefExprClass: {
 
     return SLCT_NotALiteral;
   }
 
   case Stmt::CallExprClass:
   case Stmt::CXXMemberCallExprClass: {
     const CallExpr *CE = cast<CallExpr>(E);
     if (const NamedDecl *ND = dyn_cast_or_null<NamedDecl>(CE->getCalleeDecl())) {
-      if (const FormatArgAttr *FA = ND->getAttr<FormatArgAttr>()) {
+      bool IsFirst = true;
+      StringLiteralCheckType CommonResult;
+      for (const auto *FA : ND->specific_attrs<FormatArgAttr>()) {
         const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-        return checkFormatStringExpr(S, Arg, Args,
-                                     HasVAListArg, format_idx, firstDataArg,
-                                     Type, CallType, InFunctionCall,
-                                     CheckedVarArgs, UncoveredArg, Offset);
-      } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
+        StringLiteralCheckType Result = checkFormatStringExpr(
+            S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+            CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+        if (IsFirst) {
+          CommonResult = Result;
+          IsFirst = false;
+        }
+      }
+      if (!IsFirst)
+        return CommonResult;
+
+      if (const auto *FD = dyn_cast<FunctionDecl>(ND)) {
         unsigned BuiltinID = FD->getBuiltinID();
         if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
             BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
           const Expr *Arg = CE->getArg(0);
           return checkFormatStringExpr(S, Arg, Args,
                                        HasVAListArg, format_idx,
                                        firstDataArg, Type, CallType,
                                        InFunctionCall, CheckedVarArgs,

test/Sema/attr-format_arg.c

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 int printf(const char *, ...);
 
 const char* f(const char *s) __attribute__((format_arg(1)));
 
+const char *h(const char *msg1, const char *msg2)
+    __attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
 void g(const char *s) {
   printf("%d", 123);
   printf("%d %d", 123); // expected-warning{{more '%' conversions than data arguments}}
 
   printf(f("%d"), 123);
   printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}}
+
+  printf(h(
+    "", // expected-warning {{format string is empty}}
+    ""  // expected-warning {{format string is empty}}
+  ), 123);
+  printf(h(
+    "%d",
+    ""  // expected-warning {{format string is empty}}
+  ), 123);
+  printf(h(
+    "", // expected-warning {{format string is empty}}
+    "%d"
+  ), 123);
+  printf(h("%d", "%d"), 123);
 }