From 58e1e54476d610d6c33ef483f216ed8a1282d35c Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Tue, 7 Aug 2012 08:59:46 +0000 Subject: [PATCH] Remove ScanfArgType and bake that logic into ArgType. This is useful for example for %n in printf, which expects a pointer to int with the same logic for checking as %d would have in scanf. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161407 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Analysis/Analyses/FormatString.h | 43 ++--- lib/Analysis/FormatString.cpp | 66 +++++--- lib/Analysis/PrintfFormatString.cpp | 2 +- lib/Analysis/ScanfFormatString.cpp | 147 +++++++----------- lib/Sema/SemaChecking.cpp | 8 +- test/Sema/format-strings-scanf.c | 6 + 6 files changed, 128 insertions(+), 144 deletions(-) diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h index 45779099d1..7f50ee392b 100644 --- a/include/clang/Analysis/Analyses/FormatString.h +++ b/include/clang/Analysis/Analyses/FormatString.h @@ -209,15 +209,24 @@ private: const Kind K; QualType T; const char *Name; + bool Ptr; public: - ArgType(Kind k = UnknownTy, const char *n = 0) : K(k), Name(n) {} - ArgType(QualType t, const char *n = 0) : K(SpecificTy), T(t), Name(n) {} - ArgType(CanQualType t) : K(SpecificTy), T(t), Name(0) {} + ArgType(Kind k = UnknownTy, const char *n = 0) : K(k), Name(n), Ptr(false) {} + ArgType(QualType t, const char *n = 0) + : K(SpecificTy), T(t), Name(n), Ptr(false) {} + ArgType(CanQualType t) : K(SpecificTy), T(t), Name(0), Ptr(false) {} static ArgType Invalid() { return ArgType(InvalidTy); } - bool isValid() const { return K != InvalidTy; } + /// Create an ArgType which corresponds to the type pointer to A. + static ArgType PtrTo(const ArgType& A) { + assert(A.K >= InvalidTy && "ArgType cannot be pointer to invalid/unknown"); + ArgType Res = A; + Res.Ptr = true; + return Res; + } + bool matchesType(ASTContext &C, QualType argTy) const; QualType getRepresentativeType(ASTContext &C) const; @@ -517,30 +526,6 @@ using analyze_format_string::LengthModifier; using analyze_format_string::OptionalAmount; using analyze_format_string::OptionalFlag; -class ScanfArgType : public ArgType { -public: - enum Kind { UnknownTy, InvalidTy, CStrTy, WCStrTy, PtrToArgTypeTy }; -private: - Kind K; - ArgType A; - const char *Name; - QualType getRepresentativeType(ASTContext &C) const; -public: - ScanfArgType(Kind k = UnknownTy, const char* n = 0) : K(k), Name(n) {} - ScanfArgType(ArgType a, const char *n = 0) - : K(PtrToArgTypeTy), A(a), Name(n) { - assert(A.isValid()); - } - - static ScanfArgType Invalid() { return ScanfArgType(InvalidTy); } - - bool isValid() const { return K != InvalidTy; } - - bool matchesType(ASTContext& C, QualType argTy) const; - - std::string getRepresentativeTypeName(ASTContext& C) const; -}; - class ScanfSpecifier : public analyze_format_string::FormatSpecifier { OptionalFlag SuppressAssignment; // '*' public: @@ -569,7 +554,7 @@ public: return CS.consumesDataArgument() && !SuppressAssignment; } - ScanfArgType getArgType(ASTContext &Ctx) const; + ArgType getArgType(ASTContext &Ctx) const; bool fixType(QualType QT, const LangOptions &LangOpt, ASTContext &Ctx); diff --git a/lib/Analysis/FormatString.cpp b/lib/Analysis/FormatString.cpp index 3532a9334f..e7ea48688d 100644 --- a/lib/Analysis/FormatString.cpp +++ b/lib/Analysis/FormatString.cpp @@ -233,6 +233,19 @@ clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS, //===----------------------------------------------------------------------===// bool ArgType::matchesType(ASTContext &C, QualType argTy) const { + if (Ptr) { + // It has to be a pointer. + const PointerType *PT = argTy->getAs(); + if (!PT) + return false; + + // We cannot write through a const qualified pointer. + if (PT->getPointeeType().isConstQualified()) + return false; + + argTy = PT->getPointeeType(); + } + switch (K) { case InvalidTy: llvm_unreachable("ArgType must be valid"); @@ -262,13 +275,6 @@ bool ArgType::matchesType(ASTContext &C, QualType argTy) const { argTy = ETy->getDecl()->getIntegerType(); argTy = C.getCanonicalType(argTy).getUnqualifiedType(); - if (const PointerType *PTy = argTy->getAs()) { - // Strip volatile qualifier from pointee type. - QualType Pointee = PTy->getPointeeType(); - Pointee.removeLocalVolatile(); - argTy = C.getPointerType(Pointee); - } - if (T == argTy) return true; // Check for "compatible types". @@ -375,35 +381,59 @@ bool ArgType::matchesType(ASTContext &C, QualType argTy) const { } QualType ArgType::getRepresentativeType(ASTContext &C) const { + QualType Res; switch (K) { case InvalidTy: llvm_unreachable("No representative type for Invalid ArgType"); case UnknownTy: - return QualType(); + llvm_unreachable("No representative type for Unknown ArgType"); case AnyCharTy: - return C.CharTy; + Res = C.CharTy; + break; case SpecificTy: - return T; + Res = T; + break; case CStrTy: - return C.getPointerType(C.CharTy); + Res = C.getPointerType(C.CharTy); + break; case WCStrTy: - return C.getPointerType(C.getWCharType()); + Res = C.getPointerType(C.getWCharType()); + break; case ObjCPointerTy: - return C.ObjCBuiltinIdTy; + Res = C.ObjCBuiltinIdTy; + break; case CPointerTy: - return C.VoidPtrTy; + Res = C.VoidPtrTy; + break; case WIntTy: { - return C.getWIntType(); + Res = C.getWIntType(); + break; } } - llvm_unreachable("Invalid ArgType Kind!"); + if (Ptr) + Res = C.getPointerType(Res); + return Res; } std::string ArgType::getRepresentativeTypeName(ASTContext &C) const { std::string S = getRepresentativeType(C).getAsString(); - if (Name && S != Name) - return std::string("'") + Name + "' (aka '" + S + "')"; + + std::string Alias; + if (Name) { + // Use a specific name for this type, e.g. "size_t". + Alias = Name; + if (Ptr) { + // If ArgType is actually a pointer to T, append an asterisk. + Alias += (Alias[Alias.size()-1] == '*') ? "*" : " *"; + } + // If Alias is the same as the underlying type, e.g. wchar_t, then drop it. + if (S == Alias) + Alias.clear(); + } + + if (!Alias.empty()) + return std::string("'") + Alias + "' (aka '" + S + "')"; return std::string("'") + S + "'"; } diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp index 7e078938d0..0a30328b12 100644 --- a/lib/Analysis/PrintfFormatString.cpp +++ b/lib/Analysis/PrintfFormatString.cpp @@ -331,7 +331,7 @@ ArgType PrintfSpecifier::getArgType(ASTContext &Ctx, case ConversionSpecifier::pArg: return ArgType::CPointerTy; case ConversionSpecifier::nArg: - return Ctx.getPointerType(Ctx.IntTy); + return ArgType::PtrTo(Ctx.IntTy); case ConversionSpecifier::ObjCObjArg: return ArgType::ObjCPointerTy; default: diff --git a/lib/Analysis/ScanfFormatString.cpp b/lib/Analysis/ScanfFormatString.cpp index d091237d12..80deb37fad 100644 --- a/lib/Analysis/ScanfFormatString.cpp +++ b/lib/Analysis/ScanfFormatString.cpp @@ -20,7 +20,6 @@ using clang::analyze_format_string::FormatStringHandler; using clang::analyze_format_string::LengthModifier; using clang::analyze_format_string::OptionalAmount; using clang::analyze_format_string::ConversionSpecifier; -using clang::analyze_scanf::ScanfArgType; using clang::analyze_scanf::ScanfConversionSpecifier; using clang::analyze_scanf::ScanfSpecifier; using clang::UpdateOnReturn; @@ -194,37 +193,42 @@ static ScanfSpecifierResult ParseScanfSpecifier(FormatStringHandler &H, return ScanfSpecifierResult(Start, FS); } -ScanfArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const { +ArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const { const ScanfConversionSpecifier &CS = getConversionSpecifier(); if (!CS.consumesDataArgument()) - return ScanfArgType::Invalid(); + return ArgType::Invalid(); switch(CS.getKind()) { // Signed int. case ConversionSpecifier::dArg: case ConversionSpecifier::iArg: switch (LM.getKind()) { - case LengthModifier::None: return ArgType(Ctx.IntTy); + case LengthModifier::None: + return ArgType::PtrTo(Ctx.IntTy); case LengthModifier::AsChar: - return ArgType(ArgType::AnyCharTy); - case LengthModifier::AsShort: return ArgType(Ctx.ShortTy); - case LengthModifier::AsLong: return ArgType(Ctx.LongTy); + return ArgType::PtrTo(ArgType::AnyCharTy); + case LengthModifier::AsShort: + return ArgType::PtrTo(Ctx.ShortTy); + case LengthModifier::AsLong: + return ArgType::PtrTo(Ctx.LongTy); case LengthModifier::AsLongLong: case LengthModifier::AsQuad: - return ArgType(Ctx.LongLongTy); + return ArgType::PtrTo(Ctx.LongLongTy); case LengthModifier::AsIntMax: - return ScanfArgType(Ctx.getIntMaxType(), "intmax_t *"); + return ArgType::PtrTo(ArgType(Ctx.getIntMaxType(), "intmax_t")); case LengthModifier::AsSizeT: // FIXME: ssize_t. - return ScanfArgType(); + return ArgType(); case LengthModifier::AsPtrDiff: - return ScanfArgType(Ctx.getPointerDiffType(), "ptrdiff_t *"); + return ArgType::PtrTo(ArgType(Ctx.getPointerDiffType(), "ptrdiff_t")); case LengthModifier::AsLongDouble: // GNU extension. - return ArgType(Ctx.LongLongTy); - case LengthModifier::AsAllocate: return ScanfArgType::Invalid(); - case LengthModifier::AsMAllocate: return ScanfArgType::Invalid(); + return ArgType::PtrTo(Ctx.LongLongTy); + case LengthModifier::AsAllocate: + return ArgType::Invalid(); + case LengthModifier::AsMAllocate: + return ArgType::Invalid(); } // Unsigned int. @@ -233,25 +237,31 @@ ScanfArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const { case ConversionSpecifier::xArg: case ConversionSpecifier::XArg: switch (LM.getKind()) { - case LengthModifier::None: return ArgType(Ctx.UnsignedIntTy); - case LengthModifier::AsChar: return ArgType(Ctx.UnsignedCharTy); - case LengthModifier::AsShort: return ArgType(Ctx.UnsignedShortTy); - case LengthModifier::AsLong: return ArgType(Ctx.UnsignedLongTy); + case LengthModifier::None: + return ArgType::PtrTo(Ctx.UnsignedIntTy); + case LengthModifier::AsChar: + return ArgType::PtrTo(Ctx.UnsignedCharTy); + case LengthModifier::AsShort: + return ArgType::PtrTo(Ctx.UnsignedShortTy); + case LengthModifier::AsLong: + return ArgType::PtrTo(Ctx.UnsignedLongTy); case LengthModifier::AsLongLong: case LengthModifier::AsQuad: - return ArgType(Ctx.UnsignedLongLongTy); + return ArgType::PtrTo(Ctx.UnsignedLongLongTy); case LengthModifier::AsIntMax: - return ScanfArgType(Ctx.getUIntMaxType(), "uintmax_t *"); + return ArgType::PtrTo(ArgType(Ctx.getUIntMaxType(), "uintmax_t")); case LengthModifier::AsSizeT: - return ScanfArgType(Ctx.getSizeType(), "size_t *"); + return ArgType::PtrTo(ArgType(Ctx.getSizeType(), "size_t")); case LengthModifier::AsPtrDiff: // FIXME: Unsigned version of ptrdiff_t? - return ScanfArgType(); + return ArgType(); case LengthModifier::AsLongDouble: // GNU extension. - return ArgType(Ctx.UnsignedLongLongTy); - case LengthModifier::AsAllocate: return ScanfArgType::Invalid(); - case LengthModifier::AsMAllocate: return ScanfArgType::Invalid(); + return ArgType::PtrTo(Ctx.UnsignedLongLongTy); + case LengthModifier::AsAllocate: + return ArgType::Invalid(); + case LengthModifier::AsMAllocate: + return ArgType::Invalid(); } // Float. @@ -264,12 +274,14 @@ ScanfArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const { case ConversionSpecifier::gArg: case ConversionSpecifier::GArg: switch (LM.getKind()) { - case LengthModifier::None: return ArgType(Ctx.FloatTy); - case LengthModifier::AsLong: return ArgType(Ctx.DoubleTy); + case LengthModifier::None: + return ArgType::PtrTo(Ctx.FloatTy); + case LengthModifier::AsLong: + return ArgType::PtrTo(Ctx.DoubleTy); case LengthModifier::AsLongDouble: - return ArgType(Ctx.LongDoubleTy); + return ArgType::PtrTo(Ctx.LongDoubleTy); default: - return ScanfArgType::Invalid(); + return ArgType::Invalid(); } // Char, string and scanlist. @@ -277,40 +289,42 @@ ScanfArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const { case ConversionSpecifier::sArg: case ConversionSpecifier::ScanListArg: switch (LM.getKind()) { - case LengthModifier::None: return ScanfArgType::CStrTy; + case LengthModifier::None: + return ArgType::PtrTo(ArgType::AnyCharTy); case LengthModifier::AsLong: - return ScanfArgType(ScanfArgType::WCStrTy, "wchar_t *"); + return ArgType::PtrTo(ArgType(Ctx.getWCharType(), "wchar_t")); case LengthModifier::AsAllocate: case LengthModifier::AsMAllocate: - return ScanfArgType(ArgType::CStrTy); + return ArgType::PtrTo(ArgType::CStrTy); default: - return ScanfArgType::Invalid(); + return ArgType::Invalid(); } case ConversionSpecifier::CArg: case ConversionSpecifier::SArg: // FIXME: Mac OS X specific? switch (LM.getKind()) { case LengthModifier::None: - return ScanfArgType(ScanfArgType::WCStrTy, "wchar_t *"); + return ArgType::PtrTo(ArgType(Ctx.getWCharType(), "wchar_t")); case LengthModifier::AsAllocate: case LengthModifier::AsMAllocate: - return ScanfArgType(ArgType::WCStrTy, "wchar_t **"); + return ArgType::PtrTo(ArgType(ArgType::WCStrTy, "wchar_t *")); default: - return ScanfArgType::Invalid(); + return ArgType::Invalid(); } // Pointer. case ConversionSpecifier::pArg: - return ScanfArgType(ArgType(ArgType::CPointerTy)); + return ArgType::PtrTo(ArgType::CPointerTy); + // Write-back. case ConversionSpecifier::nArg: - return ArgType(Ctx.IntTy); + return ArgType::PtrTo(Ctx.IntTy); default: break; } - return ScanfArgType(); + return ArgType(); } bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt, @@ -393,8 +407,8 @@ bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt, namedTypeToLengthModifier(PT, LM); // If fixing the length modifier was enough, we are done. - const analyze_scanf::ScanfArgType &ATR = getArgType(Ctx); - if (hasValidLengthModifier() && ATR.isValid() && ATR.matchesType(Ctx, QT)) + const analyze_scanf::ArgType &AT = getArgType(Ctx); + if (hasValidLengthModifier() && AT.isValid() && AT.matchesType(Ctx, QT)) return true; // Figure out the conversion specifier. @@ -451,54 +465,3 @@ bool clang::analyze_format_string::ParseScanfString(FormatStringHandler &H, assert(I == E && "Format string not exhausted"); return false; } - -bool ScanfArgType::matchesType(ASTContext& C, QualType argTy) const { - // It has to be a pointer type. - const PointerType *PT = argTy->getAs(); - if (!PT) - return false; - - // We cannot write through a const qualified pointer. - if (PT->getPointeeType().isConstQualified()) - return false; - - switch (K) { - case InvalidTy: - llvm_unreachable("ArgType must be valid"); - case UnknownTy: - return true; - case CStrTy: - return ArgType(ArgType::CStrTy).matchesType(C, argTy); - case WCStrTy: - return ArgType(ArgType::WCStrTy).matchesType(C, argTy); - case PtrToArgTypeTy: { - return A.matchesType(C, PT->getPointeeType()); - } - } - - llvm_unreachable("Invalid ScanfArgType Kind!"); -} - -QualType ScanfArgType::getRepresentativeType(ASTContext &C) const { - switch (K) { - case InvalidTy: - llvm_unreachable("No representative type for Invalid ArgType"); - case UnknownTy: - return QualType(); - case CStrTy: - return C.getPointerType(C.CharTy); - case WCStrTy: - return C.getPointerType(C.getWCharType()); - case PtrToArgTypeTy: - return C.getPointerType(A.getRepresentativeType(C)); - } - - llvm_unreachable("Invalid ScanfArgType Kind!"); -} - -std::string ScanfArgType::getRepresentativeTypeName(ASTContext& C) const { - std::string S = getRepresentativeType(C).getAsString(); - if (!Name) - return std::string("'") + S + "'"; - return std::string("'") + Name + "' (aka '" + S + "')"; -} diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index e699f00479..5e4309195e 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -2800,8 +2800,8 @@ bool CheckScanfHandler::HandleScanfSpecifier( if (!Ex) return true; - const analyze_scanf::ScanfArgType &ATR = FS.getArgType(S.Context); - if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) { + const analyze_format_string::ArgType &AT = FS.getArgType(S.Context); + if (AT.isValid() && !AT.matchesType(S.Context, Ex->getType())) { ScanfSpecifier fixedFS = FS; bool success = fixedFS.fixType(Ex->getType(), S.getLangOpts(), S.Context); @@ -2814,7 +2814,7 @@ bool CheckScanfHandler::HandleScanfSpecifier( EmitFormatDiagnostic( S.PDiag(diag::warn_printf_conversion_argument_type_mismatch) - << ATR.getRepresentativeTypeName(S.Context) << Ex->getType() + << AT.getRepresentativeTypeName(S.Context) << Ex->getType() << Ex->getSourceRange(), Ex->getLocStart(), /*IsStringLocation*/false, @@ -2825,7 +2825,7 @@ bool CheckScanfHandler::HandleScanfSpecifier( } else { EmitFormatDiagnostic( S.PDiag(diag::warn_printf_conversion_argument_type_mismatch) - << ATR.getRepresentativeTypeName(S.Context) << Ex->getType() + << AT.getRepresentativeTypeName(S.Context) << Ex->getType() << Ex->getSourceRange(), Ex->getLocStart(), /*IsStringLocation*/false, diff --git a/test/Sema/format-strings-scanf.c b/test/Sema/format-strings-scanf.c index 6f6cb10eb3..49c97149a7 100644 --- a/test/Sema/format-strings-scanf.c +++ b/test/Sema/format-strings-scanf.c @@ -33,6 +33,12 @@ void test(const char *s, int *i) { scanf("%*d", i); // // expected-warning{{data argument not used by format string}} scanf("%*d", i); // // expected-warning{{data argument not used by format string}} scanf("%*d%1$d", i); // no-warning + + scanf("%s", (char*)0); // no-warning + scanf("%s", (volatile char*)0); // no-warning + scanf("%s", (signed char*)0); // no-warning + scanf("%s", (unsigned char*)0); // no-warning + scanf("%hhu", (signed char*)0); // no-warning } void bad_length_modifiers(char *s, void *p, wchar_t *ws, long double *ld) { -- 2.40.0