From 772e9270f666afef722292b6aaf3791feb8fcc1f Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Wed, 7 Dec 2011 10:33:11 +0000 Subject: [PATCH] Make printf warnings refer to intmax_t et al. by name in addition to underlying type. For example, the warning for printf("%zu", 42.0); changes from "conversion specifies type 'unsigned long'" to "conversion specifies type 'size_t' (aka 'unsigned long')" (This is a second attempt after r145697, which got reverted.) llvm-svn: 146032 --- .../clang/Analysis/Analyses/FormatString.h | 14 +++++++----- clang/lib/Analysis/FormatString.cpp | 11 ++++++++-- clang/lib/Analysis/PrintfFormatString.cpp | 11 ++++++---- clang/lib/Sema/SemaChecking.cpp | 6 ++--- clang/test/Sema/format-strings-int-typedefs.c | 22 +++++++++++++++++++ clang/test/Sema/format-strings-size_t.c | 8 +++---- 6 files changed, 54 insertions(+), 18 deletions(-) create mode 100644 clang/test/Sema/format-strings-int-typedefs.c diff --git a/clang/include/clang/Analysis/Analyses/FormatString.h b/clang/include/clang/Analysis/Analyses/FormatString.h index b2d45e36cb13..c7dfbf8cb85d 100644 --- a/clang/include/clang/Analysis/Analyses/FormatString.h +++ b/clang/include/clang/Analysis/Analyses/FormatString.h @@ -199,15 +199,17 @@ protected: class ArgTypeResult { public: enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CPointerTy, - AnyCharTy, CStrTy, WCStrTy, WIntTy }; + AnyCharTy, CStrTy, WCStrTy, WIntTy, TypedefTy }; private: const Kind K; QualType T; - ArgTypeResult(bool) : K(InvalidTy) {} + const char *Name; + ArgTypeResult(bool) : K(InvalidTy), Name(0) {} public: - ArgTypeResult(Kind k = UnknownTy) : K(k) {} - ArgTypeResult(QualType t) : K(SpecificTy), T(t) {} - ArgTypeResult(CanQualType t) : K(SpecificTy), T(t) {} + ArgTypeResult(Kind k = UnknownTy) : K(k), Name(0) {} + ArgTypeResult(QualType t) : K(SpecificTy), T(t), Name(0) {} + ArgTypeResult(QualType t, const char *n) : K(TypedefTy), T(t), Name(n) {} + ArgTypeResult(CanQualType t) : K(SpecificTy), T(t), Name(0) {} static ArgTypeResult Invalid() { return ArgTypeResult(true); } @@ -222,6 +224,8 @@ public: bool matchesAnyObjCObjectRef() const { return K == ObjCPointerTy; } QualType getRepresentativeType(ASTContext &C) const; + + std::string getRepresentativeTypeName(ASTContext &C) const; }; class OptionalAmount { diff --git a/clang/lib/Analysis/FormatString.cpp b/clang/lib/Analysis/FormatString.cpp index 6498ded4e374..0853164df750 100644 --- a/clang/lib/Analysis/FormatString.cpp +++ b/clang/lib/Analysis/FormatString.cpp @@ -228,6 +228,7 @@ bool ArgTypeResult::matchesType(ASTContext &C, QualType argTy) const { return false; } + case TypedefTy: case SpecificTy: { argTy = C.getCanonicalType(argTy).getUnqualifiedType(); if (T == argTy) @@ -331,6 +332,7 @@ QualType ArgTypeResult::getRepresentativeType(ASTContext &C) const { case AnyCharTy: return C.CharTy; case SpecificTy: + case TypedefTy: return T; case CStrTy: return C.getPointerType(C.CharTy); @@ -351,6 +353,13 @@ QualType ArgTypeResult::getRepresentativeType(ASTContext &C) const { return QualType(); } +std::string ArgTypeResult::getRepresentativeTypeName(ASTContext &C) const { + if (K != TypedefTy) + return std::string("'") + getRepresentativeType(C).getAsString() + "'"; + return std::string("'") + Name + "' (aka '" + T.getAsString() + "')"; +} + + //===----------------------------------------------------------------------===// // Methods on OptionalAmount. //===----------------------------------------------------------------------===// @@ -485,5 +494,3 @@ bool FormatSpecifier::hasValidLengthModifier() const { } return false; } - - diff --git a/clang/lib/Analysis/PrintfFormatString.cpp b/clang/lib/Analysis/PrintfFormatString.cpp index 70dbfd30ceef..e14c95229258 100644 --- a/clang/lib/Analysis/PrintfFormatString.cpp +++ b/clang/lib/Analysis/PrintfFormatString.cpp @@ -301,11 +301,13 @@ ArgTypeResult PrintfSpecifier::getArgType(ASTContext &Ctx) const { case LengthModifier::AsShort: return Ctx.ShortTy; case LengthModifier::AsLong: return Ctx.LongTy; case LengthModifier::AsLongLong: return Ctx.LongLongTy; - case LengthModifier::AsIntMax: return Ctx.getIntMaxType(); + case LengthModifier::AsIntMax: + return ArgTypeResult(Ctx.getIntMaxType(), "intmax_t"); case LengthModifier::AsSizeT: // FIXME: How to get the corresponding signed version of size_t? return ArgTypeResult(); - case LengthModifier::AsPtrDiff: return Ctx.getPointerDiffType(); + case LengthModifier::AsPtrDiff: + return ArgTypeResult(Ctx.getPointerDiffType(), "ptrdiff_t"); } if (CS.isUIntArg()) @@ -317,9 +319,10 @@ ArgTypeResult PrintfSpecifier::getArgType(ASTContext &Ctx) const { case LengthModifier::AsShort: return Ctx.UnsignedShortTy; case LengthModifier::AsLong: return Ctx.UnsignedLongTy; case LengthModifier::AsLongLong: return Ctx.UnsignedLongLongTy; - case LengthModifier::AsIntMax: return Ctx.getUIntMaxType(); + case LengthModifier::AsIntMax: + return ArgTypeResult(Ctx.getUIntMaxType(), "uintmax_t"); case LengthModifier::AsSizeT: - return Ctx.getSizeType(); + return ArgTypeResult(Ctx.getSizeType(), "size_t"); case LengthModifier::AsPtrDiff: // FIXME: How to get the corresponding unsigned // version of ptrdiff_t? diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 869922faf421..f635afcc3065 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -2014,7 +2014,7 @@ bool CheckPrintfHandler::HandleAmount( if (!ATR.matchesType(S.Context, T)) { EmitFormatDiagnostic(S.PDiag(diag::warn_printf_asterisk_wrong_type) - << k << ATR.getRepresentativeType(S.Context) + << k << ATR.getRepresentativeTypeName(S.Context) << T << Arg->getSourceRange(), getLocationOfByte(Amt.getStart()), /*IsStringLocation*/true, @@ -2234,7 +2234,7 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier // type is 'wint_t' (which is defined in the system headers). EmitFormatDiagnostic( S.PDiag(diag::warn_printf_conversion_argument_type_mismatch) - << ATR.getRepresentativeType(S.Context) << Ex->getType() + << ATR.getRepresentativeTypeName(S.Context) << Ex->getType() << Ex->getSourceRange(), getLocationOfByte(CS.getStart()), /*IsStringLocation*/true, @@ -2246,7 +2246,7 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier else { S.Diag(getLocationOfByte(CS.getStart()), diag::warn_printf_conversion_argument_type_mismatch) - << ATR.getRepresentativeType(S.Context) << Ex->getType() + << ATR.getRepresentativeTypeName(S.Context) << Ex->getType() << getSpecifierRange(startSpecifier, specifierLen) << Ex->getSourceRange(); } diff --git a/clang/test/Sema/format-strings-int-typedefs.c b/clang/test/Sema/format-strings-int-typedefs.c new file mode 100644 index 000000000000..29565e69ad9c --- /dev/null +++ b/clang/test/Sema/format-strings-int-typedefs.c @@ -0,0 +1,22 @@ +// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -verify %s + +int printf(char const *, ...); + +void test(void) { + printf("%jd", 42.0); // expected-warning {{conversion specifies type 'intmax_t' (aka 'long long')}} + printf("%ju", 42.0); // expected-warning {{conversion specifies type 'uintmax_t' (aka 'unsigned long long')}} + printf("%zu", 42.0); // expected-warning {{conversion specifies type 'size_t' (aka 'unsigned long')}} + printf("%td", 42.0); // expected-warning {{conversion specifies type 'ptrdiff_t' (aka 'int')}} + + // typedef size_t et al. to something crazy. + typedef void *size_t; + typedef void *intmax_t; + typedef void *uintmax_t; + typedef void *ptrdiff_t; + + // The warning still fires, because it checks the underlying type. + printf("%jd", (intmax_t)42); // expected-warning {{conversion specifies type 'intmax_t' (aka 'long long') but the argument has type 'intmax_t' (aka 'void *')}} + printf("%ju", (uintmax_t)42); // expected-warning {{conversion specifies type 'uintmax_t' (aka 'unsigned long long') but the argument has type 'uintmax_t' (aka 'void *')}} + printf("%zu", (size_t)42); // expected-warning {{conversion specifies type 'size_t' (aka 'unsigned long') but the argument has type 'size_t' (aka 'void *')}} + printf("%td", (ptrdiff_t)42); // expected-warning {{conversion specifies type 'ptrdiff_t' (aka 'int') but the argument has type 'ptrdiff_t' (aka 'void *')}} +} diff --git a/clang/test/Sema/format-strings-size_t.c b/clang/test/Sema/format-strings-size_t.c index c86a9e442871..56b20b1b7b73 100644 --- a/clang/test/Sema/format-strings-size_t.c +++ b/clang/test/Sema/format-strings-size_t.c @@ -4,12 +4,12 @@ int printf(char const *, ...); void test(void) { // size_t - printf("%zu", (double)42); // expected-warning {{conversion specifies type 'unsigned long' but the argument has type 'double''}} + printf("%zu", (double)42); // expected-warning {{conversion specifies type 'size_t' (aka 'unsigned long') but the argument has type 'double''}} // intmax_t / uintmax_t - printf("%jd", (double)42); // expected-warning {{conversion specifies type 'long' but the argument has type 'double''}} - printf("%ju", (double)42); // expected-warning {{conversion specifies type 'unsigned long' but the argument has type 'double''}} + printf("%jd", (double)42); // expected-warning {{conversion specifies type 'intmax_t' (aka 'long') but the argument has type 'double''}} + printf("%ju", (double)42); // expected-warning {{conversion specifies type 'uintmax_t' (aka 'unsigned long') but the argument has type 'double''}} // ptrdiff_t - printf("%td", (double)42); // expected-warning {{conversion specifies type 'long' but the argument has type 'double''}} + printf("%td", (double)42); // expected-warning {{conversion specifies type 'ptrdiff_t' (aka 'long') but the argument has type 'double''}} }