diff --git a/clang/include/clang/Analysis/Analyses/FormatString.h b/clang/include/clang/Analysis/Analyses/FormatString.h index 812f5a7e470a..3a5722a54a8b 100644 --- a/clang/include/clang/Analysis/Analyses/FormatString.h +++ b/clang/include/clang/Analysis/Analyses/FormatString.h @@ -305,8 +305,6 @@ protected: public: FormatSpecifier(bool isPrintf) : CS(isPrintf), UsesPositionalArg(false), argIndex(0) {} - - virtual ~FormatSpecifier(); void setLengthModifier(LengthModifier lm) { LM = lm; @@ -341,17 +339,6 @@ 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 @@ -451,9 +438,9 @@ public: return getConversionSpecifier().consumesDataArgument(); } - /// \brief Returns the type that a data argument + /// \brief Returns the builtin 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 + /// will return null 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; @@ -481,11 +468,6 @@ public: bool hasValidPrecision() const; bool hasValidFieldWidth() const; - - static bool classof(const analyze_format_string::FormatSpecifier *FS) { - return FS->getConversionSpecifier().isPrintfKind(); - } - }; } // end analyze_printf namespace @@ -510,7 +492,6 @@ public: } }; -using analyze_format_string::ArgTypeResult; using analyze_format_string::LengthModifier; using analyze_format_string::OptionalAmount; using analyze_format_string::OptionalFlag; @@ -542,13 +523,6 @@ 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 2c012d35066b..0fbe54353eb8 100644 --- a/clang/lib/Analysis/FormatString.cpp +++ b/clang/lib/Analysis/FormatString.cpp @@ -379,11 +379,9 @@ void OptionalAmount::toString(llvm::raw_ostream &os) const { } //===----------------------------------------------------------------------===// -// Methods on FormatSpecifier. +// Methods on ConversionSpecifier. //===----------------------------------------------------------------------===// -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 4c2c4c6fb9fe..6a8673ab55ca 100644 --- a/clang/lib/Analysis/ScanfFormatString.cpp +++ b/clang/lib/Analysis/ScanfFormatString.cpp @@ -218,10 +218,4 @@ 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 1b689499edf9..03ca084dd897 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1174,11 +1174,6 @@ 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); }; } @@ -1304,52 +1299,6 @@ 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 { @@ -1621,8 +1570,47 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex)) return false; - CheckArgType(FS, CS, startSpecifier, specifierLen, argIndex); - + // 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(); + } + } + return true; }