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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@172739 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp
index b52433a0a4ef64875963506045273d427336e59a..5d37de362a41fd95943f5150522a65bbf27db828 100644 (file)
--- a/lib/Analysis/PrintfFormatString.cpp
+++ b/lib/Analysis/PrintfFormatString.cpp
@@ -499,8 +499,26 @@ bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
   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;

diff --git a/test/FixIt/format-darwin.m b/test/FixIt/format-darwin.m
index 46b6705f15810a9448f9f2d0cdc969b8b39e36d5..cfaac29e904b12b38c8628c130477ab94258dd87 100644 (file)
--- a/test/FixIt/format-darwin.m
+++ b/test/FixIt/format-darwin.m
@@ -118,6 +118,16 @@ void testPreserveHex() {
   // 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 @@ void testNoWarn() {
   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
 
 

diff --git a/test/FixIt/format.m b/test/FixIt/format.m
index 58c553a01094d0bc99bdda3c378ddfa14aa40fdc..f8ca0e124de7736c7359f38c0e376c2c15fe136c 100644 (file)
--- a/test/FixIt/format.m
+++ b/test/FixIt/format.m
@@ -213,3 +213,14 @@ void test_percent_C() {
   // 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"
+}