[cfe-commits] r109428 - in /cfe/trunk: include/clang/Analysis/Analyses/FormatString.h lib/Analysis/FormatString.cpp lib/Analysis/ScanfFormatString.cpp lib/Sema/SemaChecking.cpp

Ted Kremenek kremenek at apple.com
Mon Jul 26 12:45:54 PDT 2010


Author: kremenek
Date: Mon Jul 26 14:45:54 2010
New Revision: 109428

URL: http://llvm.org/viewvc/llvm-project?rev=109428&view=rev
Log:
Hoist argument type checking into CheckFormatHandler.  This is prep for scanf format
string argument type checking.

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

Modified: cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/FormatString.h?rev=109428&r1=109427&r2=109428&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/FormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/FormatString.h Mon Jul 26 14:45:54 2010
@@ -305,6 +305,8 @@
 public:
   FormatSpecifier(bool isPrintf)
     : CS(isPrintf), UsesPositionalArg(false), argIndex(0) {}
+  
+  virtual ~FormatSpecifier();
 
   void setLengthModifier(LengthModifier lm) {
     LM = lm;
@@ -339,6 +341,17 @@
   bool usesPositionalArg() const { return UsesPositionalArg; }
   
   bool hasValidLengthModifier() const;
+  
+  /// \brief Returns the type that a data argument
+  /// paired with this format specifier should have.  This method
+  /// will an invalid ArgTypeResult if the format specifier does not have
+  /// a matching data argument or the matching argument matches
+  /// more than one type.
+  virtual ArgTypeResult getArgType(ASTContext &Ctx) const = 0;
+  
+  const ConversionSpecifier &getConversionSpecifier() const {
+    return CS;
+  }
 };
 
 } // end analyze_format_string namespace
@@ -438,9 +451,9 @@
     return getConversionSpecifier().consumesDataArgument();
   }
 
-  /// \brief Returns the builtin type that a data argument
+  /// \brief Returns the type that a data argument
   /// paired with this format specifier should have.  This method
-  /// will return null if the format specifier does not have
+  /// will an invalid ArgTypeResult if the format specifier does not have
   /// a matching data argument or the matching argument matches
   /// more than one type.
   ArgTypeResult getArgType(ASTContext &Ctx) const;
@@ -468,6 +481,11 @@
 
   bool hasValidPrecision() const;
   bool hasValidFieldWidth() const;
+  
+  static bool classof(const analyze_format_string::FormatSpecifier *FS) {
+    return FS->getConversionSpecifier().isPrintfKind();
+  }
+  
 };
 }  // end analyze_printf namespace
 
@@ -492,6 +510,7 @@
   }      
 };
 
+using analyze_format_string::ArgTypeResult;
 using analyze_format_string::LengthModifier;
 using analyze_format_string::OptionalAmount;
 using analyze_format_string::OptionalFlag;
@@ -523,6 +542,13 @@
   bool consumesDataArgument() const {
     return CS.consumesDataArgument() && !SuppressAssignment;
   }
+  
+  /// \brief Returns the type that a data argument
+  /// paired with this format specifier should have.  This method
+  /// will an invalid ArgTypeResult if the format specifier does not have
+  /// a matching data argument or the matching argument matches
+  /// more than one type.
+  ArgTypeResult getArgType(ASTContext &Ctx) const;
 
   static ScanfSpecifier Parse(const char *beg, const char *end);
 };

Modified: cfe/trunk/lib/Analysis/FormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/FormatString.cpp?rev=109428&r1=109427&r2=109428&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/FormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/FormatString.cpp Mon Jul 26 14:45:54 2010
@@ -379,9 +379,11 @@
 }
 
 //===----------------------------------------------------------------------===//
-// Methods on ConversionSpecifier.
+// Methods on FormatSpecifier.
 //===----------------------------------------------------------------------===//
 
+FormatSpecifier::~FormatSpecifier() {}
+
 bool FormatSpecifier::hasValidLengthModifier() const {
   switch (LM.getKind()) {
     case LengthModifier::None:

Modified: cfe/trunk/lib/Analysis/ScanfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ScanfFormatString.cpp?rev=109428&r1=109427&r2=109428&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ScanfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/ScanfFormatString.cpp Mon Jul 26 14:45:54 2010
@@ -217,4 +217,10 @@
   return false;
 }
 
+ArgTypeResult ScanfSpecifier::getArgType(ASTContext &Ctx) const {
+  // FIXME: Fill in.
+  return ArgTypeResult();
+}
+
+
 

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=109428&r1=109427&r2=109428&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jul 26 14:45:54 2010
@@ -1174,6 +1174,11 @@
                     const analyze_format_string::ConversionSpecifier &CS,
                     const char *startSpecifier, unsigned specifierLen,
                     unsigned argIndex);
+  
+  void CheckArgType(const analyze_format_string::FormatSpecifier &FS,
+                    const analyze_format_string::ConversionSpecifier &CS,
+                    const char *startSpecifier, unsigned specifierLen,
+                    unsigned argIndex);
 };
 }
 
@@ -1299,6 +1304,52 @@
   return true;
 }
 
+void CheckFormatHandler::CheckArgType(
+  const analyze_format_string::FormatSpecifier &FS,
+  const analyze_format_string::ConversionSpecifier &CS,
+  const char *startSpecifier, unsigned specifierLen, unsigned argIndex) {
+  
+  const Expr *Ex = getDataArg(argIndex);
+  const analyze_format_string::ArgTypeResult &ATR = FS.getArgType(S.Context);
+
+  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 scanf/printf are varargs
+    // functions.
+    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Ex))
+      if (ICE->getType() == S.Context.IntTy)
+        if (ATR.matchesType(S.Context, ICE->getSubExpr()->getType()))
+          return;
+    
+    if (const analyze_printf::PrintfSpecifier *PFS =
+        dyn_cast<analyze_printf::PrintfSpecifier>(&FS)) {
+      // We may be able to offer a FixItHint if it is a supported type.
+      analyze_printf::PrintfSpecifier fixedFS(*PFS);
+      if (fixedFS.fixType(Ex->getType())) {
+        // Get the fix string from the fixed format specifier
+        llvm::SmallString<128> buf;
+        llvm::raw_svector_ostream os(buf);
+        fixedFS.toString(os);
+      
+        S.Diag(getLocationOfByte(CS.getStart()),
+               diag::warn_printf_conversion_argument_type_mismatch)
+          << ATR.getRepresentativeType(S.Context) << Ex->getType()
+          << getSpecifierRange(startSpecifier, specifierLen)
+        << Ex->getSourceRange()
+        << FixItHint::CreateReplacement(
+                getSpecifierRange(startSpecifier, specifierLen), os.str());
+      }
+      else {
+        S.Diag(getLocationOfByte(CS.getStart()),
+               diag::warn_printf_conversion_argument_type_mismatch)
+          << ATR.getRepresentativeType(S.Context) << Ex->getType()
+          << getSpecifierRange(startSpecifier, specifierLen)
+          << Ex->getSourceRange();
+      }
+    }
+  }
+}
+
 //===--- CHECK: Printf format string checking ------------------------------===//
 
 namespace {
@@ -1570,47 +1621,8 @@
   if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex))
     return false;
 
-  // Now type check the data expression that matches the
-  // format specifier.
-  const Expr *Ex = getDataArg(argIndex);
-  const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context);
-  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)
-        if (ATR.matchesType(S.Context, ICE->getSubExpr()->getType()))
-          return true;
-
-    // We may be able to offer a FixItHint if it is a supported type.
-    PrintfSpecifier fixedFS = FS;
-    bool success = fixedFS.fixType(Ex->getType());
-
-    if (success) {
-      // Get the fix string from the fixed format specifier
-      llvm::SmallString<128> buf;
-      llvm::raw_svector_ostream os(buf);
-      fixedFS.toString(os);
-
-      S.Diag(getLocationOfByte(CS.getStart()),
-          diag::warn_printf_conversion_argument_type_mismatch)
-        << ATR.getRepresentativeType(S.Context) << Ex->getType()
-        << getSpecifierRange(startSpecifier, specifierLen)
-        << Ex->getSourceRange()
-        << FixItHint::CreateReplacement(
-            getSpecifierRange(startSpecifier, specifierLen),
-            os.str());
-    }
-    else {
-      S.Diag(getLocationOfByte(CS.getStart()),
-             diag::warn_printf_conversion_argument_type_mismatch)
-        << ATR.getRepresentativeType(S.Context) << Ex->getType()
-        << getSpecifierRange(startSpecifier, specifierLen)
-        << Ex->getSourceRange();
-    }
-  }
-
+  CheckArgType(FS, CS, startSpecifier, specifierLen, argIndex);
+  
   return true;
 }
 





More information about the cfe-commits mailing list