bob.wilson created this revision.
bob.wilson added reviewers: bcraig, rjmccall, dblaikie.
bob.wilson added a subscriber: cfe-commits.
Herald added a subscriber: mcrosier.
The format-security warning is a special case of format-nonliteral that applies
when there are no arguments besides the format string. In those cases, for
printf and NSLog-style functions, there is an easy fix to provide a literal
format string of "%s" (or @"%@" for Objective-C), with the nonliteral string as
the argument. This patch teaches clang to provide fix-its for those cases.
http://reviews.llvm.org/D17941
Files:
lib/Sema/SemaChecking.cpp
test/Sema/format-strings-fixit.c
test/SemaObjC/format-strings-objc-fixit.m
Index: test/SemaObjC/format-strings-objc-fixit.m
===
--- /dev/null
+++ test/SemaObjC/format-strings-objc-fixit.m
@@ -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 {} @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);
Index: test/Sema/format-strings-fixit.c
===
--- test/Sema/format-strings-fixit.c
+++ test/Sema/format-strings-fixit.c
@@ -16,6 +16,8 @@
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 @@
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 @@
// 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);
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3621,20 +3621,32 @@
// 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 =
+ 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;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits