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

Michael Spencer bigcheesegs at gmail.com
Mon Jul 26 21:51:53 PDT 2010


I reverted this change in r109491 due to:

4>..\..\..\..\..\..\llvm\tools\clang\lib\Analysis\ScanfFormatString.cpp(221):
error C2065: 'ASTContext' : undeclared identifier
4>..\..\..\..\..\..\llvm\tools\clang\lib\Analysis\ScanfFormatString.cpp(221):
error C2065: 'Ctx' : undeclared identifier
4>..\..\..\..\..\..\llvm\tools\clang\lib\Analysis\ScanfFormatString.cpp(221):
error C2761: 'clang::analyze_format_string::ArgTypeResult
clang::analyze_scanf::ScanfSpecifier::getArgType(clang::ASTContext &)
const' : member function redeclaration not allowed
4>..\..\..\..\..\..\llvm\tools\clang\lib\Analysis\ScanfFormatString.cpp(221):
error C2143: syntax error : missing ';' before 'const'
4>..\..\..\..\..\..\llvm\tools\clang\lib\Analysis\ScanfFormatString.cpp(221):
error C2059: syntax error : 'const'
4>..\..\..\..\..\..\llvm\tools\clang\lib\Analysis\ScanfFormatString.cpp(221):
error C2143: syntax error : missing ';' before '{'
4>..\..\..\..\..\..\llvm\tools\clang\lib\Analysis\ScanfFormatString.cpp(221):
error C2447: '{' : missing function header (old-style formal list?)

- Michael Spencer


On Mon, Jul 26, 2010 at 3:45 PM, Ted Kremenek <kremenek at apple.com> wrote:
> 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;
>  }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>




More information about the cfe-commits mailing list