r285253 - [analyzer] Report CFNumberGetValue API misuse
Anna Zaks via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 26 15:51:47 PDT 2016
Author: zaks
Date: Wed Oct 26 17:51:47 2016
New Revision: 285253
URL: http://llvm.org/viewvc/llvm-project?rev=285253&view=rev
Log:
[analyzer] Report CFNumberGetValue API misuse
This patch contains 2 improvements to the CFNumber checker:
- Checking of CFNumberGetValue misuse.
- Treating all CFNumber API misuse errors as non-fatal. (Previously we treated errors that could cause uninitialized memory as syncs and the truncation errors as non-fatal.)
This implements a subset of functionality from https://reviews.llvm.org/D17954.
Differential Revision: https://reviews.llvm.org/D25876
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
cfe/trunk/test/Analysis/CFNumber.c
Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=285253&r1=285252&r2=285253&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Wed Oct 26 17:51:47 2016
@@ -586,8 +586,8 @@ def DirectIvarAssignmentForAnnotatedFunc
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">,
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp?rev=285253&r1=285252&r2=285253&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp Wed Oct 26 17:51:47 2016
@@ -336,15 +336,15 @@ void NilArgChecker::checkPostStmt(const
}
//===----------------------------------------------------------------------===//
-// Error reporting.
+// Checking for mismatched types passed to CFNumberCreate/CFNumberGetValue.
//===----------------------------------------------------------------------===//
namespace {
-class CFNumberCreateChecker : public Checker< check::PreStmt<CallExpr> > {
+class CFNumberChecker : public Checker< check::PreStmt<CallExpr> > {
mutable std::unique_ptr<APIMisuse> 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 @@ static const char* GetCFNumberTypeStr(ui
}
#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 @@ void CFNumberCreateChecker::checkPreStmt
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 @@ void CFNumberCreateChecker::checkPreStmt
return;
uint64_t NumberKind = V->getValue().getLimitedValue();
- Optional<uint64_t> OptTargetSize = GetCFNumberSize(Ctx, NumberKind);
+ Optional<uint64_t> 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 @@ void CFNumberCreateChecker::checkPreStmt
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<BugReport>(*BT, os.str(), N);
report->addRange(CE->getArg(2)->getSourceRange());
@@ -1272,8 +1279,8 @@ void ento::registerNilArgChecker(Checker
mgr.registerChecker<NilArgChecker>();
}
-void ento::registerCFNumberCreateChecker(CheckerManager &mgr) {
- mgr.registerChecker<CFNumberCreateChecker>();
+void ento::registerCFNumberChecker(CheckerManager &mgr) {
+ mgr.registerChecker<CFNumberChecker>();
}
void ento::registerCFRetainReleaseChecker(CheckerManager &mgr) {
Modified: cfe/trunk/test/Analysis/CFNumber.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/CFNumber.c?rev=285253&r1=285252&r2=285253&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/CFNumber.c (original)
+++ cfe/trunk/test/Analysis/CFNumber.c Wed Oct 26 17:51:47 2016
@@ -13,14 +13,16 @@ enum { kCFNumberSInt8Type = 1, kCFNumber
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 @@ __attribute__((cf_returns_not_retained))
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;
}
More information about the cfe-commits
mailing list