Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -586,8 +586,8 @@ let ParentPackage = CoreFoundation in { -def CFNumberCreateChecker : Checker<"CFNumber">, - HelpText<"Check for proper uses of CFNumberCreate">, +def CFNumberChecker : Checker<"CFNumber">, + HelpText<"Check for proper uses of CFNumber APIs">, DescFile<"BasicObjCFoundationChecks.cpp">; def CFRetainReleaseChecker : Checker<"CFRetainRelease">, Index: cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -336,15 +336,15 @@ } //===----------------------------------------------------------------------===// -// Error reporting. +// Checking for mismatched types passed to CFNumberCreate/CFNumberGetValue. //===----------------------------------------------------------------------===// namespace { -class CFNumberCreateChecker : public Checker< check::PreStmt > { +class CFNumberChecker : public Checker< check::PreStmt > { mutable std::unique_ptr BT; - mutable IdentifierInfo* II; + mutable IdentifierInfo *ICreate, *IGetValue; public: - CFNumberCreateChecker() : II(nullptr) {} + CFNumberChecker() : ICreate(nullptr), IGetValue(nullptr) {} void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; @@ -425,7 +425,7 @@ } #endif -void CFNumberCreateChecker::checkPreStmt(const CallExpr *CE, +void CFNumberChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { ProgramStateRef state = C.getState(); const FunctionDecl *FD = C.getCalleeDecl(CE); @@ -433,10 +433,12 @@ return; ASTContext &Ctx = C.getASTContext(); - if (!II) - II = &Ctx.Idents.get("CFNumberCreate"); - - if (FD->getIdentifier() != II || CE->getNumArgs() != 3) + if (!ICreate) { + ICreate = &Ctx.Idents.get("CFNumberCreate"); + IGetValue = &Ctx.Idents.get("CFNumberGetValue"); + } + if (!(FD->getIdentifier() == ICreate || FD->getIdentifier() == IGetValue) || + CE->getNumArgs() != 3) return; // Get the value of the "theType" argument. @@ -450,13 +452,13 @@ return; uint64_t NumberKind = V->getValue().getLimitedValue(); - Optional OptTargetSize = GetCFNumberSize(Ctx, NumberKind); + Optional OptCFNumberSize = GetCFNumberSize(Ctx, NumberKind); // FIXME: In some cases we can emit an error. - if (!OptTargetSize) + if (!OptCFNumberSize) return; - uint64_t TargetSize = *OptTargetSize; + uint64_t CFNumberSize = *OptCFNumberSize; // Look at the value of the integer being passed by reference. Essentially // we want to catch cases where the value passed in is not equal to the @@ -481,39 +483,44 @@ if (!T->isIntegralOrEnumerationType()) return; - uint64_t SourceSize = Ctx.getTypeSize(T); + uint64_t PrimitiveTypeSize = Ctx.getTypeSize(T); - // CHECK: is SourceSize == TargetSize - if (SourceSize == TargetSize) + if (PrimitiveTypeSize == CFNumberSize) return; - // Generate an error. Only generate a sink error node - // if 'SourceSize < TargetSize'; otherwise generate a non-fatal error node. - // // FIXME: We can actually create an abstract "CFNumber" object that has // the bits initialized to the provided values. - // - ExplodedNode *N = SourceSize < TargetSize ? C.generateErrorNode() - : C.generateNonFatalErrorNode(); + ExplodedNode *N = C.generateNonFatalErrorNode(); if (N) { SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); + bool isCreate = (FD->getIdentifier() == ICreate); + + if (isCreate) { + os << (PrimitiveTypeSize == 8 ? "An " : "A ") + << PrimitiveTypeSize << "-bit integer is used to initialize a " + << "CFNumber object that represents " + << (CFNumberSize == 8 ? "an " : "a ") + << CFNumberSize << "-bit integer; "; + } else { + os << "A CFNumber object that represents " + << (CFNumberSize == 8 ? "an " : "a ") + << CFNumberSize << "-bit integer is used to initialize " + << (PrimitiveTypeSize == 8 ? "an " : "a ") + << PrimitiveTypeSize << "-bit integer; "; + } - os << (SourceSize == 8 ? "An " : "A ") - << SourceSize << " bit integer is used to initialize a CFNumber " - "object that represents " - << (TargetSize == 8 ? "an " : "a ") - << TargetSize << " bit integer. "; - - if (SourceSize < TargetSize) - os << (TargetSize - SourceSize) - << " bits of the CFNumber value will be garbage." ; + if (PrimitiveTypeSize < CFNumberSize) + os << (CFNumberSize - PrimitiveTypeSize) + << " bits of the CFNumber value will " + << (isCreate ? "be garbage." : "overwrite adjacent storage."); else - os << (SourceSize - TargetSize) - << " bits of the input integer will be lost."; + os << (PrimitiveTypeSize - CFNumberSize) + << " bits of the integer value will be " + << (isCreate ? "lost." : "garbage."); if (!BT) - BT.reset(new APIMisuse(this, "Bad use of CFNumberCreate")); + BT.reset(new APIMisuse(this, "Bad use of CFNumber APIs")); auto report = llvm::make_unique(*BT, os.str(), N); report->addRange(CE->getArg(2)->getSourceRange()); @@ -1272,8 +1279,8 @@ mgr.registerChecker(); } -void ento::registerCFNumberCreateChecker(CheckerManager &mgr) { - mgr.registerChecker(); +void ento::registerCFNumberChecker(CheckerManager &mgr) { + mgr.registerChecker(); } void ento::registerCFRetainReleaseChecker(CheckerManager &mgr) { Index: cfe/trunk/test/Analysis/CFNumber.c =================================================================== --- cfe/trunk/test/Analysis/CFNumber.c +++ cfe/trunk/test/Analysis/CFNumber.c @@ -13,14 +13,16 @@ kCFNumberMaxType = 16 }; typedef CFIndex CFNumberType; typedef const struct __CFNumber * CFNumberRef; +typedef unsigned char Boolean; extern CFNumberRef CFNumberCreate(CFAllocatorRef allocator, CFNumberType theType, const void *valuePtr); +Boolean CFNumberGetValue(CFNumberRef number, CFNumberType theType, void *valuePtr); -CFNumberRef f1(unsigned char x) { - return CFNumberCreate(0, kCFNumberSInt16Type, &x); // expected-warning{{An 8 bit integer is used to initialize a CFNumber object that represents a 16 bit integer. 8 bits of the CFNumber value will be garbage}} +__attribute__((cf_returns_retained)) CFNumberRef f1(unsigned char x) { + return CFNumberCreate(0, kCFNumberSInt16Type, &x); // expected-warning{{An 8-bit integer is used to initialize a CFNumber object that represents a 16-bit integer; 8 bits of the CFNumber value will be garbage}} } __attribute__((cf_returns_retained)) CFNumberRef f2(unsigned short x) { - return CFNumberCreate(0, kCFNumberSInt8Type, &x); // expected-warning{{A 16 bit integer is used to initialize a CFNumber object that represents an 8 bit integer. 8 bits of the input integer will be lost}} + return CFNumberCreate(0, kCFNumberSInt8Type, &x); // expected-warning{{A 16-bit integer is used to initialize a CFNumber object that represents an 8-bit integer; 8 bits of the integer value will be lost}} } // test that the attribute overrides the naming convention. @@ -28,6 +30,18 @@ return CFNumberCreate(0, kCFNumberSInt8Type, &x); // expected-warning{{leak}} } -CFNumberRef f3(unsigned i) { - return CFNumberCreate(0, kCFNumberLongType, &i); // expected-warning{{A 32 bit integer is used to initialize a CFNumber object that represents a 64 bit integer}} +__attribute__((cf_returns_retained)) CFNumberRef f3(unsigned i) { + return CFNumberCreate(0, kCFNumberLongType, &i); // expected-warning{{A 32-bit integer is used to initialize a CFNumber object that represents a 64-bit integer}} +} + +unsigned char getValueTest1(CFNumberRef x) { + unsigned char scalar = 0; + CFNumberGetValue(x, kCFNumberSInt16Type, &scalar); // expected-warning{{A CFNumber object that represents a 16-bit integer is used to initialize an 8-bit integer; 8 bits of the CFNumber value will overwrite adjacent storage}} + return scalar; +} + +unsigned char getValueTest2(CFNumberRef x) { + unsigned short scalar = 0; + CFNumberGetValue(x, kCFNumberSInt8Type, &scalar); // expected-warning{{A CFNumber object that represents an 8-bit integer is used to initialize a 16-bit integer; 8 bits of the integer value will be garbage}} + return scalar; }