Analysis: Add support for MS specific printf format specifiers

[majnemer]

Summary

Adds support for %I, %I32 and %I64.

[jordan_rose]

This is included in PR13565, so you can track your progress there.

include/clang/Analysis/Analyses/FormatString.h

77

I don't like this name. How about AsIntWord or something? (As I understand it "word" is an overloaded term in MSVC, though.)

lib/Analysis/FormatString.cpp

240

Typo: AsInt32.

544–546

Huh, I didn't realize these were present elsewhere. They don't quite behave the same on Windows, right? See http://llvm.org/bugs/show_bug.cgi?id=13565

lib/Analysis/PrintfFormatString.cpp

313

I don't know if Windows has any ILP64 configurations, but this might need to actually look for "unsigned __int32". At the very least it should use it in the name like "uintmax_t".

(I'm doing my comments backwards, so all of this applies to the signed integers as well.)

319

Ditto -- this should be "unsigned __int64".

325–328

I think just matching AsSizeT here is probably fine, per http://msdn.microsoft.com/en-us/library/tcxf1dw6(v=vs.90).aspx

367–369

It seems like all of these are valid, just not enabled by default.

lib/Analysis/ScanfFormatString.cpp

368

From what I read at http://msdn.microsoft.com/en-us/library/kwwtf9ch(v=vs.71).aspx, %I64n is technically valid.

[hans]

Nice! LGTM too, just a few nits.

include/clang/Analysis/Analyses/FormatString.h

94

Looks like this needs an update too? This is used when emitting fix-it hints for replacing a length modifier.

lib/Analysis/FormatString.cpp

240

AsInt32?

718

It would be nice to have a test for the case when isOSMSVCRT() is false.

[cdavis5x]

Table of format specifiers and their associated character widths with MSVCRT.DLL:

Function	"%s"/"%c"	"%S"/"%C"	"%hs"/"%hc"	"%ls"/"%lc"
printf	Narrow	Wide	Narrow	Wide
wprintf	Wide	Narrow	Narrow	Wide

include/clang/Analysis/Analyses/FormatString.h

77

This type is in fact called __int3264 in MIDL-land.

What about AsIntPtr?

lib/Analysis/FormatString.cpp

544–546

Yeah, they don't. In fact, the behavior of these on Windows is very... uh, weird, to say the least. It depends on which function you're calling (printf or wprintf). There's a handy table above.

This is so, if you include <tchar.h> and call _tprintf, you can pass _TCHAR strings and it will behave correctly. (The macros and types in <tchar.h> behave differently depending on whether or not _UNICODE is #defined. Same for <windows.h> and UNICODE.) Note the non-standard "%hs" and "%hc" formats. They're needed because, in this scheme, there's otherwise no way to always pass a plain ol' char string!

[majnemer]

I've addressed some of these comments inline, I will update the patch with corrections.

lib/Analysis/FormatString.cpp

544–546

They are present elsewhere, all POSIX platforms must have a 'C' and 'S' implementation that desugars to 'lc' and 'ls'

See http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html for more details.

lib/Analysis/PrintfFormatString.cpp

313

They don't have ILP64 configurations and, in fact, can't for numerous reasons relating to ABI/source compatibility. __int32 is just a synonym for int that is vended by the compiler.

325–328

I is not an analog to POSIX's z/t, it's more like their version of UNIX's variable width long type.

[majnemer]

• Address review feedback.

[majnemer]

• Further review comments.

[majnemer]

Jordan gave the A-OK.

include/clang/Analysis/Analyses/FormatString.h

enum Kind {
     AsChar,       // 'hh'
     AsShort,      // 'h'
     AsLong,       // 'l'
     AsLongLong,   // 'll'
     AsQuad,       // 'q' (BSD, deprecated, for 64-bit integer types)
     AsIntMax,     // 'j'
     AsSizeT,      // 'z'
     AsPtrDiff,    // 't'
+    AsInt32,      // 'I32' (MSVCRT, like __int32)
+    AsInt3264,    // 'I'   (MSVCRT, like size_t/ptrdiff_t)
+    AsInt64,      // 'I64' (MSVCRT, like __int64)
     AsLongDouble, // 'L'
     AsAllocate,   // for '%as', GNU extension to C90 scanf
     AsMAllocate,  // for '%ms', GNU extension to scanf
     AsWideChar = AsLong // for '%ls', only makes sense for printf
   };
 
   LengthModifier()
     : Position(0), kind(None) {}
   LengthModifier(const char *pos, Kind k)
     : Position(pos), kind(k) {}
 
   const char *getStart() const {
     return Position;
   }
 
   unsigned getLength() const {
     switch (kind) {
       default:
         return 1;
       case AsLongLong:
       case AsChar:
         return 2;
       case None:
         return 0;

lib/Analysis/FormatString.cpp

case 'a':
       return false;
     case 'm':
       if (IsScanf) {
         lmKind = LengthModifier::AsMAllocate;
         ++I;
         break;
       }
       return false;
+    // printf: AsInt64, AsInt32, AsInt3264
+    // scanf:  AsInt64
+    case 'I':
+      if (I + 1 != E && I + 2 != E) {
+        if (I[1] == '6' && I[2] == '4') {
+          I += 3;
+          lmKind = LengthModifier::AsInt64;
+          break;
+        }
+        if (IsScanf)
+          return false;
+
+        if (I[1] == '3' && I[2] == '2') {
+          I += 3;
+          lmKind = LengthModifier::AsInt64;
+          break;
+        }
+      }
+      ++I;
+      lmKind = LengthModifier::AsInt3264;
+      break;
   }
   LengthModifier lm(lmPosition, lmKind);
   FS.setLengthModifier(lm);
   return true;
 }
 
 //===----------------------------------------------------------------------===//
 // Methods on ArgType.
analyze_format_string::LengthModifier::toString() const {
   case AsQuad:
     return "q";
   case AsIntMax:
     return "j";
   case AsSizeT:
     return "z";
   case AsPtrDiff:
     return "t";
+  case AsInt32:
+    return "I32";
+  case AsInt3264:
+    return "I";
+  case AsInt64:
+    return "I64";
   case AsLongDouble:
     return "L";
   case AsAllocate:
     return "a";
   case AsMAllocate:
     return "m";
   case None:
     return "";
const char *ConversionSpecifier::toString() const {
   case cArg: return "c";
   case sArg: return "s";
   case pArg: return "p";
   case nArg: return "n";
   case PercentArg:  return "%";
   case ScanListArg: return "[";
   case InvalidSpecifier: return NULL;
 
-  // MacOS X unicode extensions.
+  // POSIX unicode extensions.
   case CArg: return "C";
   case SArg: return "S";
 
   // Objective-C specific specifiers.
   case ObjCObjArg: return "@";
 
   // GlibC specific specifiers.
   case PrintErrno: return "m";
   }
   return NULL;
case LengthModifier::AsMAllocate:
         case ConversionSpecifier::CArg:
         case ConversionSpecifier::sArg:
         case ConversionSpecifier::SArg:
         case ConversionSpecifier::ScanListArg:
           return true;
         default:
           return false;
       }
+    case LengthModifier::AsInt32:
+    case LengthModifier::AsInt3264:
+    case LengthModifier::AsInt64:
+      switch (CS.getKind()) {
+        case ConversionSpecifier::dArg:
+        case ConversionSpecifier::iArg:
+        case ConversionSpecifier::oArg:
+        case ConversionSpecifier::uArg:
+        case ConversionSpecifier::xArg:
+        case ConversionSpecifier::XArg:
+          return Target.getTriple().isOSMSVCRT();
+        default:
+          return false;
+      }
   }
   llvm_unreachable("Invalid LengthModifier Kind!");
 }
 
 bool FormatSpecifier::hasStandardLengthModifier() const {
   switch (LM.getKind()) {
     case LengthModifier::None:
     case LengthModifier::AsChar:
     case LengthModifier::AsShort:
     case LengthModifier::AsLong:
     case LengthModifier::AsLongLong:
     case LengthModifier::AsIntMax:
     case LengthModifier::AsSizeT:
     case LengthModifier::AsPtrDiff:
     case LengthModifier::AsLongDouble:
       return true;
     case LengthModifier::AsAllocate:
     case LengthModifier::AsMAllocate:
     case LengthModifier::AsQuad:
+    case LengthModifier::AsInt32:
+    case LengthModifier::AsInt3264:
+    case LengthModifier::AsInt64:
       return false;
   }
   llvm_unreachable("Invalid LengthModifier Kind!");
 }
 
 bool FormatSpecifier::hasStandardConversionSpecifier(const LangOptions &LangOpt) const {
   switch (CS.getKind()) {
     case ConversionSpecifier::cArg:

lib/Analysis/PrintfFormatString.cpp

switch (*conversionPosition) {
     case 'c': k = ConversionSpecifier::cArg; break;
     case 'd': k = ConversionSpecifier::dArg; break;
     case 'e': k = ConversionSpecifier::eArg; break;
     case 'f': k = ConversionSpecifier::fArg; break;
     case 'g': k = ConversionSpecifier::gArg; break;
     case 'i': k = ConversionSpecifier::iArg; break;
     case 'n': k = ConversionSpecifier::nArg; break;
     case 'o': k = ConversionSpecifier::oArg; break;
-    case 'p': k = ConversionSpecifier::pArg;   break;
-    case 's': k = ConversionSpecifier::sArg;      break;
+    case 'p': k = ConversionSpecifier::pArg; break;
+    case 's': k = ConversionSpecifier::sArg; break;
     case 'u': k = ConversionSpecifier::uArg; break;
     case 'x': k = ConversionSpecifier::xArg; break;
     // POSIX specific.
     case 'C': k = ConversionSpecifier::CArg; break;
     case 'S': k = ConversionSpecifier::SArg; break;
     // Objective-C.
     case '@': k = ConversionSpecifier::ObjCObjArg; break;
     // Glibc specific.
switch (LM.getKind()) {
         return ArgType::Invalid();
     }
 
   if (CS.isIntArg())
     switch (LM.getKind()) {
       case LengthModifier::AsLongDouble:
         // GNU extension.
         return Ctx.LongLongTy;
-      case LengthModifier::None: return Ctx.IntTy;
+      case LengthModifier::None:
+      case LengthModifier::AsInt32:
+        return Ctx.IntTy;
       case LengthModifier::AsChar: return ArgType::AnyCharTy;
       case LengthModifier::AsShort: return Ctx.ShortTy;
       case LengthModifier::AsLong: return Ctx.LongTy;
       case LengthModifier::AsLongLong:
       case LengthModifier::AsQuad:
+      case LengthModifier::AsInt64:
         return Ctx.LongLongTy;
       case LengthModifier::AsIntMax:
         return ArgType(Ctx.getIntMaxType(), "intmax_t");
       case LengthModifier::AsSizeT:
         // FIXME: How to get the corresponding signed version of size_t?
         return ArgType();
+      case LengthModifier::AsInt3264:
+        return Ctx.getTargetInfo().getTriple().isArch64Bit() ? Ctx.LongLongTy
+                                                             : Ctx.IntTy;
       case LengthModifier::AsPtrDiff:
         return ArgType(Ctx.getPointerDiffType(), "ptrdiff_t");
       case LengthModifier::AsAllocate:
       case LengthModifier::AsMAllocate:
         return ArgType::Invalid();
     }
 
   if (CS.isUIntArg())
     switch (LM.getKind()) {
       case LengthModifier::AsLongDouble:
         // GNU extension.
         return Ctx.UnsignedLongLongTy;
-      case LengthModifier::None: return Ctx.UnsignedIntTy;
+      case LengthModifier::None:
+      case LengthModifier::AsInt32:
+        return Ctx.UnsignedIntTy;
       case LengthModifier::AsChar: return Ctx.UnsignedCharTy;
       case LengthModifier::AsShort: return Ctx.UnsignedShortTy;
       case LengthModifier::AsLong: return Ctx.UnsignedLongTy;
       case LengthModifier::AsLongLong:
       case LengthModifier::AsQuad:
+      case LengthModifier::AsInt64:
         return Ctx.UnsignedLongLongTy;
       case LengthModifier::AsIntMax:
         return ArgType(Ctx.getUIntMaxType(), "uintmax_t");
       case LengthModifier::AsSizeT:
         return ArgType(Ctx.getSizeType(), "size_t");
+      case LengthModifier::AsInt3264:
+        return Ctx.getTargetInfo().getTriple().isArch64Bit()
+                   ? Ctx.UnsignedLongLongTy
+                   : Ctx.UnsignedIntTy;
       case LengthModifier::AsPtrDiff:
         // FIXME: How to get the corresponding unsigned
         // version of ptrdiff_t?
         return ArgType();
       case LengthModifier::AsAllocate:
       case LengthModifier::AsMAllocate:
         return ArgType::Invalid();
     }
switch (LM.getKind()) {
       case LengthModifier::AsSizeT:
         return ArgType(); // FIXME: ssize_t
       case LengthModifier::AsPtrDiff:
         return ArgType::PtrTo(ArgType(Ctx.getPointerDiffType(), "ptrdiff_t"));
       case LengthModifier::AsLongDouble:
         return ArgType(); // FIXME: Is this a known extension?
       case LengthModifier::AsAllocate:
       case LengthModifier::AsMAllocate:
+      case LengthModifier::AsInt32:
+      case LengthModifier::AsInt3264:
+      case LengthModifier::AsInt64:
         return ArgType::Invalid();
     }
   }
 
   switch (CS.getKind()) {
     case ConversionSpecifier::sArg:
       if (LM.getKind() == LengthModifier::AsWideChar) {
         if (IsObjCLiteral)

lib/Analysis/ScanfFormatString.cpp

case ConversionSpecifier::iArg:
         case LengthModifier::AsChar:
           return ArgType::PtrTo(ArgType::AnyCharTy);
         case LengthModifier::AsShort:
           return ArgType::PtrTo(Ctx.ShortTy);
         case LengthModifier::AsLong:
           return ArgType::PtrTo(Ctx.LongTy);
         case LengthModifier::AsLongLong:
         case LengthModifier::AsQuad:
+        case LengthModifier::AsInt64:
           return ArgType::PtrTo(Ctx.LongLongTy);
         case LengthModifier::AsIntMax:
           return ArgType::PtrTo(ArgType(Ctx.getIntMaxType(), "intmax_t"));
         case LengthModifier::AsSizeT:
           // FIXME: ssize_t.
           return ArgType();
         case LengthModifier::AsPtrDiff:
           return ArgType::PtrTo(ArgType(Ctx.getPointerDiffType(), "ptrdiff_t"));
         case LengthModifier::AsLongDouble:
           // GNU extension.
           return ArgType::PtrTo(Ctx.LongLongTy);
         case LengthModifier::AsAllocate:
-          return ArgType::Invalid();
         case LengthModifier::AsMAllocate:
+        case LengthModifier::AsInt32:
+        case LengthModifier::AsInt3264:
           return ArgType::Invalid();
       }
 
     // Unsigned int.
     case ConversionSpecifier::oArg:
     case ConversionSpecifier::OArg:
     case ConversionSpecifier::uArg:
     case ConversionSpecifier::UArg:
     case ConversionSpecifier::xArg:
     case ConversionSpecifier::XArg:
       switch (LM.getKind()) {
         case LengthModifier::None:
           return ArgType::PtrTo(Ctx.UnsignedIntTy);
         case LengthModifier::AsChar:
           return ArgType::PtrTo(Ctx.UnsignedCharTy);
         case LengthModifier::AsShort:
           return ArgType::PtrTo(Ctx.UnsignedShortTy);
         case LengthModifier::AsLong:
           return ArgType::PtrTo(Ctx.UnsignedLongTy);
         case LengthModifier::AsLongLong:
         case LengthModifier::AsQuad:
+        case LengthModifier::AsInt64:
           return ArgType::PtrTo(Ctx.UnsignedLongLongTy);
         case LengthModifier::AsIntMax:
           return ArgType::PtrTo(ArgType(Ctx.getUIntMaxType(), "uintmax_t"));
         case LengthModifier::AsSizeT:
           return ArgType::PtrTo(ArgType(Ctx.getSizeType(), "size_t"));
         case LengthModifier::AsPtrDiff:
           // FIXME: Unsigned version of ptrdiff_t?
           return ArgType();
         case LengthModifier::AsLongDouble:
           // GNU extension.
           return ArgType::PtrTo(Ctx.UnsignedLongLongTy);
         case LengthModifier::AsAllocate:
-          return ArgType::Invalid();
         case LengthModifier::AsMAllocate:
+        case LengthModifier::AsInt32:
+        case LengthModifier::AsInt3264:
           return ArgType::Invalid();
       }
 
     // Float.
     case ConversionSpecifier::aArg:
     case ConversionSpecifier::AArg:
     case ConversionSpecifier::eArg:
     case ConversionSpecifier::EArg:
case ConversionSpecifier::nArg:
         case LengthModifier::AsSizeT:
           return ArgType(); // FIXME: ssize_t
         case LengthModifier::AsPtrDiff:
           return ArgType::PtrTo(ArgType(Ctx.getPointerDiffType(), "ptrdiff_t"));
         case LengthModifier::AsLongDouble:
           return ArgType(); // FIXME: Is this a known extension?
         case LengthModifier::AsAllocate:
         case LengthModifier::AsMAllocate:
+        case LengthModifier::AsInt32:
+        case LengthModifier::AsInt3264:
+        case LengthModifier::AsInt64:
           return ArgType::Invalid();
         }
 
     default:
       break;
   }
 
   return ArgType();

test/Sema/format-strings-ms.c

+// RUN: %clang_cc1 -fsyntax-only -verify -fms-compatibility -triple=i386-pc-win32 -pedantic %s
+
+int printf(const char *format, ...) __attribute__((format(printf, 1, 2)));
+
+void test() {
+  short val = 30;
+  printf("val = %I64d\n", val); // expected-warning{{'I64' length modifier is not supported by ISO C}} \
+                                // expected-warning{{format specifies type 'long long' but the argument has type 'short'}}
+}