[cfe-commits] r108907 - in /cfe/trunk: lib/Analysis/ScanfFormatString.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings-scanf.c

[Ted Kremenek]

Author: kremenek
Date: Tue Jul 20 15:04:47 2010
New Revision: 108907

URL: http://llvm.org/viewvc/llvm-project?rev=108907&view=rev
Log:
Hookup checking for invalid length modifiers in scanf format strings.

Modified:
    cfe/trunk/lib/Analysis/ScanfFormatString.cpp
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Sema/format-strings-scanf.c

Modified: cfe/trunk/lib/Analysis/ScanfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ScanfFormatString.cpp?rev=108907&r1=108906&r2=108907&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ScanfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/ScanfFormatString.cpp Tue Jul 20 15:04:47 2010
@@ -19,6 +19,7 @@
 using clang::analyze_format_string::FormatStringHandler;
 using clang::analyze_format_string::LengthModifier;
 using clang::analyze_format_string::OptionalAmount;
+using clang::analyze_format_string::ConversionSpecifier;
 using clang::analyze_scanf::ScanfConversionSpecifier;
 using clang::analyze_scanf::ScanfSpecifier;
 
@@ -146,27 +147,28 @@
   switch (*conversionPosition) {
     default:
       break;
-    case '%': k = ScanfConversionSpecifier::PercentArg;   break;
-    case 'A': k = ScanfConversionSpecifier::AArg; break;
-    case 'E': k = ScanfConversionSpecifier::EArg; break;
-    case 'F': k = ScanfConversionSpecifier::FArg; break;
-    case 'G': k = ScanfConversionSpecifier::GArg; break;
-    case 'X': k = ScanfConversionSpecifier::XArg; break;
-    case 'a': k = ScanfConversionSpecifier::aArg; break;
-    case 'd': k = ScanfConversionSpecifier::dArg; break;
-    case 'e': k = ScanfConversionSpecifier::eArg; break;
-    case 'f': k = ScanfConversionSpecifier::fArg; break;
-    case 'g': k = ScanfConversionSpecifier::gArg; break;
-    case 'i': k = ScanfConversionSpecifier::iArg; break;
-    case 'n': k = ScanfConversionSpecifier::nArg; break;
-    case 'c': k = ScanfConversionSpecifier::cArg; break;
-    case 'C': k = ScanfConversionSpecifier::CArg; break;
-    case 'S': k = ScanfConversionSpecifier::SArg; break;
-    case '[': k = ScanfConversionSpecifier::ScanListArg; break;
-    case 'u': k = ScanfConversionSpecifier::uArg; break;
-    case 'x': k = ScanfConversionSpecifier::xArg; break;
-    case 'o': k = ScanfConversionSpecifier::oArg; break;
-    case 's': k = ScanfConversionSpecifier::sArg; break;
+    case '%': k = ConversionSpecifier::PercentArg;   break;
+    case 'A': k = ConversionSpecifier::AArg; break;
+    case 'E': k = ConversionSpecifier::EArg; break;
+    case 'F': k = ConversionSpecifier::FArg; break;
+    case 'G': k = ConversionSpecifier::GArg; break;
+    case 'X': k = ConversionSpecifier::XArg; break;
+    case 'a': k = ConversionSpecifier::aArg; 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 'c': k = ConversionSpecifier::cArg; break;
+    case 'C': k = ConversionSpecifier::CArg; break;
+    case 'S': k = ConversionSpecifier::SArg; break;
+    case '[': k = ConversionSpecifier::ScanListArg; break;
+    case 'u': k = ConversionSpecifier::uArg; break;
+    case 'x': k = ConversionSpecifier::xArg; break;
+    case 'o': k = ConversionSpecifier::oArg; break;
+    case 's': k = ConversionSpecifier::sArg; break;
+    case 'p': k = ConversionSpecifier::pArg; break;
   }
   ScanfConversionSpecifier CS(conversionPosition, k);
   if (k == ScanfConversionSpecifier::ScanListArg) {

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=108907&r1=108906&r2=108907&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Jul 20 15:04:47 2010
@@ -1701,9 +1701,17 @@
     CoveredArgs.set(argIndex);
   }
   
-  // FIXME: Check that the length modifier is valid with the given
-  // conversion specifier.
-  
+  // Check the length modifier is valid with the given conversion specifier.
+  const LengthModifier &LM = FS.getLengthModifier();
+  if (!FS.hasValidLengthModifier()) {
+    S.Diag(getLocationOfByte(LM.getStart()),
+           diag::warn_format_nonsensical_length)
+      << LM.toString() << CS.toString()
+      << getSpecifierRange(startSpecifier, specifierLen)
+      << FixItHint::CreateRemoval(getSpecifierRange(LM.getStart(),
+                                                    LM.getLength()));
+  }
+
   // The remaining checks depend on the data arguments.
   if (HasVAListArg)
     return true;

Modified: cfe/trunk/test/Sema/format-strings-scanf.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-scanf.c?rev=108907&r1=108906&r2=108907&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings-scanf.c (original)
+++ cfe/trunk/test/Sema/format-strings-scanf.c Tue Jul 20 15:04:47 2010
@@ -2,6 +2,7 @@
 
 typedef __typeof(sizeof(int)) size_t;
 typedef struct _FILE FILE;
+typedef __WCHAR_TYPE__ wchar_t;
 
 int fscanf(FILE * restrict, const char * restrict, ...) ;
 int scanf(const char * restrict, ...) ;
@@ -24,3 +25,10 @@
   scanf("%*d", i); // // expected-warning{{data argument not used by format string}}
   scanf("%*d%1$d", i); // no-warning
 }
+
+void bad_length_modifiers(char *s, void *p, wchar_t *ws, long double *ld) {
+  scanf("%hhs", "foo"); // expected-warning{{length modifier 'hh' results in undefined behavior or no effect with 's' conversion specifier}}
+  scanf("%1$zp", p); // expected-warning{{length modifier 'z' results in undefined behavior or no effect with 'p' conversion specifier}}
+  scanf("%ls", ws); // no-warning
+  scanf("%#.2Lf", ld); // expected-warning{{invalid conversion specifier '#'}}
+}