OK. I will do that. > On Mar 11, 2016, at 4:15 PM, David Blaikie <dblai...@gmail.com> wrote: > > > > On Fri, Mar 11, 2016 at 4:14 PM, Bob Wilson via cfe-commits > <cfe-commits@lists.llvm.org <mailto: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 >> <mailto:tha...@chromium.org>> wrote: >> >> I think http://clang.llvm.org/docs/InternalsManual.html#fix-it-hints >> <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 <mailto: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 >> <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 >> >> <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 >> >> <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 >> >> <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 <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >> > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > <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
