From: Ted Kremenek <kremenek@apple.com>
Date: Fri, 29 Jan 2010 01:06:55 +0000 (+0000)
Subject: Add precision/field width checking to AlternateCheckPrintfString().
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0d27735c51f5bd392e673cf39a675e14e9442387;p=clang

Add precision/field width checking to AlternateCheckPrintfString().


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

diff --git a/include/clang/Analysis/Analyses/PrintfFormatString.h b/include/clang/Analysis/Analyses/PrintfFormatString.h
index 45d1055be6..151a06ebc0 100644
--- a/include/clang/Analysis/Analyses/PrintfFormatString.h
+++ b/include/clang/Analysis/Analyses/PrintfFormatString.h
@@ -104,21 +104,29 @@ class OptionalAmount {
 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 @@ public:
   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; }  
 };
 
   
diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp
index 986411fc7d..1a95e0b882 100644
--- a/lib/Analysis/PrintfFormatString.cpp
+++ b/lib/Analysis/PrintfFormatString.cpp
@@ -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 @@ static OptionalAmount ParseAmount(const char *&Beg, const char *E) {
     }
 
     if (foundDigits)
-      return OptionalAmount(accumulator);
+      return OptionalAmount(accumulator, Beg);
     
-    if (c == '*')
-      return OptionalAmount(OptionalAmount::Arg);
+    if (c == '*') {
+      ++I;
+      return OptionalAmount(OptionalAmount::Arg, Beg);
+    }
     
     break;
   }
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 3589affaf3..f34d2388b4 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1290,14 +1290,20 @@ class CheckPrintfHandler : public analyze_printf::FormatStringHandler {
   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 @@ public:
 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 @@ void CheckPrintfHandler::HandleNullChar(const char *nullCharacter) {
     << 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 @@ CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier
   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 @@ Sema::AlternateCheckPrintfString(const StringLiteral *FExpr,
   
   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);
 }