diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index 42999e2fd303..f72a1aeb8a97 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -93,6 +93,7 @@ class ObjCDeallocChecker : public Checker, check::PreObjCMessage, check::PostObjCMessage, check::BeginFunction, check::EndFunction, + eval::Assume, check::PointerEscape, check::PreStmt> { @@ -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::checkBeginFunction( } } +/// 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(IvarSym)) + RegionLoadedFrom = DerivedSym->getRegion(); + else if (auto *RegionSym = dyn_cast(IvarSym)) + RegionLoadedFrom = RegionSym->getRegion(); + else + return nullptr; + + return dyn_cast(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(IvarSym)) { - const TypedValueRegion *TVR = SRV->getRegion(); - const ObjCIvarRegion *IvarRegion = dyn_cast_or_null(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().isEmpty()) + return State; + + auto *CondBSE = dyn_cast_or_null(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(CondBSE)) { + const llvm::APInt &RHS = SIE->getRHS(); + if (RHS != 0) + return State; + NullSymbol = SIE->getLHS(); + } else if (auto *SIE = dyn_cast(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().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::checkPointerEscape( return State; for (const auto &Sym : Escaped) { - State = State->remove(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(Sym); + } + SymbolRef InstanceSymbol = getInstanceSymbolFromIvarSymbol(Sym); if (!InstanceSymbol) @@ -424,9 +497,10 @@ void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const { 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(LCtx->getDecl())->getClassInterface()) continue; @@ -452,7 +526,6 @@ void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const { 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::diagnoseExtraRelease(SymbolRef ReleasedValue, // 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(ReleasedValue)) - RegionLoadedFrom = DerivedSym->getRegion(); - else if (auto *RegionSym = dyn_cast(ReleasedValue)) - RegionLoadedFrom = RegionSym->getRegion(); - else - return false; - - auto *ReleasedIvar = dyn_cast(RegionLoadedFrom); + auto *ReleasedIvar = getIvarRegionForIvarSymbol(ReleasedValue); if (!ReleasedIvar) return false; @@ -680,6 +745,9 @@ ProgramStateRef ObjCDeallocChecker::removeValueRequiringRelease( 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(Instance); if (!Unreleased) @@ -687,7 +755,14 @@ ProgramStateRef ObjCDeallocChecker::removeValueRequiringRelease( // Mark the value as no longer requiring a release. SymbolSet::Factory &F = State->getStateManager().get_context(); - 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(Instance); diff --git a/clang/test/Analysis/DeallocMissingRelease.m b/clang/test/Analysis/DeallocMissingRelease.m index c7769db3d5f2..bb67e436b2b1 100644 --- a/clang/test/Analysis/DeallocMissingRelease.m +++ b/clang/test/Analysis/DeallocMissingRelease.m @@ -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; -@protocol NSObject -- (BOOL)isEqual:(id)object; -- (Class)class; -@end - -@interface NSObject {} -+ (instancetype)alloc; -- (void)dealloc; -- (id)init; -- (id)retain; -- (oneway void)release; -@end - -typedef struct objc_selector *SEL; - // Do not warn about missing release in -dealloc for ivars. @interface MyIvarClass1 : NSObject { @@ -447,6 +431,50 @@ void NilOutPropertyHelper(ClassWithDeallocHelpers *o) { } @end + +// Don't treat calls to system headers as escapes + +@interface ClassWhereSelfEscapesViaCallToSystem : NSObject +@property (retain) NSObject *ivar1; +@property (retain) NSObject *ivar2; +@property (retain) NSObject *ivar3; +@property (retain) NSObject *ivar4; +@property (retain) NSObject *ivar5; +@property (retain) NSObject *ivar6; +@end + +@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 +} +@end + // Don't warn when value escapes. @interface ClassWhereIvarValueEscapes : NSObject diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h b/clang/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h new file mode 100644 index 000000000000..0b9888a076af --- /dev/null +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-objc-dealloc.h @@ -0,0 +1,29 @@ +#pragma clang system_header + +#define nil ((id)0) + +typedef signed char BOOL; +@protocol NSObject +- (BOOL)isEqual:(id)object; +- (Class)class; +@end + +@interface NSObject {} ++ (instancetype)alloc; +- (void)dealloc; +- (id)init; +- (id)retain; +- (oneway void)release; +@end + +@interface NSRunLoop : NSObject ++ (NSRunLoop *)currentRunLoop; +- (void)cancelPerformSelectorsWithTarget:(id)target; +@end + +@interface NSNotificationCenter : NSObject ++ (NSNotificationCenter *)defaultCenter; +- (void)removeObserver:(id)observer; +@end + +typedef struct objc_selector *SEL;