Make diagnostic a subgroup of format-nonliteral

Author: vvuksanovic

clang/include/clang/Basic/DiagnosticGroups.td

@@ -1182,6 +1182,7 @@ def FormatY2K : DiagGroup<"format-y2k">;
 def FormatPedantic : DiagGroup<"format-pedantic">;
 def FormatSignedness : DiagGroup<"format-signedness">;
 def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
+def MissingFormatAttribute : DiagGroup<"missing-format-attribute">;
 
 def FormatOverflowNonKprintf: DiagGroup<"format-overflow-non-kprintf">;
 def FormatOverflow: DiagGroup<"format-overflow", [FormatOverflowNonKprintf]>;
@@ -1193,7 +1194,7 @@ def Format : DiagGroup<"format",
                         FormatSecurity, FormatY2K, FormatInvalidSpecifier,
                         FormatInsufficientArgs, FormatOverflow, FormatTruncation]>,
              DiagCategory<"Format String Issue">;
-def FormatNonLiteral : DiagGroup<"format-nonliteral">;
+def FormatNonLiteral : DiagGroup<"format-nonliteral", [MissingFormatAttribute]>;
 def Format2 : DiagGroup<"format=2",
                         [FormatNonLiteral, FormatSecurity, FormatY2K]>;
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3481,7 +3481,7 @@ def err_format_attribute_implicit_this_format_string : Error<
 def warn_missing_format_attribute : Warning<
   "diagnostic behavior may be improved by adding the '%0' attribute to the "
   "declaration of %1">,
-  InGroup<DiagGroup<"missing-format-attribute">>, DefaultIgnore;
+  InGroup<MissingFormatAttribute>, DefaultIgnore;
 def err_callback_attribute_no_callee : Error<
   "'callback' attribute specifies no callback callee">;
 def err_callback_attribute_invalid_callee : Error<

clang/lib/Sema/SemaChecking.cpp

@@ -7026,15 +7026,19 @@ std::string escapeFormatString(StringRef Input) {
   return Result;
 }
 
-static void CheckMissingFormatAttributes(
+static bool CheckMissingFormatAttribute(
     Sema *S, ArrayRef<const Expr *> Args, Sema::FormatArgumentPassingKind APK,
     StringLiteral *ReferenceFormatString, unsigned FormatIdx,
     unsigned FirstDataArg, FormatStringType FormatType, unsigned CallerParamIdx,
     SourceLocation Loc) {
-  NamedDecl *Caller = S->getCurFunctionOrMethodDecl();
-  if (!Caller)
-    return;
-  Caller = dyn_cast<NamedDecl>(Caller->getCanonicalDecl());
+  if (S->getDiagnostics().isIgnored(diag::warn_missing_format_attribute,
+                                    SourceLocation()))
+    return false;
+
+  DeclContext *DC = S->CurContext;
+  if (!isa<ObjCMethodDecl>(DC) && !isa<FunctionDecl>(DC) && !isa<BlockDecl>(DC))
+    return false;
+  Decl *Caller = cast<Decl>(DC)->getCanonicalDecl();
 
   unsigned NumCallerParams = getFunctionOrMethodNumParams(Caller);
 
@@ -7055,17 +7059,17 @@ static void CheckMissingFormatAttributes(
     unsigned NumCalleeArgs = Args.size() - FirstDataArg;
     if (NumCalleeArgs == 0 || NumCallerParams < NumCalleeArgs) {
       // There aren't enough arguments in the caller to pass to callee.
-      return;
+      return false;
     }
     for (unsigned CalleeIdx = Args.size() - 1, CallerIdx = NumCallerParams - 1;
          CalleeIdx >= FirstDataArg; --CalleeIdx, --CallerIdx) {
       const auto *Arg =
           dyn_cast<DeclRefExpr>(Args[CalleeIdx]->IgnoreParenCasts());
       if (!Arg)
-        return;
+        return false;
       const auto *Param = dyn_cast<ParmVarDecl>(Arg->getDecl());
       if (!Param || Param->getFunctionScopeIndex() != CallerIdx)
-        return;
+        return false;
     }
     FirstArgumentIndex =
         NumCallerParams + CallerArgumentIndexOffset - NumCalleeArgs;
@@ -7080,13 +7084,14 @@ static void CheckMissingFormatAttributes(
   case Sema::FormatArgumentPassingKind::FAPK_Elsewhere:
     // The callee has a format_matches attribute. We will emit that instead.
     if (!ReferenceFormatString)
-      return;
+      return false;
     break;
   }
 
   // Emit the diagnostic and fixit.
   unsigned FormatStringIndex = CallerParamIdx + CallerArgumentIndexOffset;
   StringRef FormatTypeName = S->GetFormatStringTypeName(FormatType);
+  NamedDecl *ND = dyn_cast<NamedDecl>(Caller);
   do {
     std::string Attr, Fixit;
     if (APK != Sema::FormatArgumentPassingKind::FAPK_Elsewhere)
@@ -7098,8 +7103,15 @@ static void CheckMissingFormatAttributes(
           << "format_matches(" << FormatTypeName << ", " << FormatStringIndex
           << ", \"" << escapeFormatString(ReferenceFormatString->getString())
           << "\")";
-    auto DB = S->Diag(Loc, diag::warn_missing_format_attribute)
-              << Attr << Caller;
+    auto DB = S->Diag(Loc, diag::warn_missing_format_attribute) << Attr;
+    if (ND)
+      DB << ND;
+    else
+      DB << "block";
+
+    // Blocks don't provide a correct end loc, so skip emitting a fixit.
+    if (isa<BlockDecl>(Caller))
+      break;
 
     SourceLocation SL;
     llvm::raw_string_ostream IS(Fixit);
@@ -7124,8 +7136,8 @@ static void CheckMissingFormatAttributes(
 
     DB << FixItHint::CreateInsertion(SL, Fixit);
   } while (false);
-  S->Diag(Caller->getLocation(), diag::note_entity_declared_at) << Caller;
 
+  // Add implicit format or format_matches attribute.
   if (APK != Sema::FormatArgumentPassingKind::FAPK_Elsewhere) {
     Caller->addAttr(FormatAttr::CreateImplicit(
         S->getASTContext(), &S->getASTContext().Idents.get(FormatTypeName),
@@ -7135,6 +7147,15 @@ static void CheckMissingFormatAttributes(
         S->getASTContext(), &S->getASTContext().Idents.get(FormatTypeName),
         FormatStringIndex, ReferenceFormatString));
   }
+
+  {
+    auto DB = S->Diag(Caller->getLocation(), diag::note_entity_declared_at);
+    if (ND)
+      DB << ND;
+    else
+      DB << "block";
+  }
+  return true;
 }
 
 bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
@@ -7193,11 +7214,10 @@ bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
       SourceMgr.isInSystemMacro(FormatLoc))
     return false;
 
-  const LangOptions &LO = getLangOpts();
-  if (CallerParamIdx && (LO.GNUMode || LO.C23 || LO.CPlusPlus11))
-    CheckMissingFormatAttributes(this, Args, APK, ReferenceFormatString,
-                                 format_idx, firstDataArg, Type,
-                                 *CallerParamIdx, Loc);
+  if (CallerParamIdx && CheckMissingFormatAttribute(
+                            this, Args, APK, ReferenceFormatString, format_idx,
+                            firstDataArg, Type, *CallerParamIdx, Loc))
+    return false;
 
   // Strftime is particular as it always uses a single 'time' argument,
   // so it is safe to pass a non-literal string.

clang/test/Sema/attr-format-missing-gnu.c

@@ -1,12 +1,15 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=gnu11 -Wmissing-format-attribute %s
-// RUN: %clang_cc1 -fsyntax-only -verify -x c++ -std=gnu++98 -Wmissing-format-attribute %s
-// RUN: %clang_cc1 -fsyntax-only -std=gnu11 -Wmissing-format-attribute -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -fsyntax-only -x c++ -std=gnu++98 -Wmissing-format-attribute -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -std=gnu11 -Wmissing-format-attribute %s
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -x c++ -std=gnu++98 -Wmissing-format-attribute %s
+// RUN: %clang_cc1 -fsyntax-only -fblocks -std=gnu11 -Wmissing-format-attribute -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fblocks -x c++ -std=gnu++98 -Wmissing-format-attribute -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 typedef unsigned long size_t;
 typedef long ssize_t;
 typedef __builtin_va_list va_list;
 
+__attribute__((format(printf, 1, 2)))
+int printf(const char *, ...);
+
 __attribute__((format(printf, 1, 0)))
 int vprintf(const char *, va_list);
 
@@ -18,3 +21,35 @@ void f1(char *out, va_list args) // #f1
   vprintf(out, args); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 0)' attribute to the declaration of 'f1'}}
                       // expected-note@#f1 {{'f1' declared here}}
 }
+
+void f2(void) {
+  void (^b1)(const char *, ...) = ^(const char *fmt, ...) { // #b1
+    va_list args;
+    vprintf(fmt, args); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 2)' attribute to the declaration of block}}
+                        // expected-note@#b1 {{block declared here}}
+  };
+
+  void (^b2)(const char *, va_list) = ^(const char *fmt, va_list args) { // #b2
+    vprintf(fmt, args); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 0)' attribute to the declaration of block}}.
+                        // expected-note@#b2 {{block declared here}}
+  };
+
+  void (^b3)(const char *, int x, float y) = ^(const char *fmt, int x, float y) { // #b3
+    printf(fmt, x, y); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 2)' attribute to the declaration of block}}.
+                       // expected-note@#b3 {{block declared here}}
+  };
+
+  void __attribute__((__format__(__printf__, 1, 2))) (^b4)(const char *, ...) =
+      ^(const char *fmt, ...) __attribute__((__format__(__printf__, 1, 2))) {
+    va_list args;
+    vprintf(fmt, args);
+  };
+
+  void __attribute__((__format__(__printf__, 2, 3))) (^b5)(const char *, const char *, ...) =
+      ^(const char *not_fmt, const char *fmt, ...) __attribute__((__format__(__printf__, 2, 3))) { // #b5
+    va_list args;
+    vprintf(fmt, args);
+    vprintf(not_fmt, args); // expected-warning{{diagnostic behavior may be improved by adding the 'format(printf, 1, 3)' attribute to the declaration of block}}
+                            // expected-note@#b5 {{block declared here}}
+  };
+}

clang/test/Sema/attr-format-missing-unsupported.c

@@ -177,10 +177,10 @@ void f15(char *ch, const char *out, ... /* args */) // #f15
 // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:1-[[@LINE+1]]:1}:"{{\[\[}}gnu::format(printf, 1, 2)]] "
 void f16(const char *a, ...) // #f16
 {
-	va_list ap;
+	va_list args;
 	const char *const b = a;
-	vprintf(b, ap); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 2)' attribute to the declaration of 'f16'}}
-                  // expected-note@#f16 {{'f16' declared here}}
+	vprintf(b, args); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 2)' attribute to the declaration of 'f16'}}
+                    // expected-note@#f16 {{'f16' declared here}}
 }
 
 // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:1-[[@LINE+1]]:1}:"{{\[\[}}gnu::format(printf, 1, 2)]] "

clang/test/Sema/attr-format-missing.c

@@ -36,8 +36,9 @@ int vscanf(const char * restrict, va_list);
 int vfscanf(FILE * restrict, const char * restrict, va_list);
 int vsscanf(const char * restrict, const char * restrict, va_list);
 
-void test(const char *s, int *i) {
-  scanf(s, i); // expected-warning{{format string is not a string literal}}
+void test(const char *s, int *i) { // #test
+  scanf(s, i); // expected-warning{{diagnostic behavior may be improved by adding the 'format(scanf, 1, 2)' attribute to the declaration of 'test'}}
+  // expected-note@#test {{'test' declared here}}
   scanf("%0d", i); // expected-warning{{zero field width in scanf format string is unused}}
   scanf("%00d", i); // expected-warning{{zero field width in scanf format string is unused}}
   scanf("%d%[asdfasdfd", i, s); // expected-warning{{no closing ']' for '%[' in scanf format string}}

clang/test/Sema/format-strings-scanf.c

@@ -31,10 +31,11 @@ void check_string_literal1( const char* s, ... ) {
   // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 
-void check_string_literal2( const char* s, ... ) {
+void check_string_literal2( const char* s, ... ) { // #check_string_literal2
   va_list ap;
   va_start(ap,s);
-  vprintf(s,ap); // expected-warning {{format string is not a string literal}}
+  vprintf(s,ap); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 2)' attribute to the declaration of 'check_string_literal2'}}
+  // expected-note@#check_string_literal2 {{'check_string_literal2' declared here}}
 }
 
 void check_string_literal3( FILE* fp, const char* s, ... ) {
@@ -44,10 +45,11 @@ void check_string_literal3( FILE* fp, const char* s, ... ) {
   // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 
-void check_string_literal4( FILE* fp, const char* s, ... ) {
+void check_string_literal4( FILE* fp, const char* s, ... ) { // #check_string_literal4
   va_list ap;
   va_start(ap,s);
-  vfprintf(fp,s,ap); // expected-warning {{format string is not a string literal}}
+  vfprintf(fp,s,ap); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 2, 3)' attribute to the declaration of 'check_string_literal4'}}
+  // expected-note@#check_string_literal4 {{'check_string_literal4' declared here}}
 }
 
 void check_string_literal5( const char* s, ... ) {
@@ -58,11 +60,12 @@ void check_string_literal5( const char* s, ... ) {
   // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 
-void check_string_literal6( const char* s, ... ) {
+void check_string_literal6( const char* s, ... ) { // #check_string_literal6
   char * b;
   va_list ap;
   va_start(ap,s);
-  vasprintf(&b,s,ap); // expected-warning {{format string is not a string literal}}
+  vasprintf(&b,s,ap); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 2)' attribute to the declaration of 'check_string_literal6'}}
+  // expected-note@#check_string_literal6 {{'check_string_literal6' declared here}}
 }
 
 void check_string_literal7( const char* s, char *buf ) {
@@ -89,16 +92,18 @@ void check_string_literal10( const char* s, char *buf, ... ) {
   // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 
-void check_string_literal11( const char* s, char *buf, ... ) {
+void check_string_literal11( const char* s, char *buf, ... ) { // #check_string_literal11
   va_list ap;
   va_start(ap,buf);
-  vsprintf(buf,s,ap); // expected-warning {{format string is not a string lit}}
+  vsprintf(buf,s,ap); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 3)' attribute to the declaration of 'check_string_literal11'}}
+  // expected-note@#check_string_literal11 {{'check_string_literal11' declared here}}
 }
 
-void check_string_literal12( const char* s, char *buf, ... ) {
+void check_string_literal12( const char* s, char *buf, ... ) { // #check_string_literal12
   va_list ap;
   va_start(ap,buf);
-  vsnprintf(buf,2,s,ap); // expected-warning {{format string is not a string lit}}
+  vsnprintf(buf,2,s,ap); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 1, 3)' attribute to the declaration of 'check_string_literal12'}}
+  // expected-note@#check_string_literal12 {{'check_string_literal12' declared here}}
 }
 
 void check_string_literal13( char *buf, ... ) {
@@ -107,10 +112,11 @@ void check_string_literal13( char *buf, ... ) {
   vsnprintf(buf,2,global_fmt,ap); // expected-warning {{format string is not a string literal}}
 }
 
-void check_string_literal14( FILE* fp, const char* s, char *buf, ... ) {
+void check_string_literal14( FILE* fp, const char* s, char *buf, ... ) { // #check_string_literal14
   va_list ap;
   va_start(ap,buf);
-  __builtin___vsnprintf_chk(buf,2,0,-1,s,ap); // expected-warning {{format string is not a string lit}}
+  __builtin___vsnprintf_chk(buf,2,0,-1,s,ap); // expected-warning {{diagnostic behavior may be improved by adding the 'format(printf, 2, 4)' attribute to the declaration of 'check_string_literal14'}}
+  // expected-note@#check_string_literal14 {{'check_string_literal14' declared here}}
 }
 
 void check_string_literal15( FILE* fp, const char* s, char *buf, ... ) {
@@ -119,10 +125,11 @@ void check_string_literal15( FILE* fp, const char* s, char *buf, ... ) {
   __builtin___vsnprintf_chk(buf,2,0,-1,global_fmt,ap); // expected-warning {{format string is not a string literal}}
 }
 
-void check_string_literal16(const char* s, ... ) {
+void check_string_literal16(const char* s, ... ) { // #check_string_literal16
   va_list ap;
   va_start(ap,s);
-  vscanf(s, ap); // expected-warning {{format string is not a string literal}}
+  vscanf(s, ap); // expected-warning {{diagnostic behavior may be improved by adding the 'format(scanf, 1, 2)' attribute to the declaration of 'check_string_literal16'}}
+  // expected-note@#check_string_literal16 {{'check_string_literal16' declared here}}
 }
 
 void check_string_literal17() {
@@ -911,12 +918,13 @@ void test_block(void) {
 
   void __attribute__((__format__(__printf__, 2, 3))) (^printf_arg2)(
       const char *, const char *, ...) =
-      ^(const char *not_fmt, const char *fmt, ...)
+      ^(const char *not_fmt, const char *fmt, ...) // #printf_arg2
           __attribute__((__format__(__printf__, 2, 3))) {
     va_list ap;
     va_start(ap, fmt);
     vprintf(fmt, ap);
-    vprintf(not_fmt, ap); // expected-warning{{format string is not a string literal}}
+    vprintf(not_fmt, ap); // expected-warning{{diagnostic behavior may be improved by adding the 'format(printf, 1, 3)' attribute to the declaration of block}}
+    // expected-note@#printf_arg2 {{block declared here}}
     va_end(ap);
   };