From 721567af3ec2a0f453f3cd9380a36a9ded7158f5 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 7 Nov 2012 17:12:37 +0000 Subject: [PATCH] [analyzer] Check that the argument to CFMakeCollectable is non-NULL. Patch by Sean McBride! llvm-svn: 167537 --- .../Checkers/BasicObjCFoundationChecks.cpp | 26 ++++++++++++++-------- clang/lib/StaticAnalyzer/Checkers/Checkers.td | 2 +- .../test/Analysis/diagnostics/undef-value-param.m | 2 +- clang/test/Analysis/retain-release.m | 19 +++++++++++----- clang/tools/scan-build/scan-build.1 | 5 +++-- clang/www/analyzer/available_checks.html | 2 +- 6 files changed, 37 insertions(+), 19 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 0dd6478..eba534e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -363,15 +363,15 @@ void CFNumberCreateChecker::checkPreStmt(const CallExpr *CE, } //===----------------------------------------------------------------------===// -// CFRetain/CFRelease checking for null arguments. +// CFRetain/CFRelease/CFMakeCollectable checking for null arguments. //===----------------------------------------------------------------------===// namespace { class CFRetainReleaseChecker : public Checker< check::PreStmt > { mutable OwningPtr BT; - mutable IdentifierInfo *Retain, *Release; + mutable IdentifierInfo *Retain, *Release, *MakeCollectable; public: - CFRetainReleaseChecker(): Retain(0), Release(0) {} + CFRetainReleaseChecker(): Retain(0), Release(0), MakeCollectable(0) {} void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; }; } // end anonymous namespace @@ -392,12 +392,14 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE, ASTContext &Ctx = C.getASTContext(); Retain = &Ctx.Idents.get("CFRetain"); Release = &Ctx.Idents.get("CFRelease"); - BT.reset(new APIMisuse("null passed to CFRetain/CFRelease")); + MakeCollectable = &Ctx.Idents.get("CFMakeCollectable"); + BT.reset( + new APIMisuse("null passed to CFRetain/CFRelease/CFMakeCollectable")); } - // Check if we called CFRetain/CFRelease. + // Check if we called CFRetain/CFRelease/CFMakeCollectable. const IdentifierInfo *FuncII = FD->getIdentifier(); - if (!(FuncII == Retain || FuncII == Release)) + if (!(FuncII == Retain || FuncII == Release || FuncII == MakeCollectable)) return; // FIXME: The rest of this just checks that the argument is non-null. @@ -426,9 +428,15 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE, if (!N) return; - const char *description = (FuncII == Retain) - ? "Null pointer argument in call to CFRetain" - : "Null pointer argument in call to CFRelease"; + const char *description; + if (FuncII == Retain) + description = "Null pointer argument in call to CFRetain"; + else if (FuncII == Release) + description = "Null pointer argument in call to CFRelease"; + else if (FuncII == MakeCollectable) + description = "Null pointer argument in call to CFMakeCollectable"; + else + llvm_unreachable("impossible case"); BugReport *report = new BugReport(*BT, description, N); report->addRange(Arg->getSourceRange()); diff --git a/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/clang/lib/StaticAnalyzer/Checkers/Checkers.td index b4b40fa..235e633 100644 --- a/clang/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/lib/StaticAnalyzer/Checkers/Checkers.td @@ -433,7 +433,7 @@ def CFNumberCreateChecker : Checker<"CFNumber">, DescFile<"BasicObjCFoundationChecks.cpp">; def CFRetainReleaseChecker : Checker<"CFRetainRelease">, - HelpText<"Check for null arguments to CFRetain/CFRelease">, + HelpText<"Check for null arguments to CFRetain/CFRelease/CFMakeCollectable">, DescFile<"BasicObjCFoundationChecks.cpp">; def CFErrorChecker : Checker<"CFError">, diff --git a/clang/test/Analysis/diagnostics/undef-value-param.m b/clang/test/Analysis/diagnostics/undef-value-param.m index ae82839..d2a7a08 100644 --- a/clang/test/Analysis/diagnostics/undef-value-param.m +++ b/clang/test/Analysis/diagnostics/undef-value-param.m @@ -460,7 +460,7 @@ static void CreateRef(SCDynamicStoreRef *storeRef, unsigned x) { //CHECK: //CHECK: descriptionNull pointer argument in call to CFRelease //CHECK: categoryAPI Misuse (Apple) -//CHECK: typenull passed to CFRetain/CFRelease +//CHECK: typenull passed to CFRetain/CFRelease/CFMakeCollectable //CHECK: issue_context_kindObjective-C method //CHECK: issue_contexttest //CHECK: issue_hash5 diff --git a/clang/test/Analysis/retain-release.m b/clang/test/Analysis/retain-release.m index 514bd12..eb2554f 100644 --- a/clang/test/Analysis/retain-release.m +++ b/clang/test/Analysis/retain-release.m @@ -62,6 +62,7 @@ typedef const struct __CFAllocator * CFAllocatorRef; extern const CFAllocatorRef kCFAllocatorDefault; extern CFTypeRef CFRetain(CFTypeRef cf); extern void CFRelease(CFTypeRef cf); +extern CFTypeRef CFMakeCollectable(CFTypeRef cf); typedef struct { } CFArrayCallBacks; @@ -508,31 +509,39 @@ void f15() { CFRelease(*B); // no-warning } -// Test when we pass NULL to CFRetain/CFRelease. +// Test when we pass NULL to CFRetain/CFRelease/CFMakeCollectable. void f16(int x, CFTypeRef p) { if (p) return; - if (x) { + if (x > 0) { CFRelease(p); // expected-warning{{Null pointer argument in call to CFRelease}} } - else { + else if (x < 0) { CFRetain(p); // expected-warning{{Null pointer argument in call to CFRetain}} } + else { + CFMakeCollectable(p); // expected-warning{{Null pointer argument in call to CFMakeCollectable}} + } } // Test that an object is non-null after being CFRetained/CFReleased. void f17(int x, CFTypeRef p) { - if (x) { + if (x > 0) { CFRelease(p); if (!p) CFRelease(0); // no-warning } - else { + else if (x < 0) { CFRetain(p); if (!p) CFRetain(0); // no-warning } + else { + CFMakeCollectable(p); + if (!p) + CFMakeCollectable(0); // no-warning + } } // Test basic tracking of ivars associated with 'self'. For the retain/release diff --git a/clang/tools/scan-build/scan-build.1 b/clang/tools/scan-build/scan-build.1 index e53acdb..10ddc7f 100644 --- a/clang/tools/scan-build/scan-build.1 +++ b/clang/tools/scan-build/scan-build.1 @@ -270,9 +270,10 @@ Check for proper uses of .Fn CFNumberCreate . .It osx.coreFoundation.CFRetainRelease Check for null arguments to -.Fn CFRetain +.Fn CFRetain , +.Fn CFRelease , and -.Fn CFRelease . +.Fn CFMakeCollectable . .It osx.coreFoundation.containers.OutOfBounds Checks for index out-of-bounds when using the .Vt CFArray diff --git a/clang/www/analyzer/available_checks.html b/clang/www/analyzer/available_checks.html index 12d836c..4f8971c 100644 --- a/clang/www/analyzer/available_checks.html +++ b/clang/www/analyzer/available_checks.html @@ -125,7 +125,7 @@ osx.coreFoundation.CFNumberCheck for proper uses of CFNumberCreate. -osx.coreFoundation.CFRetainReleaseCheck for null arguments to CFRetain/CFRelease. +osx.coreFoundation.CFRetainReleaseCheck for null arguments to CFRetain/CFRelease/CFMakeCollectable. osx.coreFoundation.containers.OutOfBoundsChecks for index out-of-bounds when using 'CFArray' API. -- 2.7.4