From: Bob Wilson Date: Tue, 15 Mar 2016 20:56:38 +0000 (+0000) Subject: Move the fixit for -Wformat-security to a note. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=95e5cfb2fad4d8431a4d74206b7aa150e1359aac;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@263584 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 81d59ba787..f7958cb0d0 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 966d349392..cc261e0596 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -3628,19 +3628,20 @@ bool Sema::CheckFormatArguments(ArrayRef 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/test/Sema/format-strings-fixit.c b/test/Sema/format-strings-fixit.c index fb617dd500..b982eb45e5 100644 --- a/test/Sema/format-strings-fixit.c +++ b/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/test/Sema/format-strings.c b/test/Sema/format-strings.c index 69723f6e1a..5559710c60 100644 --- a/test/Sema/format-strings.c +++ b/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/test/SemaCXX/format-strings-0x.cpp b/test/SemaCXX/format-strings-0x.cpp index ad57b773e0..7d37f8276f 100644 --- a/test/SemaCXX/format-strings-0x.cpp +++ b/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/test/SemaCXX/format-strings.cpp b/test/SemaCXX/format-strings.cpp index fa7251d0dd..b7ef1d709f 100644 --- a/test/SemaCXX/format-strings.cpp +++ b/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/test/SemaObjC/format-strings-objc-fixit.m b/test/SemaObjC/format-strings-objc-fixit.m deleted file mode 100644 index feaebeec57..0000000000 --- a/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 {} @end -@interface NSString : NSObject - (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/test/SemaObjC/format-strings-objc.m b/test/SemaObjC/format-strings-objc.m index 079460cc76..a1ebf03f8e 100644 --- a/test/SemaObjC/format-strings-objc.m +++ b/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}}