[cfe-commits] r94792 - in /cfe/trunk: include/clang/Analysis/Analyses/PrintfFormatString.h lib/Analysis/PrintfFormatString.cpp lib/Sema/SemaChecking.cpp

[Ted Kremenek]

Author: kremenek
Date: Thu Jan 28 20:40:24 2010
New Revision: 94792

URL: http://llvm.org/viewvc/llvm-project?rev=94792&view=rev
Log:
Alternate format string checking: issue a warning for invalid conversion specifiers.

Modified:
    cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h
    cfe/trunk/lib/Analysis/PrintfFormatString.cpp
    cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h?rev=94792&r1=94791&r2=94792&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h Thu Jan 28 20:40:24 2010
@@ -200,7 +200,9 @@
   
   virtual void HandleIncompletePrecision(const char *periodChar) {}
   
-  virtual void HandleInvalidConversionSpecifier(const char *conversionChar) {}
+  virtual void HandleInvalidConversionSpecifier(const FormatSpecifier &FS,
+                                                const char *startSpecifier,
+                                                unsigned specifierLen) {}
   
   virtual bool HandleFormatSpecifier(const FormatSpecifier &FS,
                                      const char *startSpecifier,

Modified: cfe/trunk/lib/Analysis/PrintfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=94792&r1=94791&r2=94792&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Thu Jan 28 20:40:24 2010
@@ -21,17 +21,17 @@
 class FormatSpecifierResult {
   FormatSpecifier FS;
   const char *Start;
-  bool HasError;
+  bool Stop;
 public:
-  FormatSpecifierResult(bool err = false)
-    : Start(0), HasError(err) {}
+  FormatSpecifierResult(bool stop = false)
+    : Start(0), Stop(stop) {}
   FormatSpecifierResult(const char *start,
                         const FormatSpecifier &fs)
-    : FS(fs), Start(start), HasError(false) {}
+    : FS(fs), Start(start), Stop(false) {}
 
   
   const char *getStart() const { return Start; }
-  bool hasError() const { return HasError; }
+  bool shouldStop() const { return Stop; }
   bool hasValue() const { return Start != 0; }
   const FormatSpecifier &getValue() const {
     assert(hasValue());
@@ -194,11 +194,10 @@
   
   // Finally, look for the conversion specifier.
   const char *conversionPosition = I++;
-  ConversionSpecifier::Kind k;
+  ConversionSpecifier::Kind k = ConversionSpecifier::InvalidSpecifier;
   switch (*conversionPosition) {
     default:
-      H.HandleInvalidConversionSpecifier(conversionPosition);
-      return true;      
+      break;
     // C99: 7.19.6.1 (section 8).
     case 'd': k = ConversionSpecifier::dArg; break;
     case 'i': k = ConversionSpecifier::iArg; break;
@@ -223,6 +222,11 @@
     case '@': k = ConversionSpecifier::ObjCObjArg; break;      
   }
   FS.setConversionSpecifier(ConversionSpecifier(conversionPosition, k));
+
+  if (k == ConversionSpecifier::InvalidSpecifier) {
+    H.HandleInvalidConversionSpecifier(FS, Beg, I - Beg);
+    return false; // Keep processing format specifiers.
+  }
   return FormatSpecifierResult(Start, FS);
 }
 
@@ -232,13 +236,14 @@
   // Keep looking for a format specifier until we have exhausted the string.
   while (I != E) {
     const FormatSpecifierResult &FSR = ParseFormatSpecifier(H, I, E);
-    // Did an error of any kind occur when parsing the specifier?  If so,
-    // don't do any more processing.
-    if (FSR.hasError())
+    // Did a fail-stop error of any kind occur when parsing the specifier?
+    // If so, don't do any more processing.
+    if (FSR.shouldStop())
       return true;;
-    // Done processing the string?
+    // Did we exhaust the string or encounter an error that
+    // we can recover from?
     if (!FSR.hasValue())
-      break;    
+      continue;
     // We have a format specifier.  Pass it to the callback.
     if (!H.HandleFormatSpecifier(FSR.getValue(), FSR.getStart(),
                                  I - FSR.getStart()))

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=94792&r1=94791&r2=94792&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Jan 28 20:40:24 2010
@@ -1306,7 +1306,16 @@
       TheCall(theCall), FormatIdx(formatIdx) {}
   
   void DoneProcessing();
-    
+   
+//  void HandleIncompleteFormatSpecifier(const char *startSpecifier,
+//                                       const char *endSpecifier);
+  
+//  void HandleIncompletePrecision(const char *periodChar);
+  
+  void HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
+                                        const char *startSpecifier,
+                                        unsigned specifierLen);
+  
   void HandleNullChar(const char *nullCharacter);
   
   bool HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS,
@@ -1331,6 +1340,20 @@
   return S.getLocationOfStringLiteralByte(FExpr, x - Beg);  
 }
 
+void CheckPrintfHandler::
+HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
+                                 const char *startSpecifier,
+                                 unsigned specifierLen) {
+  
+  ++NumConversions;
+  
+  SourceLocation Loc =
+    getLocationOfByte(FS.getConversionSpecifier().getStart());
+  S.Diag(Loc, diag::warn_printf_invalid_conversion)
+      << llvm::StringRef(startSpecifier, specifierLen)
+      << getFormatRange();  
+}
+
 void CheckPrintfHandler::HandleNullChar(const char *nullCharacter) {
   // The presence of a null character is likely an error.
   S.Diag(getLocationOfByte(nullCharacter),
@@ -1373,7 +1396,6 @@
   }
   return true;
 }
-                                      
 
 bool
 CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS,
@@ -1397,20 +1419,17 @@
     return false;
   }
 
-  ++NumConversions;  
-  
   // Check for using an Objective-C specific conversion specifier
   // in a non-ObjC literal.
   if (!IsObjCLiteral && CS.isObjCArg()) {
-    SourceLocation Loc = getLocationOfByte(CS.getStart());
-    S.Diag(Loc, diag::warn_printf_invalid_conversion)
-      << llvm::StringRef(startSpecifier, specifierLen)
-      << getFormatRange();
+    HandleInvalidConversionSpecifier(FS, startSpecifier, specifierLen);
     
     // Continue checking the other format specifiers.
     return true;
   }
   
+  ++NumConversions;  
+  
   // Are we using '%n'?  Issue a warning about this being
   // a possible security issue.
   if (CS.getKind() == ConversionSpecifier::OutIntPtrArg) {