Extend format string type-checking to include '%p'. Fixes remaining cases PR 4468.

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

diff --git a/include/clang/Analysis/Analyses/PrintfFormatString.h b/include/clang/Analysis/Analyses/PrintfFormatString.h
index 4e23e7c76803d25e92bc93f7d46fbe014a1b0e35..039e5a96e6a5073f557623c5105295f365c7cc29 100644 (file)
--- a/include/clang/Analysis/Analyses/PrintfFormatString.h
+++ b/include/clang/Analysis/Analyses/PrintfFormatString.h
@@ -25,8 +25,8 @@ namespace analyze_printf {
 
 class ArgTypeResult {
 public:
-  enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CStrTy,
-              WCStrTy };
+  enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CPointerTy,
+              CStrTy, WCStrTy };
 private:
   const Kind K;
   QualType T;

diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp
index 99dc22901c75896620e009464c2e7d7ce022ee46..de84af6a55ec8e2a5001d905cbacaec423fd8e35 100644 (file)
--- a/lib/Analysis/PrintfFormatString.cpp
+++ b/lib/Analysis/PrintfFormatString.cpp
@@ -39,7 +39,6 @@ public:
                         const FormatSpecifier &fs)
     : FS(fs), Start(start), Stop(false) {}
 
-
   const char *getStart() const { return Start; }
   bool shouldStop() const { return Stop; }
   bool hasValue() const { return Start != 0; }
@@ -179,7 +178,6 @@ static bool ParseFieldWidth(FormatStringHandler &H, FormatSpecifier &FS,
   return false;
 }
 
-
 static bool ParseArgPosition(FormatStringHandler &H,
                              FormatSpecifier &FS, const char *Start,
                              const char *&Beg, const char *E) {
@@ -424,95 +422,111 @@ FormatStringHandler::~FormatStringHandler() {}
 //===----------------------------------------------------------------------===//
 
 bool ArgTypeResult::matchesType(ASTContext &C, QualType argTy) const {
-  assert(isValid());
-
-  if (K == UnknownTy)
-    return true;
-
-  if (K == SpecificTy) {
-    argTy = C.getCanonicalType(argTy).getUnqualifiedType();
-
-    if (T == argTy)
+  switch (K) {
+    case InvalidTy:
+      assert(false && "ArgTypeResult must be valid");
       return true;
 
-    if (const BuiltinType *BT = argTy->getAs<BuiltinType>())
-      switch (BT->getKind()) {
-        default:
-          break;
-        case BuiltinType::Char_S:
-        case BuiltinType::SChar:
-          return T == C.UnsignedCharTy;
-        case BuiltinType::Char_U:
-        case BuiltinType::UChar:
-          return T == C.SignedCharTy;
-        case BuiltinType::Short:
-          return T == C.UnsignedShortTy;
-        case BuiltinType::UShort:
-          return T == C.ShortTy;
-        case BuiltinType::Int:
-          return T == C.UnsignedIntTy;
-        case BuiltinType::UInt:
-          return T == C.IntTy;
-        case BuiltinType::Long:
-          return T == C.UnsignedLongTy;
-        case BuiltinType::ULong:
-          return T == C.LongTy;
-        case BuiltinType::LongLong:
-          return T == C.UnsignedLongLongTy;
-        case BuiltinType::ULongLong:
-          return T == C.LongLongTy;
-      }
-
-    return false;
-  }
+    case UnknownTy:
+      return true;
 
-  if (K == CStrTy) {
-    const PointerType *PT = argTy->getAs<PointerType>();
-    if (!PT)
+    case SpecificTy: {
+      argTy = C.getCanonicalType(argTy).getUnqualifiedType();
+      if (T == argTy)
+        return true;
+      if (const BuiltinType *BT = argTy->getAs<BuiltinType>())
+        switch (BT->getKind()) {
+          default:
+            break;
+          case BuiltinType::Char_S:
+          case BuiltinType::SChar:
+            return T == C.UnsignedCharTy;
+          case BuiltinType::Char_U:
+          case BuiltinType::UChar:
+            return T == C.SignedCharTy;
+          case BuiltinType::Short:
+            return T == C.UnsignedShortTy;
+          case BuiltinType::UShort:
+            return T == C.ShortTy;
+          case BuiltinType::Int:
+            return T == C.UnsignedIntTy;
+          case BuiltinType::UInt:
+            return T == C.IntTy;
+          case BuiltinType::Long:
+            return T == C.UnsignedLongTy;
+          case BuiltinType::ULong:
+            return T == C.LongTy;
+          case BuiltinType::LongLong:
+            return T == C.UnsignedLongLongTy;
+          case BuiltinType::ULongLong:
+            return T == C.LongLongTy;
+        }
       return false;
+    }
 
-    QualType pointeeTy = PT->getPointeeType();
-
-    if (const BuiltinType *BT = pointeeTy->getAs<BuiltinType>())
-      switch (BT->getKind()) {
-        case BuiltinType::Void:
-        case BuiltinType::Char_U:
-        case BuiltinType::UChar:
-        case BuiltinType::Char_S:
-        case BuiltinType::SChar:
-          return true;
-        default:
-          break;
-      }
-
-    return false;
-  }
+    case CStrTy: {
+      const PointerType *PT = argTy->getAs<PointerType>();
+      if (!PT)
+        return false;
+      QualType pointeeTy = PT->getPointeeType();
+      if (const BuiltinType *BT = pointeeTy->getAs<BuiltinType>())
+        switch (BT->getKind()) {
+          case BuiltinType::Void:
+          case BuiltinType::Char_U:
+          case BuiltinType::UChar:
+          case BuiltinType::Char_S:
+          case BuiltinType::SChar:
+            return true;
+          default:
+            break;
+        }
 
-  if (K == WCStrTy) {
-    const PointerType *PT = argTy->getAs<PointerType>();
-    if (!PT)
       return false;
+    }
 
-    QualType pointeeTy =
-      C.getCanonicalType(PT->getPointeeType()).getUnqualifiedType();
+    case WCStrTy: {
+      const PointerType *PT = argTy->getAs<PointerType>();
+      if (!PT)
+        return false;
+      QualType pointeeTy =
+        C.getCanonicalType(PT->getPointeeType()).getUnqualifiedType();
+      return pointeeTy == C.getWCharType();
+    }
 
-    return pointeeTy == C.getWCharType();
+    case CPointerTy:
+      return argTy->getAs<PointerType>() != NULL ||
+            argTy->getAs<ObjCObjectPointerType>() != NULL;
+
+    case ObjCPointerTy:
+      return argTy->getAs<ObjCObjectPointerType>() != NULL;
   }
 
+  // FIXME: Should be unreachable, but Clang is currently emitting
+  // a warning.
   return false;
 }
 
 QualType ArgTypeResult::getRepresentativeType(ASTContext &C) const {
-  assert(isValid());
-  if (K == SpecificTy)
-    return T;
-  if (K == CStrTy)
-    return C.getPointerType(C.CharTy);
-  if (K == WCStrTy)
-    return C.getPointerType(C.getWCharType());
-  if (K == ObjCPointerTy)
-    return C.ObjCBuiltinIdTy;
-
+  switch (K) {
+    case InvalidTy:
+      assert(false && "No representative type for Invalid ArgTypeResult");
+      // Fall-through.
+    case UnknownTy:
+      return QualType();
+    case SpecificTy:
+      return T;
+    case CStrTy:
+      return C.getPointerType(C.CharTy);
+    case WCStrTy:
+      return C.getPointerType(C.getWCharType());
+    case ObjCPointerTy:
+      return C.ObjCBuiltinIdTy;
+    case CPointerTy:
+      return C.VoidPtrTy;
+  }
+
+  // FIXME: Should be unreachable, but Clang is currently emitting
+  // a warning.
   return QualType();
 }
 
@@ -673,6 +687,8 @@ ArgTypeResult FormatSpecifier::getArgType(ASTContext &Ctx) const {
       return ArgTypeResult::WCStrTy;
     case ConversionSpecifier::CArg:
       return Ctx.WCharTy;
+    case ConversionSpecifier::VoidPtrArg:
+      return ArgTypeResult::CPointerTy;
     default:
       break;
   }

diff --git a/test/Sema/format-strings-fixit.c b/test/Sema/format-strings-fixit.c
index 9a1fef0401ae8d8189009a131212c84c383c7d2f..bbdd4d81059539e63496f98450d4443ca4ec286c 100644 (file)
--- a/test/Sema/format-strings-fixit.c
+++ b/test/Sema/format-strings-fixit.c
@@ -14,6 +14,7 @@ void test() {
   printf("%s", (int) 123);
   printf("abc%0f", "testing testing 123");
   printf("%u", (long) -12);
+  printf("%p", 123);
 
   // Larger types
   printf("%+.2d", (unsigned long long) 123456);

diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c
index d6d37961d922a70c858b38d875ca44d07f8b3523..72aa5927c3077dae702ff5fc55da409025878106 100644 (file)
--- a/test/Sema/format-strings.c
+++ b/test/Sema/format-strings.c
@@ -176,6 +176,7 @@ void test10(int x, float f, int i, long long lli) {
 
 void test11(void *p, char *s) {
   printf("%p", p); // no-warning
+  printf("%p", 123); // expected-warning{{conversion specifies type 'void *' but the argument has type 'int'}}
   printf("%.4p", p); // expected-warning{{precision used in 'p' conversion specifier (where it has no meaning)}}
   printf("%+p", p); // expected-warning{{flag '+' results in undefined behavior in 'p' conversion specifier}}
   printf("% p", p); // expected-warning{{flag ' ' results in undefined behavior in 'p' conversion specifier}}

diff --git a/test/SemaObjC/format-strings-objc.m b/test/SemaObjC/format-strings-objc.m
index 1fcc34f695d80eb621a4704bf9bde68d3543410e..d89f50afa968a8bafa9ebe1dac9a120cb4cb3382 100644 (file)
--- a/test/SemaObjC/format-strings-objc.m
+++ b/test/SemaObjC/format-strings-objc.m
@@ -55,3 +55,11 @@ void rdar_7068334() {
 void rdar_7697748() {
   NSLog(@"%@!"); // expected-warning{{more '%' conversions than data arguments}}
 }
+
+@protocol Foo;
+
+void test_p_conversion_with_objc_pointer(id x, id<Foo> y) {
+  printf("%p", x); // no-warning
+  printf("%p", y); // no-warning
+}
+