[cfe-commits] r94774 - 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 19:06:55 2010
New Revision: 94774

URL: http://llvm.org/viewvc/llvm-project?rev=94774&view=rev
Log:
Add precision/field width checking to AlternateCheckPrintfString().

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=94774&r1=94773&r2=94774&view=diff

==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/PrintfFormatString.h Thu Jan 28 19:06:55 2010
@@ -104,21 +104,29 @@
 public:
   enum HowSpecified { NotSpecified, Constant, Arg };
 
-  OptionalAmount(HowSpecified h = NotSpecified) : hs(h), amt(0) {}
-  OptionalAmount(unsigned i) : hs(Constant), amt(i) {}
+  OptionalAmount(HowSpecified h, const char *st) 
+    : start(st), hs(h), amt(0) {}
+
+  OptionalAmount()
+    : start(0), hs(NotSpecified), amt(0) {}
+  
+  OptionalAmount(unsigned i, const char *st) 
+    : start(start), hs(Constant), amt(i) {}
 
   HowSpecified getHowSpecified() const { return hs; }
+  bool hasDataArgument() const { return hs == Arg; }
 
   unsigned getConstantAmount() const { 
     assert(hs == Constant);
     return amt;
   }
 
-  unsigned getArgumentsConsumed() {
-    return hs == Arg ? 1 : 0;
+  const char *getStart() const {
+    return start;
   }
   
 private:
+  const char *start;
   HowSpecified hs;
   unsigned amt;
 };
@@ -176,7 +184,7 @@
   bool isLeftJustified() const { return flags & LeftJustified; }
   bool hasPlusPrefix() const { return flags & PlusPrefix; }
   bool hasAlternativeForm() const { return flags & AlternativeForm; }
-  bool hasLeadingZeros() const { return flags & LeadingZeroes; }
+  bool hasLeadingZeros() const { return flags & LeadingZeroes; }  
 };
 
   

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

==============================================================================
--- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Thu Jan 28 19:06:55 2010
@@ -15,7 +15,7 @@
 #include "clang/Analysis/Analyses/PrintfFormatString.h"
 
 using namespace clang;
-using namespace analyze_printf;
+using namespace clang::analyze_printf;
 
 namespace {
 class FormatSpecifierResult {
@@ -70,10 +70,12 @@
     }
 
     if (foundDigits)
-      return OptionalAmount(accumulator);
+      return OptionalAmount(accumulator, Beg);
     
-    if (c == '*')
-      return OptionalAmount(OptionalAmount::Arg);
+    if (c == '*') {
+      ++I;
+      return OptionalAmount(OptionalAmount::Arg, Beg);
+    }
     
     break;
   }

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

==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Jan 28 19:06:55 2010
@@ -1290,14 +1290,20 @@
   const unsigned NumDataArgs;
   const bool IsObjCLiteral;
   const char *Beg; // Start of format string.
+  const bool HasVAListArg;
+  const CallExpr *TheCall;
+  unsigned FormatIdx;
 public:  
   CheckPrintfHandler(Sema &s, const StringLiteral *fexpr,
                      const Expr *origFormatExpr,
                      unsigned numDataArgs, bool isObjCLiteral,
-                     const char *beg)
+                     const char *beg, bool hasVAListArg,
+                     const CallExpr *theCall, unsigned formatIdx)
     : S(s), FExpr(fexpr), OrigFormatExpr(origFormatExpr),
       NumConversions(0), NumDataArgs(numDataArgs),
-      IsObjCLiteral(isObjCLiteral), Beg(beg) {}
+      IsObjCLiteral(isObjCLiteral), Beg(beg),
+      HasVAListArg(hasVAListArg),
+      TheCall(theCall), FormatIdx(formatIdx) {}
     
   void HandleNullChar(const char *nullCharacter);
   
@@ -1307,6 +1313,11 @@
 private:
   SourceRange getFormatRange();
   SourceLocation getLocationOfByte(const char *x);
+  
+  bool HandleAmount(const analyze_printf::OptionalAmount &Amt,
+                    unsigned MissingArgDiag, unsigned BadTypeDiag);
+  
+  const Expr *getDataArg(unsigned i) const;
 };
 }
 
@@ -1325,6 +1336,43 @@
     << getFormatRange();
 }
 
+const Expr *CheckPrintfHandler::getDataArg(unsigned i) const {
+  return TheCall->getArg(FormatIdx + i);  
+}
+
+bool
+CheckPrintfHandler::HandleAmount(const analyze_printf::OptionalAmount &Amt,
+                                 unsigned MissingArgDiag,
+                                 unsigned BadTypeDiag) {
+
+  if (Amt.hasDataArgument()) {
+    ++NumConversions;
+    if (!HasVAListArg) {
+      if (NumConversions > NumDataArgs) {
+        S.Diag(getLocationOfByte(Amt.getStart()), MissingArgDiag)
+          << getFormatRange();      
+        // Don't do any more checking.  We will just emit
+        // spurious errors.
+        return false;
+      }
+      
+      // Type check the data argument.  It should be an 'int'.
+      const Expr *Arg = getDataArg(NumConversions);
+      QualType T = Arg->getType();
+      const BuiltinType *BT = T->getAs<BuiltinType>();
+      if (!BT || BT->getKind() != BuiltinType::Int) {
+        S.Diag(getLocationOfByte(Amt.getStart()), BadTypeDiag)
+          << T << getFormatRange() << Arg->getSourceRange();
+        // Don't do any more checking.  We will just emit
+        // spurious errors.
+        return false;
+      }
+    }
+  }
+  return true;
+}
+                                      
+
 bool
 CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS,
                                           const char *startSpecifier,
@@ -1333,6 +1381,22 @@
   using namespace analyze_printf;
   const ConversionSpecifier &CS = FS.getConversionSpecifier();
 
+  // First check if the field width, precision, and conversion specifier
+  // have matching data arguments.
+  if (!HandleAmount(FS.getFieldWidth(),
+                    diag::warn_printf_asterisk_width_missing_arg,
+                    diag::warn_printf_asterisk_width_wrong_type)) {
+    return false;
+  }
+    
+  if (!HandleAmount(FS.getPrecision(),
+                    diag::warn_printf_asterisk_precision_missing_arg,
+                    diag::warn_printf_asterisk_precision_wrong_type)) {
+    return false;
+  }
+
+  ++NumConversions;  
+  
   // Check for using an Objective-C specific conversion specifier
   // in a non-ObjC literal.
   if (!IsObjCLiteral && CS.isObjCArg()) {
@@ -1377,7 +1441,8 @@
   
   CheckPrintfHandler H(*this, FExpr, OrigFormatExpr,
                        TheCall->getNumArgs() - firstDataArg,
-                       isa<ObjCStringLiteral>(OrigFormatExpr), Str);
+                       isa<ObjCStringLiteral>(OrigFormatExpr), Str,
+                       HasVAListArg, TheCall, format_idx);
 
   analyze_printf::ParseFormatString(H, Str, Str + StrLen);
 }