[cfe-commits] r109428 - in /cfe/trunk: include/clang/Analysis/Analyses/FormatString.h lib/Analysis/FormatString.cpp lib/Analysis/ScanfFormatString.cpp lib/Sema/SemaChecking.cpp
Daniel Dunbar
daniel at zuster.org
Tue Jul 27 10:17:08 PDT 2010
Hi Michael,
Thanks for the revert.
However, please also keep in mind that we have *very* limited Win32
development resources. Most Clang/LLVM developers have limited access
to Windows systems, we have no buildbot coverage of VS 2010 (which
isn't even released), etc. In general, until the platform is more
strongly supported I recommend trying to resolve the problems, or let
the developer know with some hints of what to fix, before resorting to
reversion, to avoid excess churn on the tree.
Cheers,
- Daniel
On Mon, Jul 26, 2010 at 9:51 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
> 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
>>
>
> _______________________________________________
> 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