r262261 - [analyzer] Don't treat calls to system headers as escaping in CheckObjCDealloc.
Devin Coughlin via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 29 13:44:08 PST 2016
Author: dcoughlin
Date: Mon Feb 29 15:44:08 2016
New Revision: 262261
URL: http://llvm.org/viewvc/llvm-project?rev=262261&view=rev
Log:
[analyzer] Don't treat calls to system headers as escaping in CheckObjCDealloc.
This prevents false negatives when a -dealloc method, for example, removes itself as
as an observer with [[NSNotificationCenter defaultCenter] removeObserver:self]. It is
unlikely that passing 'self' to a system header method will release 'self''s instance
variables, so this is unlikely to produce false positives.
A challenge here is that while CheckObjCDealloc no longer treats these calls as
escaping, the rest of the analyzer still does. In particular, this means that loads
from the same instance variable before and after a call to a system header will
result in different symbols being loaded by the region store. To account for this,
the checker now treats different ivar symbols with the same instance and ivar decl as
the same for the purpose of release checking and more eagerly removes a release
requirement when an instance variable is assumed to be nil. This was not needed before
because when an ivar escaped its release requirement was always removed -- now the
requirement is not removed for calls to system headers.
Added:
cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
cfe/trunk/test/Analysis/DeallocMissingRelease.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp?rev=262261&r1=262260&r2=262261&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Mon Feb 29 15:44:08 2016
@@ -93,6 +93,7 @@ class ObjCDeallocChecker
: public Checker<check::ASTDecl<ObjCImplementationDecl>,
check::PreObjCMessage, check::PostObjCMessage,
check::BeginFunction, check::EndFunction,
+ eval::Assume,
check::PointerEscape,
check::PreStmt<ReturnStmt>> {
@@ -111,6 +112,9 @@ public:
void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
+ ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
ProgramStateRef checkPointerEscape(ProgramStateRef State,
const InvalidatedSymbols &Escaped,
const CallEvent *Call,
@@ -129,6 +133,7 @@ private:
SymbolRef getValueReleasedByNillingOut(const ObjCMethodCall &M,
CheckerContext &C) const;
+ const ObjCIvarRegion *getIvarRegionForIvarSymbol(SymbolRef IvarSym) const;
SymbolRef getInstanceSymbolFromIvarSymbol(SymbolRef IvarSym) const;
ReleaseRequirement
@@ -284,20 +289,31 @@ void ObjCDeallocChecker::checkBeginFunct
}
}
+/// Given a symbol for an ivar, return the ivar region it was loaded from.
+/// Returns nullptr if the instance symbol cannot be found.
+const ObjCIvarRegion *
+ObjCDeallocChecker::getIvarRegionForIvarSymbol(SymbolRef IvarSym) const {
+ const MemRegion *RegionLoadedFrom = nullptr;
+ if (auto *DerivedSym = dyn_cast<SymbolDerived>(IvarSym))
+ RegionLoadedFrom = DerivedSym->getRegion();
+ else if (auto *RegionSym = dyn_cast<SymbolRegionValue>(IvarSym))
+ RegionLoadedFrom = RegionSym->getRegion();
+ else
+ return nullptr;
+
+ return dyn_cast<ObjCIvarRegion>(RegionLoadedFrom);
+}
+
/// Given a symbol for an ivar, return a symbol for the instance containing
/// the ivar. Returns nullptr if the instance symbol cannot be found.
SymbolRef
ObjCDeallocChecker::getInstanceSymbolFromIvarSymbol(SymbolRef IvarSym) const {
- if (auto *SRV = dyn_cast<SymbolRegionValue>(IvarSym)) {
- const TypedValueRegion *TVR = SRV->getRegion();
- const ObjCIvarRegion *IvarRegion = dyn_cast_or_null<ObjCIvarRegion>(TVR);
- if (!IvarRegion)
- return nullptr;
- return IvarRegion->getSymbolicBase()->getSymbol();
- }
+ const ObjCIvarRegion *IvarRegion = getIvarRegionForIvarSymbol(IvarSym);
+ if (!IvarRegion)
+ return nullptr;
- return nullptr;
+ return IvarRegion->getSymbolicBase()->getSymbol();
}
/// If we are in -dealloc or -dealloc is on the stack, handle the call if it is
@@ -362,11 +378,58 @@ void ObjCDeallocChecker::checkPreStmt(
diagnoseMissingReleases(C);
}
+/// When a symbol is assumed to be nil, remove it from the set of symbols
+/// require to be nil.
+ProgramStateRef ObjCDeallocChecker::evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const {
+ if (State->get<UnreleasedIvarMap>().isEmpty())
+ return State;
+
+ auto *CondBSE = dyn_cast_or_null<BinarySymExpr>(Cond.getAsSymExpr());
+ if (!CondBSE)
+ return State;
+
+ BinaryOperator::Opcode OpCode = CondBSE->getOpcode();
+ if (Assumption) {
+ if (OpCode != BO_EQ)
+ return State;
+ } else {
+ if (OpCode != BO_NE)
+ return State;
+ }
+
+ SymbolRef NullSymbol = nullptr;
+ if (auto *SIE = dyn_cast<SymIntExpr>(CondBSE)) {
+ const llvm::APInt &RHS = SIE->getRHS();
+ if (RHS != 0)
+ return State;
+ NullSymbol = SIE->getLHS();
+ } else if (auto *SIE = dyn_cast<IntSymExpr>(CondBSE)) {
+ const llvm::APInt &LHS = SIE->getLHS();
+ if (LHS != 0)
+ return State;
+ NullSymbol = SIE->getRHS();
+ } else {
+ return State;
+ }
+
+ SymbolRef InstanceSymbol = getInstanceSymbolFromIvarSymbol(NullSymbol);
+ if (!InstanceSymbol)
+ return State;
+
+ State = removeValueRequiringRelease(State, InstanceSymbol, NullSymbol);
+
+ return State;
+}
+
/// If a symbol escapes conservatively assume unseen code released it.
ProgramStateRef ObjCDeallocChecker::checkPointerEscape(
ProgramStateRef State, const InvalidatedSymbols &Escaped,
const CallEvent *Call, PointerEscapeKind Kind) const {
+ if (State->get<UnreleasedIvarMap>().isEmpty())
+ return State;
+
// Don't treat calls to '[super dealloc]' as escaping for the purposes
// of this checker. Because the checker diagnoses missing releases in the
// post-message handler for '[super dealloc], escaping here would cause
@@ -376,7 +439,17 @@ ProgramStateRef ObjCDeallocChecker::chec
return State;
for (const auto &Sym : Escaped) {
- State = State->remove<UnreleasedIvarMap>(Sym);
+ if (!Call || (Call && !Call->isInSystemHeader())) {
+ // If Sym is a symbol for an object with instance variables that
+ // must be released, remove these obligations when the object escapes
+ // unless via a call to a system function. System functions are
+ // very unlikely to release instance variables on objects passed to them,
+ // and are frequently called on 'self' in -dealloc (e.g., to remove
+ // observers) -- we want to avoid false negatives from escaping on
+ // them.
+ State = State->remove<UnreleasedIvarMap>(Sym);
+ }
+
SymbolRef InstanceSymbol = getInstanceSymbolFromIvarSymbol(Sym);
if (!InstanceSymbol)
@@ -424,9 +497,10 @@ void ObjCDeallocChecker::diagnoseMissing
if (SelfRegion != IvarRegion->getSuperRegion())
continue;
+ const ObjCIvarDecl *IvarDecl = IvarRegion->getDecl();
// Prevent an inlined call to -dealloc in a super class from warning
// about the values the subclass's -dealloc should release.
- if (IvarRegion->getDecl()->getContainingInterface() !=
+ if (IvarDecl->getContainingInterface() !=
cast<ObjCMethodDecl>(LCtx->getDecl())->getClassInterface())
continue;
@@ -452,7 +526,6 @@ void ObjCDeallocChecker::diagnoseMissing
std::string Buf;
llvm::raw_string_ostream OS(Buf);
- const ObjCIvarDecl *IvarDecl = IvarRegion->getDecl();
const ObjCInterfaceDecl *Interface = IvarDecl->getContainingInterface();
// If the class is known to have a lifecycle with teardown that is
// separate from -dealloc, do not warn about missing releases. We
@@ -521,15 +594,7 @@ bool ObjCDeallocChecker::diagnoseExtraRe
// values that must not be released in the state. This is because even if
// these values escape, it is still an error under the rules of MRR to
// release them in -dealloc.
- const MemRegion *RegionLoadedFrom = nullptr;
- if (auto *DerivedSym = dyn_cast<SymbolDerived>(ReleasedValue))
- RegionLoadedFrom = DerivedSym->getRegion();
- else if (auto *RegionSym = dyn_cast<SymbolRegionValue>(ReleasedValue))
- RegionLoadedFrom = RegionSym->getRegion();
- else
- return false;
-
- auto *ReleasedIvar = dyn_cast<ObjCIvarRegion>(RegionLoadedFrom);
+ auto *ReleasedIvar = getIvarRegionForIvarSymbol(ReleasedValue);
if (!ReleasedIvar)
return false;
@@ -680,6 +745,9 @@ ProgramStateRef ObjCDeallocChecker::remo
ProgramStateRef State, SymbolRef Instance, SymbolRef Value) const {
assert(Instance);
assert(Value);
+ const ObjCIvarRegion *RemovedRegion = getIvarRegionForIvarSymbol(Value);
+ if (!RemovedRegion)
+ return State;
const SymbolSet *Unreleased = State->get<UnreleasedIvarMap>(Instance);
if (!Unreleased)
@@ -687,7 +755,14 @@ ProgramStateRef ObjCDeallocChecker::remo
// Mark the value as no longer requiring a release.
SymbolSet::Factory &F = State->getStateManager().get_context<SymbolSet>();
- SymbolSet NewUnreleased = F.remove(*Unreleased, Value);
+ SymbolSet NewUnreleased = *Unreleased;
+ for (auto &Sym : *Unreleased) {
+ const ObjCIvarRegion *UnreleasedRegion = getIvarRegionForIvarSymbol(Sym);
+ assert(UnreleasedRegion);
+ if (RemovedRegion->getDecl() == UnreleasedRegion->getDecl()) {
+ NewUnreleased = F.remove(NewUnreleased, Sym);
+ }
+ }
if (NewUnreleased.isEmpty()) {
return State->remove<UnreleasedIvarMap>(Instance);
Modified: cfe/trunk/test/Analysis/DeallocMissingRelease.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocMissingRelease.m?rev=262261&r1=262260&r2=262261&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/DeallocMissingRelease.m (original)
+++ cfe/trunk/test/Analysis/DeallocMissingRelease.m Mon Feb 29 15:44:08 2016
@@ -1,7 +1,7 @@
// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -verify %s
// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -triple x86_64-apple-darwin10 -fobjc-arc -fobjc-runtime-has-weak -verify %s
-#define nil ((id)0)
+#include "Inputs/system-header-simulator-for-objc-dealloc.h"
#define NON_ARC !__has_feature(objc_arc)
@@ -16,22 +16,6 @@
// expected-no-diagnostics
#endif
-typedef signed char BOOL;
- at protocol NSObject
-- (BOOL)isEqual:(id)object;
-- (Class)class;
- at end
-
- at interface NSObject <NSObject> {}
-+ (instancetype)alloc;
-- (void)dealloc;
-- (id)init;
-- (id)retain;
-- (oneway void)release;
- at end
-
-typedef struct objc_selector *SEL;
-
// Do not warn about missing release in -dealloc for ivars.
@interface MyIvarClass1 : NSObject {
@@ -446,6 +430,50 @@ void NilOutPropertyHelper(ClassWithDeall
#endif
}
@end
+
+
+// Don't treat calls to system headers as escapes
+
+ at interface ClassWhereSelfEscapesViaCallToSystem : NSObject
+ at property (retain) NSObject *ivar1;
+ at property (retain) NSObject *ivar2;
+ at property (retain) NSObject *ivar3;
+ at property (retain) NSObject *ivar4;
+ at property (retain) NSObject *ivar5;
+ at property (retain) NSObject *ivar6;
+ at end
+
+ at implementation ClassWhereSelfEscapesViaCallToSystem
+- (void)dealloc; {
+#if NON_ARC
+ [_ivar2 release];
+ if (_ivar3) {
+ [_ivar3 release];
+ }
+#endif
+
+ [[NSRunLoop currentRunLoop] cancelPerformSelectorsWithTarget:self];
+ [[NSNotificationCenter defaultCenter] removeObserver:self];
+
+#if NON_ARC
+ [_ivar4 release];
+
+ if (_ivar5) {
+ [_ivar5 release];
+ }
+#endif
+
+ [[NSNotificationCenter defaultCenter] removeObserver:self];
+
+#if NON_ARC
+ if (_ivar6) {
+ [_ivar6 release];
+ }
+
+ [super dealloc]; // expected-warning {{The '_ivar1' ivar in 'ClassWhereSelfEscapesViaCallToSystem' was retained by a synthesized property but not released before '[super dealloc]'}}
+#endif
+}
+ at end
// Don't warn when value escapes.
Added: 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=262261&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h (added)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h Mon Feb 29 15:44:08 2016
@@ -0,0 +1,29 @@
+#pragma clang system_header
+
+#define nil ((id)0)
+
+typedef signed char BOOL;
+ at protocol NSObject
+- (BOOL)isEqual:(id)object;
+- (Class)class;
+ at end
+
+ at interface NSObject <NSObject> {}
++ (instancetype)alloc;
+- (void)dealloc;
+- (id)init;
+- (id)retain;
+- (oneway void)release;
+ at end
+
+ at interface NSRunLoop : NSObject
++ (NSRunLoop *)currentRunLoop;
+- (void)cancelPerformSelectorsWithTarget:(id)target;
+ at end
+
+ at interface NSNotificationCenter : NSObject
++ (NSNotificationCenter *)defaultCenter;
+- (void)removeObserver:(id)observer;
+ at end
+
+typedef struct objc_selector *SEL;
More information about the cfe-commits
mailing list