Add basic type checking of format string conversion specifiers and their arguments. Thanks to Cristian Draghici for his help with this patch!

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

include/clang/Basic/DiagnosticSemaKinds.td

@@ -2449,6 +2449,9 @@ def warn_printf_incomplete_specifier : Warning<
   "incomplete format specifier">, InGroup<Format>;
 def warn_printf_missing_format_string : Warning<
   "format string missing">, InGroup<Format>;
+def warn_printf_conversion_argument_type_mismatch : Warning<
+  "conversion specifies type %0 but the argument has type %1">,
+  InGroup<Format>;
 def warn_null_arg : Warning<
   "null passed to a callee which requires a non-null argument">,
   InGroup<NonNull>;
@@ -2463,10 +2466,10 @@ def warn_printf_asterisk_width_missing_arg : Warning<
 def warn_printf_asterisk_precision_missing_arg : Warning<
   "'.*' specified field precision is missing a matching 'int' argument">;
 def warn_printf_asterisk_width_wrong_type : Warning<
-  "field width should have type 'int', but argument has type %0">,
+  "field width should have type %0, but argument has type %1">,
   InGroup<Format>;
 def warn_printf_asterisk_precision_wrong_type : Warning<
-  "field precision should have type 'int', but argument has type %0">,
+  "field precision should have type %0, but argument has type %1">,
   InGroup<Format>;
 
 // CHECK: returning address/reference of stack memory

lib/Sema/SemaChecking.cpp

@@ -1081,6 +1081,8 @@ private:
                     unsigned MissingArgDiag, unsigned BadTypeDiag,
 					const char *startSpecifier, unsigned specifierLen);
   
+  bool MatchType(QualType A, QualType B, bool ignoreSign);
+  
   const Expr *getDataArg(unsigned i) const;
 };
 }
@@ -1132,6 +1134,47 @@ const Expr *CheckPrintfHandler::getDataArg(unsigned i) const {
   return TheCall->getArg(FormatIdx + i);  
 }
 
+bool CheckPrintfHandler::MatchType(QualType A, QualType B, bool ignoreSign) {
+  A = S.Context.getCanonicalType(A).getUnqualifiedType();
+  B = S.Context.getCanonicalType(B).getUnqualifiedType();
+  
+  if (A == B)
+    return true;
+  
+  if (ignoreSign) {
+    if (const BuiltinType *BT = B->getAs<BuiltinType>()) {
+      switch (BT->getKind()) {
+        default:
+          return false;
+        case BuiltinType::Char_S:          
+        case BuiltinType::SChar:
+          return A == S.Context.UnsignedCharTy;
+        case BuiltinType::Char_U:
+        case BuiltinType::UChar:
+          return A == S.Context.SignedCharTy;
+        case BuiltinType::Short:
+          return A == S.Context.UnsignedShortTy;
+        case BuiltinType::UShort:
+          return A == S.Context.ShortTy;          
+        case BuiltinType::Int:
+          return A == S.Context.UnsignedIntTy;
+        case BuiltinType::UInt:
+          return A == S.Context.IntTy;
+        case BuiltinType::Long:
+          return A == S.Context.UnsignedLongTy;
+        case BuiltinType::ULong:
+          return A == S.Context.LongTy;
+        case BuiltinType::LongLong:
+          return A == S.Context.UnsignedLongLongTy;
+        case BuiltinType::ULongLong:
+          return A == S.Context.LongLongTy;          
+      }
+      return A == B;
+    }
+  }
+  return false;  
+}
+
 bool
 CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt,
                                  unsigned MissingArgDiag,
@@ -1156,13 +1199,11 @@ CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt,
       // doesn't emit a warning for that case.
       const Expr *Arg = getDataArg(NumConversions);
       QualType T = Arg->getType();
-      const BuiltinType *BT = T->getAs<BuiltinType>();            
-      if (!BT || (BT->getKind() != BuiltinType::Int &&
-                  BT->getKind() != BuiltinType::UInt)) {
+      if (!MatchType(T, S.Context.IntTy, true)) {
         S.Diag(getLocationOfByte(Amt.getStart()), BadTypeDiag)
-          << T
-		  << getFormatSpecifierRange(startSpecifier, specifierLen)
-		  << Arg->getSourceRange();
+          << S.Context.IntTy << T
+          << getFormatSpecifierRange(startSpecifier, specifierLen)
+          << Arg->getSourceRange();
         // Don't do any more checking.  We will just emit
         // spurious errors.
         return false;
@@ -1234,6 +1275,22 @@ CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier
     // Don't do any more checking.
     return false;
   }
+  
+  // Now type check the data expression that matches the
+  // format specifier.
+  const Expr *Ex = getDataArg(NumConversions);
+  const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context);
+    
+  if (const QualType *T = ATR.getSpecificType()) {
+    if (!MatchType(*T, Ex->getType(), true)) {
+      S.Diag(getLocationOfByte(CS.getStart()),
+             diag::warn_printf_conversion_argument_type_mismatch)
+        << *T << Ex->getType()
+        << getFormatSpecifierRange(startSpecifier, specifierLen)
+        << Ex->getSourceRange();
+    }
+    return true;
+  }
 
   return true;
 }

test/Sema/format-strings.c

@@ -71,8 +71,8 @@ void check_invalid_specifier(FILE* fp, char *buf)
 {
   printf("%s%lb%d","unix",10,20); // expected-warning {{invalid conversion specifier 'b'}}
   fprintf(fp,"%%%l"); // expected-warning {{incomplete format specifier}}
-  sprintf(buf,"%%%%%ld%d%d", 1, 2, 3); // no-warning
-  snprintf(buf, 2, "%%%%%ld%;%d", 1, 2, 3); // expected-warning {{invalid conversion specifier ';'}}
+  sprintf(buf,"%%%%%ld%d%d", 1, 2, 3); // expected-warning{{conversion specifies type 'long' but the argument has type 'int'}}
+  snprintf(buf, 2, "%%%%%ld%;%d", 1, 2, 3); // expected-warning{{conversion specifies type 'long' but the argument has type 'int'}} expected-warning {{invalid conversion specifier ';'}}
 }
 
 void check_null_char_string(char* b)
@@ -162,3 +162,11 @@ void test10(int x, float f, int i) {
   printf("%.", x);  // expected-warning{{incomplete format specifier}}
 } 
 
+typedef struct __aslclient *aslclient;
+typedef struct __aslmsg *aslmsg;
+int asl_log(aslclient asl, aslmsg msg, int level, const char *format, ...) __attribute__((__format__ (__printf__, 4, 5)));
+void test_asl(aslclient asl) {
+  // Test case from <rdar://problem/7341605>.
+  asl_log(asl, 0, 3, "Error: %m"); // no-warning
+  asl_log(asl, 0, 3, "Error: %W"); // expected-warning{{invalid conversion specifier 'W'}}
+}

test/Sema/ucn-cstring.c

@@ -5,8 +5,8 @@ int printf(const char *, ...);
 int main(void) {
   int a[sizeof("hello \u2192 \u2603 \u2190 world") == 24 ? 1 : -1];
   
-  printf("%s (%d)\n", "hello \u2192 \u2603 \u2190 world", sizeof("hello \u2192 \u2603 \u2190 world"));
-  printf("%s (%d)\n", "\U00010400\U0001D12B", sizeof("\U00010400\U0001D12B"));
+  printf("%s (%zd)\n", "hello \u2192 \u2603 \u2190 world", sizeof("hello \u2192 \u2603 \u2190 world"));
+  printf("%s (%zd)\n", "\U00010400\U0001D12B", sizeof("\U00010400\U0001D12B"));
   // Some error conditions...
   printf("%s\n", "\U"); // expected-error{{\u used with no following hex digits}}
   printf("%s\n", "\U00"); // expected-error{{incomplete universal character name}}