Move the fixit for -Wformat-security to a note.

r263299 added a fixit for the -Wformat-security warning, but that runs
into complications with our guideline that error recovery should be done
as-if the fixit had been applied. Putting the fixit on a note avoids that.

llvm-svn: 263584

8 files changed, 29 insertions, 42 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 81d59ba7872..f7958cb0d09 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7152,6 +7152,8 @@ def warn_scanf_scanlist_incomplete : Warning<
 def note_format_string_defined : Note<"format string is defined here">;
 def note_format_fix_specifier : Note<"did you mean to use '%0'?">;
 def note_printf_c_str: Note<"did you mean to call the %0 method?">;
+def note_format_security_fixit: Note<
+  "treat the string as an argument to avoid this">;
 
 def warn_null_arg : Warning<
   "null passed to a callee that requires a non-null argument">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 966d3493926..cc261e05963 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3628,19 +3628,20 @@ bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
   // If there are no arguments specified, warn with -Wformat-security, otherwise
   // warn only with -Wformat-nonliteral.
   if (Args.size() == firstDataArg) {
-    const SemaDiagnosticBuilder &D =
-      Diag(FormatLoc, diag::warn_format_nonliteral_noargs);
+    Diag(FormatLoc, diag::warn_format_nonliteral_noargs)
+      << OrigFormatExpr->getSourceRange();
     switch (Type) {
     default:
-      D << OrigFormatExpr->getSourceRange();
       break;
     case FST_Kprintf:
     case FST_FreeBSDKPrintf:
     case FST_Printf:
-      D << FixItHint::CreateInsertion(FormatLoc, "\"%s\", ");
+      Diag(FormatLoc, diag::note_format_security_fixit)
+        << FixItHint::CreateInsertion(FormatLoc, "\"%s\", ");
       break;
     case FST_NSString:
-      D << FixItHint::CreateInsertion(FormatLoc, "@\"%@\", ");
+      Diag(FormatLoc, diag::note_format_security_fixit)
+        << FixItHint::CreateInsertion(FormatLoc, "@\"%@\", ");
       break;
     }
   } else {
diff --git a/clang/test/Sema/format-strings-fixit.c b/clang/test/Sema/format-strings-fixit.c
index fb617dd500f..b982eb45e5f 100644
--- a/clang/test/Sema/format-strings-fixit.c
+++ b/clang/test/Sema/format-strings-fixit.c
@@ -16,8 +16,6 @@ typedef __UINTMAX_TYPE__ uintmax_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __WCHAR_TYPE__ wchar_t;
 
-extern const char *NonliteralString;
-
 void test() {
   // Basic types
   printf("%s", (int) 123);
@@ -96,9 +94,6 @@ void test() {
   printf("%G", (long double) 42);
   printf("%a", (long double) 42);
   printf("%A", (long double) 42);
-
-  // nonliteral format
-  printf(NonliteralString);
 }
 
 int scanf(char const *, ...);
@@ -223,7 +218,6 @@ void test2(int intSAParm[static 2]) {
 // CHECK: printf("%LG", (long double) 42);
 // CHECK: printf("%La", (long double) 42);
 // CHECK: printf("%LA", (long double) 42);
-// CHECK: printf("%s", NonliteralString);
 
 // CHECK: scanf("%99s", str);
 // CHECK: scanf("%s", vstr);
diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c
index 69723f6e1ac..5559710c603 100644
--- a/clang/test/Sema/format-strings.c
+++ b/clang/test/Sema/format-strings.c
@@ -29,15 +29,22 @@ void check_string_literal( FILE* fp, const char* s, char *buf, ... ) {
   va_start(ap,buf);
 
   printf(s); // expected-warning {{format string is not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   vprintf(s,ap); // expected-warning {{format string is not a string literal}}
   fprintf(fp,s); // expected-warning {{format string is not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   vfprintf(fp,s,ap); // expected-warning {{format string is not a string literal}}
   asprintf(&b,s); // expected-warning {{format string is not a string lit}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   vasprintf(&b,s,ap); // expected-warning {{format string is not a string literal}}
   sprintf(buf,s); // expected-warning {{format string is not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   snprintf(buf,2,s); // expected-warning {{format string is not a string lit}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   __builtin___sprintf_chk(buf,0,-1,s); // expected-warning {{format string is not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   __builtin___snprintf_chk(buf,2,0,-1,s); // expected-warning {{format string is not a string lit}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   vsprintf(buf,s,ap); // expected-warning {{format string is not a string lit}}
   vsnprintf(buf,2,s,ap); // expected-warning {{format string is not a string lit}}
   vsnprintf(buf,2,global_fmt,ap); // expected-warning {{format string is not a string literal}}
@@ -72,13 +79,18 @@ void check_string_literal2( FILE* fp, const char* s, char *buf, ... ) {
   va_start(ap,buf);
 
   printf(s); // expected-warning {{format string is not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   vprintf(s,ap); // no-warning
   fprintf(fp,s); // expected-warning {{format string is not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   vfprintf(fp,s,ap); // no-warning
   asprintf(&b,s); // expected-warning {{format string is not a string lit}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   vasprintf(&b,s,ap); // no-warning
   sprintf(buf,s); // expected-warning {{format string is not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   snprintf(buf,2,s); // expected-warning {{format string is not a string lit}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   __builtin___vsnprintf_chk(buf,2,0,-1,s,ap); // no-warning
 
   vscanf(s, ap); // expected-warning {{format string is not a string literal}}
@@ -88,6 +100,7 @@ void check_conditional_literal(const char* s, int i) {
   printf(i == 1 ? "yes" : "no"); // no-warning
   printf(i == 0 ? (i == 1 ? "yes" : "no") : "dont know"); // no-warning
   printf(i == 0 ? (i == 1 ? s : "no") : "dont know"); // expected-warning{{format string is not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   printf("yes" ?: "no %d", 1); // expected-warning{{data argument not used by format string}}
   printf(0 ? "yes %s" : "no %d", 1); // no-warning
   printf(0 ? "yes %d" : "no %s", 1); // expected-warning{{format specifies type 'char *'}}
@@ -202,8 +215,11 @@ void test_constant_bindings(void) {
   printf(s1); // no-warning
   printf(s2); // no-warning
   printf(s3); // expected-warning{{not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   printf(s4); // expected-warning{{not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   printf(s5); // expected-warning{{not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 
 
@@ -214,6 +230,7 @@ void test_constant_bindings(void) {
 void test9(char *P) {
   int x;
   printf(P);   // expected-warning {{format string is not a string literal (potentially insecure)}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
   printf(P, 42);
 }
 
@@ -632,5 +649,6 @@ extern void test_format_security_extra_args(const char*, int, ...)
     __attribute__((__format__(__printf__, 1, 3)));
 void test_format_security_pos(char* string) {
   test_format_security_extra_args(string, 5); // expected-warning {{format string is not a string literal (potentially insecure)}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 #pragma GCC diagnostic warning "-Wformat-nonliteral"
diff --git a/clang/test/SemaCXX/format-strings-0x.cpp b/clang/test/SemaCXX/format-strings-0x.cpp
index ad57b773e0a..7d37f8276f2 100644
--- a/clang/test/SemaCXX/format-strings-0x.cpp
+++ b/clang/test/SemaCXX/format-strings-0x.cpp
@@ -15,6 +15,7 @@ void f(char **sp, float *fp) {
   scanf("%afoobar", fp);
   printf(nullptr);
   printf(*sp); // expected-warning {{not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
 
   // PR13099
   printf(
diff --git a/clang/test/SemaCXX/format-strings.cpp b/clang/test/SemaCXX/format-strings.cpp
index fa7251d0dd7..b7ef1d709f2 100644
--- a/clang/test/SemaCXX/format-strings.cpp
+++ b/clang/test/SemaCXX/format-strings.cpp
@@ -54,6 +54,7 @@ void rdar8269537(const char *f)
   test_null_format(0); // no-warning
   test_null_format(__null); // no-warning
   test_null_format(f); // expected-warning {{not a string literal}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 
 int Foo::printf(const char *fmt, ...) {
diff --git a/clang/test/SemaObjC/format-strings-objc-fixit.m b/clang/test/SemaObjC/format-strings-objc-fixit.m
deleted file mode 100644
index feaebeec57f..00000000000
--- a/clang/test/SemaObjC/format-strings-objc-fixit.m
+++ /dev/null
@@ -1,31 +0,0 @@
-// RUN: cp %s %t
-// RUN: %clang_cc1 -x objective-c -triple x86_64-apple-darwin -Wno-objc-root-class -pedantic -Wall -fixit %t
-// RUN: %clang_cc1 -x objective-c -triple x86_64-apple-darwin -Wno-objc-root-class -fsyntax-only -pedantic -Wall -Werror %t
-// RUN: %clang_cc1 -x objective-c -triple x86_64-apple-darwin -Wno-objc-root-class -E -o - %t | FileCheck %s
-
-typedef signed char BOOL;
-typedef unsigned int NSUInteger;
-typedef struct _NSZone NSZone;
-@class NSCoder, NSString, NSEnumerator;
-@protocol NSObject  - (BOOL)isEqual:(id)object; @end
-@protocol NSCopying  - (id)copyWithZone:(NSZone *)zone; @end
-@protocol NSMutableCopying  - (id)mutableCopyWithZone:(NSZone *)zone; @end
-@protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end
-@interface NSObject <NSObject> {} @end
-@interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding>  - (NSUInteger)length; @end
-extern void NSLog(NSString *format, ...);
-
-/* This is a test of the various code modification hints that are
-   provided as part of warning or extension diagnostics. All of the
-   warnings will be fixed by -fixit, and the resulting file should
-   compile cleanly with -Werror -pedantic. */
-
-extern NSString *NonliteralString;
-
-void test() {
-  // nonliteral format
-  NSLog(NonliteralString);
-}
-
-// Validate the fixes.
-// CHECK: NSLog(@"%@", NonliteralString);
diff --git a/clang/test/SemaObjC/format-strings-objc.m b/clang/test/SemaObjC/format-strings-objc.m
index 079460cc76c..a1ebf03f8ef 100644
--- a/clang/test/SemaObjC/format-strings-objc.m
+++ b/clang/test/SemaObjC/format-strings-objc.m
@@ -116,6 +116,7 @@ NSString *test_literal_propagation(void) {
   NSLog(ns2); // expected-warning {{more '%' conversions than data arguments}}
   NSString * ns3 = ns1;
   NSLog(ns3); // expected-warning {{format string is not a string literal}}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
 
   NSString * const ns6 = @"split" " string " @"%s"; // expected-note {{format string is defined here}}
   NSLog(ns6); // expected-warning {{more '%' conversions than data arguments}}