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

Bob Wilson via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 15 13:56:39 PDT 2016


Author: bwilson
Date: Tue Mar 15 15:56:38 2016
New Revision: 263584

URL: http://llvm.org/viewvc/llvm-project?rev=263584&view=rev
Log:
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.

Removed:
    cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Sema/format-strings-fixit.c
    cfe/trunk/test/Sema/format-strings.c
    cfe/trunk/test/SemaCXX/format-strings-0x.cpp
    cfe/trunk/test/SemaCXX/format-strings.cpp
    cfe/trunk/test/SemaObjC/format-strings-objc.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=263584&r1=263583&r2=263584&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Mar 15 15:56:38 2016
@@ -7152,6 +7152,8 @@ def warn_scanf_scanlist_incomplete : War
 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">,

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=263584&r1=263583&r2=263584&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Mar 15 15:56:38 2016
@@ -3628,19 +3628,20 @@ bool Sema::CheckFormatArguments(ArrayRef
   // 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 {

Modified: cfe/trunk/test/Sema/format-strings-fixit.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-fixit.c?rev=263584&r1=263583&r2=263584&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings-fixit.c (original)
+++ cfe/trunk/test/Sema/format-strings-fixit.c Tue Mar 15 15:56:38 2016
@@ -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);

Modified: cfe/trunk/test/Sema/format-strings.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=263584&r1=263583&r2=263584&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Tue Mar 15 15:56:38 2016
@@ -29,15 +29,22 @@ void check_string_literal( FILE* fp, con
   va_start(ap,buf);
 
   printf(s); // expected-warning {{format string is not a string literal}}
+  // expected-note at -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 at -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 at -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 at -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 at -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 at -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 at -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, co
   va_start(ap,buf);
 
   printf(s); // expected-warning {{format string is not a string literal}}
+  // expected-note at -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 at -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 at -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 at -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 at -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 cha
   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 at -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 at -1{{treat the string as an argument to avoid this}}
   printf(s4); // expected-warning{{not a string literal}}
+  // expected-note at -1{{treat the string as an argument to avoid this}}
   printf(s5); // expected-warning{{not a string literal}}
+  // expected-note at -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 at -1{{treat the string as an argument to avoid this}}
   printf(P, 42);
 }
 
@@ -632,5 +649,6 @@ extern void test_format_security_extra_a
     __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 at -1{{treat the string as an argument to avoid this}}
 }
 #pragma GCC diagnostic warning "-Wformat-nonliteral"

Modified: cfe/trunk/test/SemaCXX/format-strings-0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/format-strings-0x.cpp?rev=263584&r1=263583&r2=263584&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/format-strings-0x.cpp (original)
+++ cfe/trunk/test/SemaCXX/format-strings-0x.cpp Tue Mar 15 15:56:38 2016
@@ -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 at -1{{treat the string as an argument to avoid this}}
 
   // PR13099
   printf(

Modified: cfe/trunk/test/SemaCXX/format-strings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/format-strings.cpp?rev=263584&r1=263583&r2=263584&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/format-strings.cpp (original)
+++ cfe/trunk/test/SemaCXX/format-strings.cpp Tue Mar 15 15:56:38 2016
@@ -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 at -1{{treat the string as an argument to avoid this}}
 }
 
 int Foo::printf(const char *fmt, ...) {

Removed: cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m?rev=263583&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m (original)
+++ cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m (removed)
@@ -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;
- at class NSCoder, NSString, NSEnumerator;
- at protocol NSObject  - (BOOL)isEqual:(id)object; @end
- at protocol NSCopying  - (id)copyWithZone:(NSZone *)zone; @end
- at protocol NSMutableCopying  - (id)mutableCopyWithZone:(NSZone *)zone; @end
- at protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end
- at interface NSObject <NSObject> {} @end
- at 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);

Modified: cfe/trunk/test/SemaObjC/format-strings-objc.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-strings-objc.m?rev=263584&r1=263583&r2=263584&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/format-strings-objc.m (original)
+++ cfe/trunk/test/SemaObjC/format-strings-objc.m Tue Mar 15 15:56:38 2016
@@ -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 at -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}}




More information about the cfe-commits mailing list