diff options
| -rw-r--r-- | lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp | 124 | ||||
| -rw-r--r-- | test/Analysis/DeallocMissingRelease.m | 21 |
2 files changed, 113 insertions, 32 deletions
diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index f9fd9fcf95..84856bd6dc 100644 --- a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -103,6 +103,7 @@ class ObjCDeallocChecker std::unique_ptr<BugType> MissingReleaseBugType; std::unique_ptr<BugType> ExtraReleaseBugType; + std::unique_ptr<BugType> MistakenDeallocBugType; public: ObjCDeallocChecker(); @@ -130,14 +131,20 @@ private: bool diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall &M, CheckerContext &C) const; - SymbolRef getValueExplicitlyReleased(const ObjCMethodCall &M, - CheckerContext &C) const; + bool diagnoseMistakenDealloc(SymbolRef DeallocedValue, + const ObjCMethodCall &M, + CheckerContext &C) const; + SymbolRef getValueReleasedByNillingOut(const ObjCMethodCall &M, CheckerContext &C) const; const ObjCIvarRegion *getIvarRegionForIvarSymbol(SymbolRef IvarSym) const; SymbolRef getInstanceSymbolFromIvarSymbol(SymbolRef IvarSym) const; + const ObjCPropertyImplDecl* + findPropertyOnDeallocatingInstance(SymbolRef IvarSym, + CheckerContext &C) const; + ReleaseRequirement getDeallocReleaseRequirement(const ObjCPropertyImplDecl *PropImpl) const; @@ -336,7 +343,14 @@ void ObjCDeallocChecker::checkPreObjCMessage( if (!instanceDeallocIsOnStack(C, DeallocedInstance)) return; - SymbolRef ReleasedValue = getValueExplicitlyReleased(M, C); + SymbolRef ReleasedValue = nullptr; + + if (M.getSelector() == ReleaseSel) { + ReleasedValue = M.getReceiverSVal().getAsSymbol(); + } else if (M.getSelector() == DeallocSel && !M.isReceiverSelfOrSuper()) { + if (diagnoseMistakenDealloc(M.getReceiverSVal().getAsSymbol(), M, C)) + return; + } if (ReleasedValue) { // An instance variable symbol was released with -release: @@ -597,40 +611,55 @@ void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const { assert(!LCtx->inTopFrame() || State->get<UnreleasedIvarMap>().isEmpty()); } +/// Given a symbol, determine whether the symbol refers to an ivar on +/// the top-most deallocating instance. If so, find the property for that +/// ivar, if one exists. Otherwise return null. +const ObjCPropertyImplDecl * +ObjCDeallocChecker::findPropertyOnDeallocatingInstance( + SymbolRef IvarSym, CheckerContext &C) const { + SVal DeallocedInstance; + if (!isInInstanceDealloc(C, DeallocedInstance)) + return nullptr; + + // Try to get the region from which the ivar value was loaded. + auto *IvarRegion = getIvarRegionForIvarSymbol(IvarSym); + if (!IvarRegion) + return nullptr; + + // Don't try to find the property if the ivar was not loaded from the + // given instance. + if (DeallocedInstance.castAs<loc::MemRegionVal>().getRegion() != + IvarRegion->getSuperRegion()) + return nullptr; + + const LocationContext *LCtx = C.getLocationContext(); + const ObjCIvarDecl *IvarDecl = IvarRegion->getDecl(); + + const ObjCImplDecl *Container = getContainingObjCImpl(LCtx); + const ObjCPropertyImplDecl *PropImpl = + Container->FindPropertyImplIvarDecl(IvarDecl->getIdentifier()); + return PropImpl; +} + /// Emits a warning if the current context is -dealloc and ReleasedValue /// must not be directly released in a -dealloc. Returns true if a diagnostic /// was emitted. bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall &M, CheckerContext &C) const { - SVal DeallocedInstance; - if (!isInInstanceDealloc(C, DeallocedInstance)) - return false; - // Try to get the region from which the the released value was loaded. // Note that, unlike diagnosing for missing releases, here we don't track // 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. - auto *ReleasedIvar = getIvarRegionForIvarSymbol(ReleasedValue); - if (!ReleasedIvar) - return false; + const ObjCPropertyImplDecl *PropImpl = + findPropertyOnDeallocatingInstance(ReleasedValue, C); - if (DeallocedInstance.castAs<loc::MemRegionVal>().getRegion() != - ReleasedIvar->getSuperRegion()) + if (!PropImpl) return false; - const LocationContext *LCtx = C.getLocationContext(); - const ObjCIvarDecl *ReleasedIvarDecl = ReleasedIvar->getDecl(); - // If the ivar belongs to a property that must not be released directly // in dealloc, emit a warning. - const ObjCImplDecl *Container = getContainingObjCImpl(LCtx); - const ObjCPropertyImplDecl *PropImpl = - Container->FindPropertyImplIvarDecl(ReleasedIvarDecl->getIdentifier()); - if (!PropImpl) - return false; - if (getDeallocReleaseRequirement(PropImpl) != ReleaseRequirement::MustNotReleaseDirectly) { return false; @@ -661,6 +690,7 @@ bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue, (PropDecl->getSetterKind() == ObjCPropertyDecl::Assign && !PropDecl->isReadOnly())); + const ObjCImplDecl *Container = getContainingObjCImpl(C.getLocationContext()); OS << "The '" << *PropImpl->getPropertyIvarDecl() << "' ivar in '" << *Container << "' was synthesized for "; @@ -681,6 +711,43 @@ bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue, return true; } +/// Emits a warning if the current context is -dealloc and DeallocedValue +/// must not be directly dealloced in a -dealloc. Returns true if a diagnostic +/// was emitted. +bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue, + const ObjCMethodCall &M, + CheckerContext &C) const { + + // Find the property backing the instance variable that M + // is dealloc'ing. + const ObjCPropertyImplDecl *PropImpl = + findPropertyOnDeallocatingInstance(DeallocedValue, C); + if (!PropImpl) + return false; + + if (getDeallocReleaseRequirement(PropImpl) != + ReleaseRequirement::MustRelease) { + return false; + } + + ExplodedNode *ErrNode = C.generateErrorNode(); + if (!ErrNode) + return false; + + std::string Buf; + llvm::raw_string_ostream OS(Buf); + + OS << "'" << *PropImpl->getPropertyIvarDecl() + << "' should be released rather than deallocated"; + + std::unique_ptr<BugReport> BR( + new BugReport(*MistakenDeallocBugType, OS.str(), ErrNode)); + BR->addRange(M.getOriginExpr()->getSourceRange()); + + C.emitReport(std::move(BR)); + + return true; +} ObjCDeallocChecker:: ObjCDeallocChecker() @@ -693,6 +760,10 @@ ObjCDeallocChecker:: ExtraReleaseBugType.reset( new BugType(this, "Extra ivar release", categories::MemoryCoreFoundationObjectiveC)); + + MistakenDeallocBugType.reset( + new BugType(this, "Mistaken dealloc", + categories::MemoryCoreFoundationObjectiveC)); } void ObjCDeallocChecker::initIdentifierInfoAndSelectors( @@ -840,17 +911,6 @@ ReleaseRequirement ObjCDeallocChecker::getDeallocReleaseRequirement( llvm_unreachable("Unrecognized setter kind"); } -/// Returns the released value if M is a call to -release. Returns -/// nullptr otherwise. -SymbolRef -ObjCDeallocChecker::getValueExplicitlyReleased(const ObjCMethodCall &M, - CheckerContext &C) const { - if (M.getSelector() != ReleaseSel) - return nullptr; - - return M.getReceiverSVal().getAsSymbol(); -} - /// Returns the released value if M is a call a setter that releases /// and nils out its underlying instance variable. SymbolRef diff --git a/test/Analysis/DeallocMissingRelease.m b/test/Analysis/DeallocMissingRelease.m index 75afd0e5f1..26f32db7ff 100644 --- a/test/Analysis/DeallocMissingRelease.m +++ b/test/Analysis/DeallocMissingRelease.m @@ -733,3 +733,24 @@ __attribute__((objc_root_class)) } @end +// Warn about calling -dealloc rather than release by mistake. + +@interface CallDeallocOnRetainPropIvar : NSObject { + NSObject *okToDeallocDirectly; +} + +@property (retain) NSObject *ivar; +@end + +@implementation CallDeallocOnRetainPropIvar +- (void)dealloc +{ +#if NON_ARC + // Only warn for synthesized ivars. + [okToDeallocDirectly dealloc]; // now-warning + [_ivar dealloc]; // expected-warning {{'_ivar' should be released rather than deallocated}} + + [super dealloc]; +#endif +} +@end |