From: Alex Lorenz Date: Thu, 5 Jul 2018 22:51:11 +0000 (+0000) Subject: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %tu/%td on Darwin X-Git-Tag: llvmorg-7.0.0-rc1~2114 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b2043ac72a1f1623fad47806f3e7881b7e70dcb4;p=platform%2Fupstream%2Fllvm.git [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %tu/%td on Darwin The '%tu'/'%td' as formatting specifiers have been used to print out the NSInteger/NSUInteger values for a long time. Typically their ABI matches, but that's not the case on watchOS. The ABI difference boils down to the following: - Regular 32-bit darwin targets (like armv7) use 'ptrdiff_t' of type 'int', which matches 'NSInteger'. - WatchOS arm target (armv7k) uses 'ptrdiff_t' of type 'long', which doesn't match 'NSInteger' of type 'int'. Because of this ABI difference these specifiers trigger -Wformat warnings only for watchOS builds, which is really inconvenient for cross-platform code. This patch avoids this -Wformat warning for '%tu'/'%td' and NS[U]Integer only, and instead uses the new -Wformat-pedantic warning that JF introduced in https://reviews.llvm.org/D47290. This is acceptable because Darwin guarantees that, despite the watchOS ABI differences, sizeof(ptrdiff_t) == sizeof(NS[U]Integer), and alignof(ptrdiff_t) == alignof(NS[U]Integer) so the warning is therefore noisy for pedantic reasons. I'll update public documentation to ensure that this behaviour is properly communicated. rdar://41739204 Differential Revision: https://reviews.llvm.org/D48852 llvm-svn: 336396 --- diff --git a/clang/include/clang/Analysis/Analyses/FormatString.h b/clang/include/clang/Analysis/Analyses/FormatString.h index 6d76718..6f8bb9b 100644 --- a/clang/include/clang/Analysis/Analyses/FormatString.h +++ b/clang/include/clang/Analysis/Analyses/FormatString.h @@ -257,7 +257,12 @@ private: const Kind K; QualType T; const char *Name = nullptr; - bool Ptr = false, IsSizeT = false; + bool Ptr = false; + + /// The TypeKind identifies certain well-known types like size_t and + /// ptrdiff_t. + enum class TypeKind { DontCare, SizeT, PtrdiffT }; + TypeKind TK = TypeKind::DontCare; public: ArgType(Kind K = UnknownTy, const char *N = nullptr) : K(K), Name(N) {} @@ -267,7 +272,9 @@ public: static ArgType Invalid() { return ArgType(InvalidTy); } bool isValid() const { return K != InvalidTy; } - bool isSizeT() const { return IsSizeT; } + bool isSizeT() const { return TK == TypeKind::SizeT; } + + bool isPtrdiffT() const { return TK == TypeKind::PtrdiffT; } /// Create an ArgType which corresponds to the type pointer to A. static ArgType PtrTo(const ArgType& A) { @@ -280,7 +287,15 @@ public: /// Create an ArgType which corresponds to the size_t/ssize_t type. static ArgType makeSizeT(const ArgType &A) { ArgType Res = A; - Res.IsSizeT = true; + Res.TK = TypeKind::SizeT; + return Res; + } + + /// Create an ArgType which corresponds to the ptrdiff_t/unsigned ptrdiff_t + /// type. + static ArgType makePtrdiffT(const ArgType &A) { + ArgType Res = A; + Res.TK = TypeKind::PtrdiffT; return Res; } diff --git a/clang/lib/Analysis/PrintfFormatString.cpp b/clang/lib/Analysis/PrintfFormatString.cpp index 93d64f0..688b203 100644 --- a/clang/lib/Analysis/PrintfFormatString.cpp +++ b/clang/lib/Analysis/PrintfFormatString.cpp @@ -472,7 +472,8 @@ ArgType PrintfSpecifier::getArgType(ASTContext &Ctx, ? ArgType(Ctx.LongLongTy, "__int64") : ArgType(Ctx.IntTy, "__int32"); case LengthModifier::AsPtrDiff: - return ArgType(Ctx.getPointerDiffType(), "ptrdiff_t"); + return ArgType::makePtrdiffT( + ArgType(Ctx.getPointerDiffType(), "ptrdiff_t")); case LengthModifier::AsAllocate: case LengthModifier::AsMAllocate: case LengthModifier::AsWide: @@ -505,7 +506,8 @@ ArgType PrintfSpecifier::getArgType(ASTContext &Ctx, ? ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64") : ArgType(Ctx.UnsignedIntTy, "unsigned __int32"); case LengthModifier::AsPtrDiff: - return ArgType(Ctx.getUnsignedPointerDiffType(), "unsigned ptrdiff_t"); + return ArgType::makePtrdiffT( + ArgType(Ctx.getUnsignedPointerDiffType(), "unsigned ptrdiff_t")); case LengthModifier::AsAllocate: case LengthModifier::AsMAllocate: case LengthModifier::AsWide: diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 98aeb61..cd2aabb 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -6894,10 +6894,11 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, QualType CastTy; std::tie(CastTy, CastTyName) = shouldNotPrintDirectly(S.Context, IntendedTy, E); if (!CastTy.isNull()) { - // %zi/%zu are OK to use for NSInteger/NSUInteger of type int + // %zi/%zu and %td/%tu are OK to use for NSInteger/NSUInteger of type int // (long in ASTContext). Only complain to pedants. if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") && - AT.isSizeT() && AT.matchesType(S.Context, CastTy)) + (AT.isSizeT() || AT.isPtrdiffT()) && + AT.matchesType(S.Context, CastTy)) Pedantic = true; IntendedTy = CastTy; ShouldNotPrintDirectly = true; diff --git a/clang/test/SemaObjC/format-size-spec-nsinteger.m b/clang/test/SemaObjC/format-size-spec-nsinteger.m index 9eb954e..e7c0a0d 100644 --- a/clang/test/SemaObjC/format-size-spec-nsinteger.m +++ b/clang/test/SemaObjC/format-size-spec-nsinteger.m @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat %s // RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat-pedantic -DPEDANTIC %s +// RUN: %clang_cc1 -triple thumbv7k-apple-watchos2.0.0 -fsyntax-only -fblocks -verify %s +// RUN: %clang_cc1 -triple thumbv7k-apple-watchos2.0.0 -fsyntax-only -fblocks -verify -Wformat-pedantic -DPEDANTIC %s #if !defined(PEDANTIC) // expected-no-diagnostics @@ -8,9 +10,16 @@ #if __LP64__ typedef unsigned long NSUInteger; typedef long NSInteger; +typedef long ptrdiff_t; #else typedef unsigned int NSUInteger; typedef int NSInteger; +#if __is_target_os(watchos) + // Watch ABI uses long for ptrdiff_t. + typedef long ptrdiff_t; +#else + typedef int ptrdiff_t; +#endif #endif @class NSString; @@ -28,3 +37,16 @@ void testSizeSpecifier() { // expected-warning@-4 {{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}} #endif } + +void testPtrdiffSpecifier(ptrdiff_t x) { + NSInteger i = 0; + NSUInteger j = 0; + + NSLog(@"ptrdiff_t NSUinteger: %tu", j); + NSLog(@"ptrdiff_t NSInteger: %td", i); + NSLog(@"ptrdiff_t %tu, %td", x, x); +#if __is_target_os(watchos) && defined(PEDANTIC) + // expected-warning@-4 {{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}} + // expected-warning@-4 {{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}} +#endif +}