r261935 - [analyzer] Warn on use of 'self' after call to to [super dealloc].
Devin Coughlin via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 25 15:36:52 PST 2016
Author: dcoughlin
Date: Thu Feb 25 17:36:52 2016
New Revision: 261935
URL: http://llvm.org/viewvc/llvm-project?rev=261935&view=rev
Log:
[analyzer] Warn on use of 'self' after call to to [super dealloc].
Referring to 'self' after a call to [super dealloc] is a use-after-free in
Objective-C because NSObject's -dealloc frees the memory pointed to by self.
This patch extends the ObjCSuperDeallocChecker to catch this error.
rdar://problem/6953275
Differential Revision: http://reviews.llvm.org/D17528
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp?rev=261935&r1=261934&r2=261935&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp Thu Feb 25 17:36:52 2016
@@ -8,7 +8,7 @@
//===----------------------------------------------------------------------===//
//
// This defines ObjCSuperDeallocChecker, a builtin check that warns when
-// [super dealloc] is called twice on the same instance in MRR mode.
+// self is used after a call to [super dealloc] in MRR mode.
//
//===----------------------------------------------------------------------===//
@@ -25,7 +25,8 @@ using namespace ento;
namespace {
class ObjCSuperDeallocChecker
- : public Checker<check::PostObjCMessage, check::PreObjCMessage> {
+ : public Checker<check::PostObjCMessage, check::PreObjCMessage,
+ check::PreCall, check::Location> {
mutable IdentifierInfo *IIdealloc, *IINSObject;
mutable Selector SELdealloc;
@@ -40,12 +41,24 @@ public:
ObjCSuperDeallocChecker();
void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
+
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+
+ void checkLocation(SVal l, bool isLoad, const Stmt *S,
+ CheckerContext &C) const;
+
+private:
+
+ void diagnoseCallArguments(const CallEvent &CE, CheckerContext &C) const;
+
+ void reportUseAfterDealloc(SymbolRef Sym, StringRef Desc, const Stmt *S,
+ CheckerContext &C) const;
};
} // End anonymous namespace.
// Remember whether [super dealloc] has previously been called on the
-// a SymbolRef for the receiver.
+// SymbolRef for the receiver.
REGISTER_SET_WITH_PROGRAMSTATE(CalledSuperDealloc, SymbolRef)
class SuperDeallocBRVisitor final
@@ -71,40 +84,36 @@ public:
void ObjCSuperDeallocChecker::checkPreObjCMessage(const ObjCMethodCall &M,
CheckerContext &C) const {
- if (!isSuperDeallocMessage(M))
- return;
ProgramStateRef State = C.getState();
SymbolRef ReceiverSymbol = M.getReceiverSVal().getAsSymbol();
- assert(ReceiverSymbol && "No receiver symbol at call to [super dealloc]?");
+ if (!ReceiverSymbol) {
+ diagnoseCallArguments(M, C);
+ return;
+ }
bool AlreadyCalled = State->contains<CalledSuperDealloc>(ReceiverSymbol);
-
- // If [super dealloc] has not been called, there is nothing to do. We'll
- // note the fact that [super dealloc] was called in checkPostObjCMessage.
if (!AlreadyCalled)
return;
- // We have a duplicate [super dealloc] method call.
- // This likely causes a crash, so stop exploring the
- // path by generating a sink.
- ExplodedNode *ErrNode = C.generateErrorNode();
- // If we've already reached this node on another path, return.
- if (!ErrNode)
- return;
+ StringRef Desc;
- // Generate the report.
- std::unique_ptr<BugReport> BR(
- new BugReport(*DoubleSuperDeallocBugType,
- "[super dealloc] should not be called multiple times",
- ErrNode));
- BR->addRange(M.getOriginExpr()->getSourceRange());
- BR->addVisitor(llvm::make_unique<SuperDeallocBRVisitor>(ReceiverSymbol));
- C.emitReport(std::move(BR));
+ if (isSuperDeallocMessage(M)) {
+ Desc = "[super dealloc] should not be called multiple times";
+ } else {
+ Desc = StringRef();
+ }
+
+ reportUseAfterDealloc(ReceiverSymbol, Desc, M.getOriginExpr(), C);
return;
}
+void ObjCSuperDeallocChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ diagnoseCallArguments(Call, C);
+}
+
void ObjCSuperDeallocChecker::checkPostObjCMessage(const ObjCMethodCall &M,
CheckerContext &C) const {
// Check for [super dealloc] method call.
@@ -122,6 +131,92 @@ void ObjCSuperDeallocChecker::checkPostO
C.addTransition(State);
}
+void ObjCSuperDeallocChecker::checkLocation(SVal L, bool IsLoad, const Stmt *S,
+ CheckerContext &C) const {
+ SymbolRef BaseSym = L.getLocSymbolInBase();
+ if (!BaseSym)
+ return;
+
+ ProgramStateRef State = C.getState();
+
+ if (!State->contains<CalledSuperDealloc>(BaseSym))
+ return;
+
+ const MemRegion *R = L.getAsRegion();
+ if (!R)
+ return;
+
+ // Climb the super regions to find the base symbol while recording
+ // the second-to-last region for error reporting.
+ const MemRegion *PriorSubRegion = nullptr;
+ while (const SubRegion *SR = dyn_cast<SubRegion>(R)) {
+ if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(SR)) {
+ BaseSym = SymR->getSymbol();
+ break;
+ } else {
+ R = SR->getSuperRegion();
+ PriorSubRegion = SR;
+ }
+ }
+
+ StringRef Desc = StringRef();
+ auto *IvarRegion = dyn_cast_or_null<ObjCIvarRegion>(PriorSubRegion);
+
+ if (IvarRegion) {
+ std::string Buf;
+ llvm::raw_string_ostream OS(Buf);
+ OS << "use of instance variable '" << *IvarRegion->getDecl() <<
+ "' after the instance has been freed with call to [super dealloc]";
+ Desc = OS.str();
+ }
+
+ reportUseAfterDealloc(BaseSym, Desc, S, C);
+}
+
+/// Report a use-after-dealloc on \param Sym. If not empty,
+/// \param Desc will be used to describe the error; otherwise,
+/// a default warning will be used.
+void ObjCSuperDeallocChecker::reportUseAfterDealloc(SymbolRef Sym,
+ StringRef Desc,
+ const Stmt *S,
+ CheckerContext &C) const {
+ // We have a use of self after free.
+ // This likely causes a crash, so stop exploring the
+ // path by generating a sink.
+ ExplodedNode *ErrNode = C.generateErrorNode();
+ // If we've already reached this node on another path, return.
+ if (!ErrNode)
+ return;
+
+ if (Desc.empty())
+ Desc = "use of 'self' after it has been freed with call to [super dealloc]";
+
+ // Generate the report.
+ std::unique_ptr<BugReport> BR(
+ new BugReport(*DoubleSuperDeallocBugType, Desc, ErrNode));
+ BR->addRange(S->getSourceRange());
+ BR->addVisitor(llvm::make_unique<SuperDeallocBRVisitor>(Sym));
+ C.emitReport(std::move(BR));
+}
+
+/// Diagnose if any of the arguments to \param CE have already been
+/// dealloc'd.
+void ObjCSuperDeallocChecker::diagnoseCallArguments(const CallEvent &CE,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ unsigned ArgCount = CE.getNumArgs();
+ for (unsigned I = 0; I < ArgCount; I++) {
+ SymbolRef Sym = CE.getArgSVal(I).getAsSymbol();
+ if (!Sym)
+ continue;
+
+ if (State->contains<CalledSuperDealloc>(Sym)) {
+ reportUseAfterDealloc(Sym, StringRef(), CE.getArgExpr(I), C);
+ return;
+ }
+ }
+}
+
ObjCSuperDeallocChecker::ObjCSuperDeallocChecker()
: IIdealloc(nullptr), IINSObject(nullptr) {
Modified: cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m?rev=261935&r1=261934&r2=261935&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m (original)
+++ cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m Thu Feb 25 17:36:52 2016
@@ -35,8 +35,9 @@ typedef struct objc_selector *SEL;
return self;
}
- (void)dealloc {
- [super dealloc];
- [_ivar release];
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ [_ivar release]; // expected-warning {{use of instance variable '_ivar' after the instance has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of instance variable '_ivar' after the instance has been freed with call to [super dealloc]}}
}
@end
@@ -54,8 +55,46 @@ typedef struct objc_selector *SEL;
return self;
}
- (void)dealloc {
- [super dealloc];
- _delegate = nil;
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ _delegate = nil; // expected-warning {{use of instance variable '_delegate' after the instance has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of instance variable '_delegate' after the instance has been freed with call to [super dealloc]}}
+}
+ at end
+
+
+struct SomeStruct {
+ int f;
+};
+
+ at interface SuperDeallocThenAssignIvarField : NSObject {
+ struct SomeStruct _s;
+}
+ at end
+
+ at implementation SuperDeallocThenAssignIvarField
+- (void)dealloc {
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ _s.f = 7; // expected-warning {{use of instance variable '_s' after the instance has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of instance variable '_s' after the instance has been freed with call to [super dealloc]}}
+}
+ at end
+
+ at interface OtherClassWithIvar {
+ at public
+ int _otherIvar;
+}
+ at end;
+
+ at interface SuperDeallocThenAssignIvarIvar : NSObject {
+ OtherClassWithIvar *_ivar;
+}
+ at end
+
+ at implementation SuperDeallocThenAssignIvarIvar
+- (void)dealloc {
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ _ivar->_otherIvar = 7; // expected-warning {{use of instance variable '_ivar' after the instance has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of instance variable '_ivar' after the instance has been freed with call to [super dealloc]}}
}
@end
@@ -72,8 +111,9 @@ typedef struct objc_selector *SEL;
return self;
}
- (void)dealloc {
- [super dealloc];
- self.ivar = nil;
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ self.ivar = nil; // expected-warning {{use of 'self' after it has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of 'self' after it has been freed with call to [super dealloc]}}
}
@end
@@ -90,8 +130,9 @@ typedef struct objc_selector *SEL;
return self;
}
- (void)dealloc {
- [super dealloc];
- self.delegate = nil;
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ self.delegate = nil; // expected-warning {{use of 'self' after it has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of 'self' after it has been freed with call to [super dealloc]}}
}
@end
@@ -103,8 +144,9 @@ typedef struct objc_selector *SEL;
- (void)_invalidate {
}
- (void)dealloc {
- [super dealloc];
- [self _invalidate];
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ [self _invalidate]; // expected-warning {{use of 'self' after it has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of 'self' after it has been freed with call to [super dealloc]}}
}
@end
@@ -117,8 +159,23 @@ static void _invalidate(NSObject *object
@implementation SuperDeallocThenCallNonObjectiveCMethodClass
- (void)dealloc {
- [super dealloc];
- _invalidate(self);
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ _invalidate(self); // expected-warning {{use of 'self' after it has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of 'self' after it has been freed with call to [super dealloc]}}
+}
+ at end
+
+ at interface SuperDeallocThenCallObjectiveClassMethodClass : NSObject { }
+ at end
+
+ at implementation SuperDeallocThenCallObjectiveClassMethodClass
++ (void) invalidate:(id)arg; {
+}
+
+- (void)dealloc {
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ [SuperDeallocThenCallObjectiveClassMethodClass invalidate:self]; // expected-warning {{use of 'self' after it has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of 'self' after it has been freed with call to [super dealloc]}}
}
@end
@@ -132,13 +189,14 @@ static void _invalidate(NSObject *object
- (void)_invalidate {
}
- (void)dealloc {
- if (_ivar) {
+ if (_ivar) { // expected-note {{Taking false branch}}
[_ivar release];
[super dealloc];
return;
}
- [super dealloc];
- [self _invalidate];
+ [super dealloc]; // expected-note {{[super dealloc] called here}}
+ [self _invalidate]; // expected-warning {{use of 'self' after it has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of 'self' after it has been freed with call to [super dealloc]}}
}
@end
@@ -244,11 +302,15 @@ static void _invalidate(NSObject *object
// Do not warn about calling [super dealloc] twice if when the analyzer has
// inlined the call to its super deallocator.
- at interface SuperClassCallingSuperDealloc : NSObject
+ at interface SuperClassCallingSuperDealloc : NSObject {
+ NSObject *_ivar;
+}
@end
@implementation SuperClassCallingSuperDealloc
- (void)dealloc; {
+ [_ivar release]; // no-warning
+
[super dealloc];
}
@end
@@ -291,8 +353,8 @@ static void _invalidate(NSObject *object
- (void)dealloc; {
[super dealloc]; // expected-note {{[super dealloc] called here}}
- [self anotherMethod];
- [super dealloc]; // expected-warning {{[super dealloc] should not be called multiple times}}
- // expected-note at -1 {{[super dealloc] should not be called multiple times}}
+ [self anotherMethod]; // expected-warning {{use of 'self' after it has been freed with call to [super dealloc]}}
+ // expected-note at -1 {{use of 'self' after it has been freed with call to [super dealloc]}}
+ [super dealloc];
}
@end
More information about the cfe-commits
mailing list