diff --git a/clang/include/clang/Analysis/Analyses/FormatString.h b/clang/include/clang/Analysis/Analyses/FormatString.h index 3a5722a54a8b..812f5a7e470a 100644 --- a/clang/include/clang/Analysis/Analyses/FormatString.h +++ b/clang/include/clang/Analysis/Analyses/FormatString.h @@ -305,6 +305,8 @@ protected: public: FormatSpecifier(bool isPrintf) : CS(isPrintf), UsesPositionalArg(false), argIndex(0) {} + + virtual ~FormatSpecifier(); void setLengthModifier(LengthModifier lm) { LM = lm; @@ -339,6 +341,17 @@ public: 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 @@ public: 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 @@ public: 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 @@ public: } }; +using analyze_format_string::ArgTypeResult; using analyze_format_string::LengthModifier; using analyze_format_string::OptionalAmount; using analyze_format_string::OptionalFlag; @@ -523,6 +542,13 @@ public: 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); }; diff --git a/clang/lib/Analysis/FormatString.cpp b/clang/lib/Analysis/FormatString.cpp index 0fbe54353eb8..2c012d35066b 100644 --- a/clang/lib/Analysis/FormatString.cpp +++ b/clang/lib/Analysis/FormatString.cpp @@ -379,9 +379,11 @@ void OptionalAmount::toString(llvm::raw_ostream &os) const { } //===----------------------------------------------------------------------===// -// Methods on ConversionSpecifier. +// Methods on FormatSpecifier. //===----------------------------------------------------------------------===// +FormatSpecifier::~FormatSpecifier() {} + bool FormatSpecifier::hasValidLengthModifier() const { switch (LM.getKind()) { case LengthModifier::None: diff --git a/clang/lib/Analysis/ScanfFormatString.cpp b/clang/lib/Analysis/ScanfFormatString.cpp index 61af1d673797..d65352ac879e 100644 --- a/clang/lib/Analysis/ScanfFormatString.cpp +++ b/clang/lib/Analysis/ScanfFormatString.cpp @@ -217,4 +217,10 @@ bool clang::analyze_format_string::ParseScanfString(FormatStringHandler &H, return false; } +ArgTypeResult ScanfSpecifier::getArgType(ASTContext &Ctx) const { + // FIXME: Fill in. + return ArgTypeResult(); +} + + diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 03ca084dd897..1b689499edf9 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1174,6 +1174,11 @@ protected: 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 @@ CheckFormatHandler::CheckNumArgs( 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(Ex)) + if (ICE->getType() == S.Context.IntTy) + if (ATR.matchesType(S.Context, ICE->getSubExpr()->getType())) + return; + + if (const analyze_printf::PrintfSpecifier *PFS = + dyn_cast(&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 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier 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(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; }