[cfe-commits] r172739 - in /cfe/trunk: lib/Analysis/PrintfFormatString.cpp test/FixIt/format-darwin.m test/FixIt/format.m

[Jordan Rose]

Author: jrose
Date: Thu Jan 17 12:47:16 2013
New Revision: 172739

URL: http://llvm.org/viewvc/llvm-project?rev=172739&view=rev
Log:
Format strings: correct signedness if already correcting width (%d,%u).

It is valid to do this:
  printf("%u", (int)x);

But if we see this:
  printf("%lu", (int)x);

...our fixit should suggest %d, not %u.

Modified:
    cfe/trunk/lib/Analysis/PrintfFormatString.cpp
    cfe/trunk/test/FixIt/format-darwin.m
    cfe/trunk/test/FixIt/format.m

Modified: cfe/trunk/lib/Analysis/PrintfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=172739&r1=172738&r2=172739&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Thu Jan 17 12:47:16 2013
@@ -499,8 +499,26 @@
   if (isa<TypedefType>(QT) && (LangOpt.C99 || LangOpt.CPlusPlus11))
     namedTypeToLengthModifier(QT, LM);
 
-  // If fixing the length modifier was enough, we are done.
+  // If fixing the length modifier was enough, we might be done.
   if (hasValidLengthModifier(Ctx.getTargetInfo())) {
+    // If we're going to offer a fix anyway, make sure the sign matches.
+    switch (CS.getKind()) {
+    case ConversionSpecifier::uArg:
+    case ConversionSpecifier::UArg:
+      if (QT->isSignedIntegerType())
+        CS.setKind(clang::analyze_format_string::ConversionSpecifier::dArg);
+      break;
+    case ConversionSpecifier::dArg:
+    case ConversionSpecifier::DArg:
+    case ConversionSpecifier::iArg:
+      if (QT->isUnsignedIntegerType())
+        CS.setKind(clang::analyze_format_string::ConversionSpecifier::uArg);
+      break;
+    default:
+      // Other specifiers do not have signed/unsigned variants.
+      break;
+    }
+
     const analyze_printf::ArgType &ATR = getArgType(Ctx, IsObjCLiteral);
     if (ATR.isValid() && ATR.matchesType(Ctx, QT))
       return true;

Modified: cfe/trunk/test/FixIt/format-darwin.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/format-darwin.m?rev=172739&r1=172738&r2=172739&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/format-darwin.m (original)
+++ cfe/trunk/test/FixIt/format-darwin.m Thu Jan 17 12:47:16 2013
@@ -118,6 +118,16 @@
   // CHECK-64: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:"(unsigned long)"
 }
 
+void testSignedness(NSInteger i, NSUInteger u) {
+  printf("%d", u); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+  printf("%i", u); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+  printf("%u", i); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+
+  // CHECK-64: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%lu"
+  // CHECK-64: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%lu"
+  // CHECK-64: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%ld"
+}
+
 void testNoWarn() {
   printf("%ld", getNSInteger()); // no-warning
   printf("%lu", getNSUInteger()); // no-warning
@@ -154,6 +164,14 @@
   printf("%lu", getUInt32()); // no-warning
 }
 
+void testSignedness(NSInteger i, NSUInteger u) {
+  // It is valid to use a specifier with the opposite signedness as long as
+  // the type is correct.
+  printf("%d", u); // no-warning
+  printf("%i", u); // no-warning
+  printf("%u", i); // no-warning
+}
+
 #endif
 
 

Modified: cfe/trunk/test/FixIt/format.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/format.m?rev=172739&r1=172738&r2=172739&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/format.m (original)
+++ cfe/trunk/test/FixIt/format.m Thu Jan 17 12:47:16 2013
@@ -213,3 +213,14 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:22}:"(unichar)"
 }
+
+
+void testSignedness(long i, unsigned long u) {
+  printf("%d", u); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned long'}}
+  printf("%i", u); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned long'}}
+  printf("%u", i); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'long'}}
+
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%lu"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%lu"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%ld"
+}