[cfe-commits] r157961 - in /cfe/trunk: lib/Analysis/FormatString.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings-enum-fixed-type.cpp test/Sema/format-strings-enum.c

Jordan Rose jordan_rose at apple.com
Mon Jun 4 15:48:58 PDT 2012


Author: jrose
Date: Mon Jun  4 17:48:57 2012
New Revision: 157961

URL: http://llvm.org/viewvc/llvm-project?rev=157961&view=rev
Log:
Teach printf/scanf about enums with fixed underlying types.

Added:
    cfe/trunk/test/Sema/format-strings-enum-fixed-type.cpp
    cfe/trunk/test/Sema/format-strings-enum.c
Modified:
    cfe/trunk/lib/Analysis/FormatString.cpp
    cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/lib/Analysis/FormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/FormatString.cpp?rev=157961&r1=157960&r2=157961&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/FormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/FormatString.cpp Mon Jun  4 17:48:57 2012
@@ -241,6 +241,9 @@
       return true;
       
     case AnyCharTy: {
+      if (const EnumType *ETy = argTy->getAs<EnumType>())
+        argTy = ETy->getDecl()->getIntegerType();
+
       if (const BuiltinType *BT = argTy->getAs<BuiltinType>())
         switch (BT->getKind()) {
           default:
@@ -255,7 +258,10 @@
     }
       
     case SpecificTy: {
+      if (const EnumType *ETy = argTy->getAs<EnumType>())
+        argTy = ETy->getDecl()->getIntegerType();
       argTy = C.getCanonicalType(argTy).getUnqualifiedType();
+
       if (T == argTy)
         return true;
       // Check for "compatible types".

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=157961&r1=157960&r2=157961&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jun  4 17:48:57 2012
@@ -2396,17 +2396,26 @@
   const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context,
                                                            IsObjCLiteral);
   if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
-    // Check if we didn't match because of an implicit cast from a 'char'
-    // or 'short' to an 'int'.  This is done because printf is a varargs
-    // function.
-    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Ex))
-      if (ICE->getType() == S.Context.IntTy ||
-          ICE->getType() == S.Context.UnsignedIntTy) {
-        // All further checking is done on the subexpression.
+    // Look through argument promotions for our error message's reported type.
+    // This includes the integral and floating promotions, but excludes array
+    // and function pointer decay; seeing that an argument intended to be a
+    // string has type 'char [6]' is probably more confusing than 'char *'.
+    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Ex)) {
+      if (ICE->getCastKind() == CK_IntegralCast ||
+          ICE->getCastKind() == CK_FloatingCast) {
         Ex = ICE->getSubExpr();
-        if (ATR.matchesType(S.Context, Ex->getType()))
-          return true;
+
+        // Check if we didn't match because of an implicit cast from a 'char'
+        // or 'short' to an 'int'.  This is done because printf is a varargs
+        // function.
+        if (ICE->getType() == S.Context.IntTy ||
+            ICE->getType() == S.Context.UnsignedIntTy) {
+          // All further checking is done on the subexpression.
+          if (ATR.matchesType(S.Context, Ex->getType()))
+            return true;
+        }
       }
+    }
 
     // We may be able to offer a FixItHint if it is a supported type.
     PrintfSpecifier fixedFS = FS;
@@ -2415,7 +2424,7 @@
 
     if (success) {
       // Get the fix string from the fixed format specifier
-      SmallString<128> buf;
+      SmallString<16> buf;
       llvm::raw_svector_ostream os(buf);
       fixedFS.toString(os);
 

Added: cfe/trunk/test/Sema/format-strings-enum-fixed-type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-enum-fixed-type.cpp?rev=157961&view=auto
==============================================================================
--- cfe/trunk/test/Sema/format-strings-enum-fixed-type.cpp (added)
+++ cfe/trunk/test/Sema/format-strings-enum-fixed-type.cpp Mon Jun  4 17:48:57 2012
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -x c++ -std=c++11 -verify %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -x objective-c -verify %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -x objective-c++ -verify %s
+
+#ifdef __cplusplus
+# define EXTERN_C extern "C"
+#else
+# define EXTERN_C extern
+#endif
+
+EXTERN_C int printf(const char *,...);
+
+typedef enum : short { Constant = 0 } TestEnum;
+// Note that in C (and Objective-C), the type of 'Constant' is 'short'.
+// In C++ (and Objective-C++) it is 'TestEnum'.
+// This is why we don't check for that in the expected output.
+
+void test(TestEnum input) {
+  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has type 'TestEnum'}}
+  printf("%hhd", Constant); // expected-warning{{format specifies type 'char'}}
+  
+  printf("%hd", input); // no-warning
+  printf("%hd", Constant); // no-warning
+
+  // While these are less correct, they are still safe.
+  printf("%d", input); // no-warning
+  printf("%d", Constant); // no-warning
+  
+  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'TestEnum'}}
+  printf("%lld", Constant); // expected-warning{{format specifies type 'long long'}}
+}
+
+
+typedef enum : unsigned long { LongConstant = ~0UL } LongEnum;
+
+void testLong(LongEnum input) {
+  printf("%u", input); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'LongEnum'}}
+  printf("%u", LongConstant); // expected-warning{{format specifies type 'unsigned int'}}
+  
+  printf("%lu", input);
+  printf("%lu", LongConstant);
+}
+
+
+typedef short short_t;
+typedef enum : short_t { ShortConstant = 0 } ShortEnum;
+
+void testUnderlyingTypedef(ShortEnum input) {
+  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has type 'ShortEnum'}}
+  printf("%hhd", ShortConstant); // expected-warning{{format specifies type 'char'}}
+  
+  printf("%hd", input); // no-warning
+  printf("%hd", ShortConstant); // no-warning
+  
+  // While these are less correct, they are still safe.
+  printf("%d", input); // no-warning
+  printf("%d", ShortConstant); // no-warning
+  
+  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'ShortEnum'}}
+  printf("%lld", ShortConstant); // expected-warning{{format specifies type 'long long'}}
+}
+
+
+typedef ShortEnum ShortEnum2;
+
+void testTypedefChain(ShortEnum2 input) {
+  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has type 'ShortEnum2' (aka 'ShortEnum')}}
+  printf("%hd", input); // no-warning
+  printf("%d", input); // no-warning
+  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'ShortEnum2' (aka 'ShortEnum')}}
+}
+
+
+typedef enum : char { CharConstant = 'a' } CharEnum;
+
+// %hhd is deliberately not required to be signed, because 'char' isn't either.
+// This is a separate code path in FormatString.cpp.
+void testChar(CharEnum input) {
+  printf("%hhd", input); // no-warning
+  printf("%hhd", CharConstant); // no-warning
+
+  // This is not correct but it is safe. We warn because '%hd' shows intent.
+  printf("%hd", input); // expected-warning{{format specifies type 'short' but the argument has type 'CharEnum'}}
+  printf("%hd", CharConstant); // expected-warning{{format specifies type 'short'}}
+  
+  // This is not correct but it matches the promotion rules (and is safe).
+  printf("%d", input); // no-warning
+  printf("%d", CharConstant); // no-warning
+  
+  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'CharEnum'}}
+  printf("%lld", CharConstant); // expected-warning{{format specifies type 'long long'}}
+}

Added: cfe/trunk/test/Sema/format-strings-enum.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-enum.c?rev=157961&view=auto
==============================================================================
--- cfe/trunk/test/Sema/format-strings-enum.c (added)
+++ cfe/trunk/test/Sema/format-strings-enum.c Mon Jun  4 17:48:57 2012
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x c++ -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x c++ -std=c++11 -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x objective-c -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x objective-c++ -std=c++11 -verify %s
+
+#ifdef __cplusplus
+# define EXTERN_C extern "C"
+#else
+# define EXTERN_C extern
+#endif
+
+EXTERN_C int printf(const char *,...);
+
+typedef enum { Constant = 0 } TestEnum;
+// Note that in C, the type of 'Constant' is 'int'. In C++ it is 'TestEnum'.
+// This is why we don't check for that in the expected output.
+
+void test(TestEnum input) {
+    printf("%d", input); // no-warning
+    printf("%d", Constant); // no-warning
+
+    printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'TestEnum'}}
+    printf("%lld", Constant); // expected-warning{{format specifies type 'long long'}}
+}
+
+
+typedef enum { LongConstant = ~0UL } LongEnum;
+
+void testLong(LongEnum input) {
+  printf("%u", input); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'LongEnum'}}
+  printf("%u", LongConstant); // expected-warning{{format specifies type 'unsigned int'}}
+  
+  printf("%lu", input);
+  printf("%lu", LongConstant);
+}





More information about the cfe-commits mailing list