Skip to content

Commit

Permalink
[Sema] -Wformat-pedantic only for NSInteger/NSUInteger %tu/%td on Darwin
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hyp committed Jul 5, 2018
1 parent bd09f77 commit b2043ac
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 7 deletions.
21 changes: 18 additions & 3 deletions clang/include/clang/Analysis/Analyses/FormatString.h
Expand Up @@ -257,7 +257,12 @@ class ArgType {
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) {}
Expand All @@ -267,7 +272,9 @@ class ArgType {
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) {
Expand All @@ -280,7 +287,15 @@ class ArgType {
/// 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;
}

Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Analysis/PrintfFormatString.cpp
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Sema/SemaChecking.cpp
Expand Up @@ -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;
Expand Down
22 changes: 22 additions & 0 deletions 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
Expand All @@ -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;
Expand All @@ -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
}

0 comments on commit b2043ac

Please sign in to comment.