On Fri, Mar 11, 2016 at 4:14 PM, Bob Wilson via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> I’m not sure how to interpret that. It says: "Clang must recover from > errors as if the fix-it had been applied.” I suppose that format-security > could be an error if you’re building with -Werror, but I had been > interpreting that to mean an error that would block further compilation. > Can someone clarify this expectation? > > I don’t think we can change -Wformat-security to be a note. > The suggestion isn't to change it to be a note, but to keep the warning but move the fixit to a note associated with the warning. That way -fix-it doesn't automatically apply the fix-it, which avoids the issue of recovery as-if the fixit was applied. > It is an existing warning, and people expect us to match GCC on it as well. > > > On Mar 11, 2016, at 4:03 PM, Nico Weber <tha...@chromium.org> wrote: > > I think http://clang.llvm.org/docs/InternalsManual.html#fix-it-hints says > that if a fixit is on a warning, then clang should process the code as if > the fixit had been applied. That's not the case here, so I think the fixit > should be on a note instead. > > On Fri, Mar 11, 2016 at 4:55 PM, Bob Wilson via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: bwilson >> Date: Fri Mar 11 15:55:37 2016 >> New Revision: 263299 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=263299&view=rev >> Log: >> Add fix-it for format-security warnings. >> >> Added: >> cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m >> Modified: >> cfe/trunk/lib/Sema/SemaChecking.cpp >> cfe/trunk/test/Sema/format-strings-fixit.c >> >> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=263299&r1=263298&r2=263299&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Mar 11 15:55:37 2016 >> @@ -3621,20 +3621,32 @@ bool Sema::CheckFormatArguments(ArrayRef >> // format is either NSString or CFString. This is a hack to prevent >> // diag when using the NSLocalizedString and CFCopyLocalizedString >> macros >> // which are usually used in place of NS and CF string literals. >> - if (Type == FST_NSString && >> - SourceMgr.isInSystemMacro(Args[format_idx]->getLocStart())) >> + SourceLocation FormatLoc = Args[format_idx]->getLocStart(); >> + if (Type == FST_NSString && SourceMgr.isInSystemMacro(FormatLoc)) >> return false; >> >> // If there are no arguments specified, warn with -Wformat-security, >> otherwise >> // warn only with -Wformat-nonliteral. >> - if (Args.size() == firstDataArg) >> - Diag(Args[format_idx]->getLocStart(), >> - diag::warn_format_nonliteral_noargs) >> + if (Args.size() == firstDataArg) { >> + const SemaDiagnosticBuilder &D = >> + Diag(FormatLoc, diag::warn_format_nonliteral_noargs); >> + switch (Type) { >> + default: >> + D << OrigFormatExpr->getSourceRange(); >> + break; >> + case FST_Kprintf: >> + case FST_FreeBSDKPrintf: >> + case FST_Printf: >> + D << FixItHint::CreateInsertion(FormatLoc, "\"%s\", "); >> + break; >> + case FST_NSString: >> + D << FixItHint::CreateInsertion(FormatLoc, "@\"%@\", "); >> + break; >> + } >> + } else { >> + Diag(FormatLoc, diag::warn_format_nonliteral) >> << OrigFormatExpr->getSourceRange(); >> - else >> - Diag(Args[format_idx]->getLocStart(), >> - diag::warn_format_nonliteral) >> - << OrigFormatExpr->getSourceRange(); >> + } >> return false; >> } >> >> >> 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=263299&r1=263298&r2=263299&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Sema/format-strings-fixit.c (original) >> +++ cfe/trunk/test/Sema/format-strings-fixit.c Fri Mar 11 15:55:37 2016 >> @@ -16,6 +16,8 @@ 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); >> @@ -94,6 +96,9 @@ 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 *, ...); >> @@ -218,6 +223,7 @@ 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); >> >> Added: 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=263299&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m (added) >> +++ cfe/trunk/test/SemaObjC/format-strings-objc-fixit.m Fri Mar 11 >> 15:55:37 2016 >> @@ -0,0 +1,31 @@ >> +// 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); >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
