r263299 - Add fix-it for format-security warnings.
Bob Wilson via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 15 14:03:39 PDT 2016
I think r263584 does what you suggest. Let me know if not.
> On Mar 11, 2016, at 4:53 PM, Bob Wilson via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
> OK. I will do that.
>
>> On Mar 11, 2016, at 4:15 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>
>>
>>
>> On Fri, Mar 11, 2016 at 4:14 PM, Bob Wilson via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at 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 <thakis at chromium.org <mailto:thakis at 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 at lists.llvm.org <mailto:cfe-commits at 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;
>>> + 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);
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org <mailto:cfe-commits at 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 at lists.llvm.org <mailto:cfe-commits at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160315/8f5b8c05/attachment-0001.html>
More information about the cfe-commits
mailing list