r262729 - [analyzer] Add diagnostic in ObjCDeallocChecker for use of -dealloc instead of -release.
Devin Coughlin via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 4 10:09:58 PST 2016
Author: dcoughlin
Date: Fri Mar 4 12:09:58 2016
New Revision: 262729
URL: http://llvm.org/viewvc/llvm-project?rev=262729&view=rev
Log:
[analyzer] Add diagnostic in ObjCDeallocChecker for use of -dealloc instead of -release.
In dealloc methods, the analyzer now warns when -dealloc is called directly on
a synthesized retain/copy ivar instead of -release. This is intended to find mistakes of
the form:
- (void)dealloc {
[_ivar dealloc]; // Mistaken call to -dealloc instead of -release
[super dealloc];
}
rdar://problem/16227989
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=262729&r1=262728&r2=262729&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Fri Mar 4 12:09:58 2016
@@ -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::checkPreObjCMes
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::diagnoseMissing
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::diagnoseExtraRe
(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::diagnoseExtraRe
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::g
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
Modified: cfe/trunk/test/Analysis/DeallocMissingRelease.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocMissingRelease.m?rev=262729&r1=262728&r2=262729&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/DeallocMissingRelease.m (original)
+++ cfe/trunk/test/Analysis/DeallocMissingRelease.m Fri Mar 4 12:09:58 2016
@@ -733,3 +733,24 @@ __attribute__((objc_root_class))
}
@end
+// Warn about calling -dealloc rather than release by mistake.
+
+ at interface CallDeallocOnRetainPropIvar : NSObject {
+ NSObject *okToDeallocDirectly;
+}
+
+ at property (retain) NSObject *ivar;
+ at end
+
+ at 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
+}
+ at end
More information about the cfe-commits
mailing list