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 <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);
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 &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;
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to