r264463 - [analyzer] Add CIFIlter modeling to DeallocChecker.
Devin Coughlin via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 25 14:18:23 PDT 2016
Author: dcoughlin
Date: Fri Mar 25 16:18:22 2016
New Revision: 264463
URL: http://llvm.org/viewvc/llvm-project?rev=264463&view=rev
Log:
[analyzer] Add CIFIlter modeling to DeallocChecker.
The -dealloc method in CIFilter is highly unusual in that it will release
instance variables belonging to its *subclasses* if the variable name
starts with "input" or backs a property whose name starts with "input".
Subclasses should not release these ivars in their own -dealloc method --
doing so could result in an over release.
Before this commit, the DeallocChecker would warn about missing releases for
such "input" properties -- which could cause users of the analyzer to add
over releases to silence the warning.
To avoid this, DeallocChecker now treats CIFilter "input-prefixed" ivars
as MustNotReleaseDirectly and so will not require a release. Further, it
will now warn when such an ivar is directly released in -dealloc.
rdar://problem/25364901
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
cfe/trunk/test/Analysis/DeallocMissingRelease.m
cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp?rev=264463&r1=264462&r2=264463&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Fri Mar 25 16:18:22 2016
@@ -98,7 +98,8 @@ class ObjCDeallocChecker
check::PointerEscape,
check::PreStmt<ReturnStmt>> {
- mutable IdentifierInfo *NSObjectII, *SenTestCaseII, *Block_releaseII;
+ mutable IdentifierInfo *NSObjectII, *SenTestCaseII, *Block_releaseII,
+ *CIFilterII;
mutable Selector DeallocSel, ReleaseSel;
std::unique_ptr<BugType> MissingReleaseBugType;
@@ -169,6 +170,8 @@ private:
void initIdentifierInfoAndSelectors(ASTContext &Ctx) const;
bool classHasSeparateTeardown(const ObjCInterfaceDecl *ID) const;
+
+ bool isReleasedByCIFilterDealloc(const ObjCPropertyImplDecl *PropImpl) const;
};
} // End anonymous namespace.
@@ -688,19 +691,27 @@ bool ObjCDeallocChecker::diagnoseExtraRe
assert(PropDecl->getSetterKind() == ObjCPropertyDecl::Weak ||
(PropDecl->getSetterKind() == ObjCPropertyDecl::Assign &&
- !PropDecl->isReadOnly()));
+ !PropDecl->isReadOnly()) ||
+ isReleasedByCIFilterDealloc(PropImpl)
+ );
const ObjCImplDecl *Container = getContainingObjCImpl(C.getLocationContext());
OS << "The '" << *PropImpl->getPropertyIvarDecl()
- << "' ivar in '" << *Container
- << "' was synthesized for ";
+ << "' ivar in '" << *Container;
- if (PropDecl->getSetterKind() == ObjCPropertyDecl::Weak)
- OS << "a weak";
- else
- OS << "an assign, readwrite";
- OS << " property but was released in 'dealloc'";
+ if (isReleasedByCIFilterDealloc(PropImpl)) {
+ OS << "' will be released by '-[CIFilter dealloc]' but also released here";
+ } else {
+ OS << "' was synthesized for ";
+
+ if (PropDecl->getSetterKind() == ObjCPropertyDecl::Weak)
+ OS << "a weak";
+ else
+ OS << "an assign, readwrite";
+
+ OS << " property but was released in 'dealloc'";
+ }
std::unique_ptr<BugReport> BR(
new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode));
@@ -751,7 +762,7 @@ bool ObjCDeallocChecker::diagnoseMistake
ObjCDeallocChecker::
ObjCDeallocChecker()
- : NSObjectII(nullptr), SenTestCaseII(nullptr) {
+ : NSObjectII(nullptr), SenTestCaseII(nullptr), CIFilterII(nullptr) {
MissingReleaseBugType.reset(
new BugType(this, "Missing ivar release (leak)",
@@ -774,6 +785,7 @@ void ObjCDeallocChecker::initIdentifierI
NSObjectII = &Ctx.Idents.get("NSObject");
SenTestCaseII = &Ctx.Idents.get("SenTestCase");
Block_releaseII = &Ctx.Idents.get("_Block_release");
+ CIFilterII = &Ctx.Idents.get("CIFilter");
IdentifierInfo *DeallocII = &Ctx.Idents.get("dealloc");
IdentifierInfo *ReleaseII = &Ctx.Idents.get("release");
@@ -894,6 +906,9 @@ ReleaseRequirement ObjCDeallocChecker::g
// the value in their instance variables must be released in -dealloc.
case ObjCPropertyDecl::Retain:
case ObjCPropertyDecl::Copy:
+ if (isReleasedByCIFilterDealloc(PropImpl))
+ return ReleaseRequirement::MustNotReleaseDirectly;
+
return ReleaseRequirement::MustRelease;
case ObjCPropertyDecl::Weak:
@@ -1019,6 +1034,37 @@ bool ObjCDeallocChecker::classHasSeparat
return true;
}
+/// The -dealloc method in CIFilter highly unusual in that is will release
+/// instance variables belonging to its *subclasses* if the variable name
+/// starts with "input" or backs a property whose name starts with "input".
+/// Subclasses should not release these ivars in their own -dealloc method --
+/// doing so could result in an over release.
+///
+/// This method returns true if the property will be released by
+/// -[CIFilter dealloc].
+bool ObjCDeallocChecker::isReleasedByCIFilterDealloc(
+ const ObjCPropertyImplDecl *PropImpl) const {
+ assert(PropImpl->getPropertyIvarDecl());
+ StringRef PropName = PropImpl->getPropertyDecl()->getName();
+ StringRef IvarName = PropImpl->getPropertyIvarDecl()->getName();
+
+ const char *ReleasePrefix = "input";
+ if (!(PropName.startswith(ReleasePrefix) ||
+ IvarName.startswith(ReleasePrefix))) {
+ return false;
+ }
+
+ const ObjCInterfaceDecl *ID =
+ PropImpl->getPropertyIvarDecl()->getContainingInterface();
+ for ( ; ID ; ID = ID->getSuperClass()) {
+ IdentifierInfo *II = ID->getIdentifier();
+ if (II == CIFilterII)
+ return true;
+ }
+
+ return false;
+}
+
void ento::registerObjCDeallocChecker(CheckerManager &Mgr) {
const LangOptions &LangOpts = Mgr.getLangOpts();
// These checker only makes sense under MRR.
Modified: cfe/trunk/test/Analysis/DeallocMissingRelease.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocMissingRelease.m?rev=264463&r1=264462&r2=264463&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/DeallocMissingRelease.m (original)
+++ cfe/trunk/test/Analysis/DeallocMissingRelease.m Fri Mar 25 16:18:22 2016
@@ -747,10 +747,100 @@ __attribute__((objc_root_class))
{
#if NON_ARC
// Only warn for synthesized ivars.
- [okToDeallocDirectly dealloc]; // now-warning
+ [okToDeallocDirectly dealloc]; // no-warning
[_ivar dealloc]; // expected-warning {{'_ivar' should be released rather than deallocated}}
[super dealloc];
#endif
}
@end
+
+// CIFilter special cases.
+// By design, -[CIFilter dealloc] releases (by calling -setValue: forKey: with
+// 'nil') all ivars (even in its *subclasses*) with names starting with
+// 'input' or that are backed by properties with names starting with 'input'.
+// The Dealloc checker needs to take particular care to not warn about missing
+// releases in this case -- if the user adds a release quiet the
+// warning it may result in an over release.
+
+ at interface ImmediateSubCIFilter : CIFilter {
+ NSObject *inputIvar;
+ NSObject *nonInputIvar;
+ NSObject *notPrefixedButBackingPrefixedProperty;
+ NSObject *inputPrefixedButBackingNonPrefixedProperty;
+}
+
+ at property(retain) NSObject *inputIvar;
+ at property(retain) NSObject *nonInputIvar;
+ at property(retain) NSObject *inputAutoSynthesizedIvar;
+ at property(retain) NSObject *inputExplicitlySynthesizedToNonPrefixedIvar;
+ at property(retain) NSObject *nonPrefixedPropertyBackedByExplicitlySynthesizedPrefixedIvar;
+
+ at end
+
+ at implementation ImmediateSubCIFilter
+ at synthesize inputIvar = inputIvar;
+ at synthesize nonInputIvar = nonInputIvar;
+ at synthesize inputExplicitlySynthesizedToNonPrefixedIvar = notPrefixedButBackingPrefixedProperty;
+ at synthesize nonPrefixedPropertyBackedByExplicitlySynthesizedPrefixedIvar = inputPrefixedButBackingNonPrefixedProperty;
+
+- (void)dealloc {
+#if NON_ARC
+ // We don't want warnings here for:
+ // inputIvar
+ // inputAutoSynthesizedIvar
+ // inputExplicitlySynthesizedToNonPrefixedIvar
+ // inputPrefixedButBackingNonPrefixedProperty
+ [super dealloc];
+ // expected-warning at -1 {{The 'nonInputIvar' ivar in 'ImmediateSubCIFilter' was retained by a synthesized property but not released before '[super dealloc]'}}
+#endif
+}
+ at end
+
+ at interface SubSubCIFilter : CIFilter {
+ NSObject *inputIvarInSub;
+}
+
+ at property(retain) NSObject *inputIvarInSub;
+ at end
+
+ at implementation SubSubCIFilter
+ at synthesize inputIvarInSub = inputIvarInSub;
+
+- (void)dealloc {
+// Don't warn about inputIvarInSub.
+#if NON_ARC
+ [super dealloc];
+#endif
+}
+ at end
+ at interface OverreleasingCIFilter : CIFilter {
+ NSObject *inputIvar;
+}
+
+ at property(retain) NSObject *inputIvar;
+ at end
+
+ at implementation OverreleasingCIFilter
+ at synthesize inputIvar = inputIvar;
+
+- (void)dealloc {
+#if NON_ARC
+ // This is an over release because CIFilter's dealloc will also release it.
+ [inputIvar release]; // expected-warning {{The 'inputIvar' ivar in 'OverreleasingCIFilter' will be released by '-[CIFilter dealloc]' but also released here}}
+ [super dealloc]; // no-warning
+ #endif
+}
+ at end
+
+
+ at interface NotMissingDeallocCIFilter : CIFilter {
+ NSObject *inputIvar;
+}
+
+ at property(retain) NSObject *inputIvar;
+ at end
+
+ at implementation NotMissingDeallocCIFilter // no-warning
+ at synthesize inputIvar = inputIvar;
+ at end
Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h?rev=264463&r1=264462&r2=264463&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h Fri Mar 25 16:18:22 2016
@@ -30,3 +30,6 @@ typedef struct objc_selector *SEL;
void _Block_release(const void *aBlock);
#define Block_release(...) _Block_release((const void *)(__VA_ARGS__))
+
+ at interface CIFilter : NSObject
+ at end
More information about the cfe-commits
mailing list