Meinersbur created this revision.
Meinersbur added reviewers: dlj, rsmith, Jean-Daniel.

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, GNU gettext's ngettext (select plural or singular version of 
format string) has two __format_arg__ attributes.


Repository:
  rC Clang

https://reviews.llvm.org/D48734

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/attr-format_arg.c


Index: test/Sema/attr-format_arg.c
===================================================================
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,15 @@
 
 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("", ""), 123); // expected-warning 2{{format string is empty}}
 }
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
   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 FormatArgAttr *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 FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
         unsigned BuiltinID = FD->getBuiltinID();
         if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
             BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {


Index: test/Sema/attr-format_arg.c
===================================================================
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,15 @@
 
 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("", ""), 123); // expected-warning 2{{format string is empty}}
 }
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
   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 FormatArgAttr *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 FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
         unsigned BuiltinID = FD->getBuiltinID();
         if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
             BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to