Hi jordan_rose, zaks.anna, krememek,
Implement a path checker that warns about duplicate calls to
[super dealloc]. This is the first step before re-implementing
the use-after-free checks using a path checker (see D5042).
FIXME: This path checker needs to discriminate between different
'self' symbols when recursing into the -dealloc method in the
same class (for different objects). See test case.
http://reviews.llvm.org/D5238
Files:
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
test/Analysis/DeallocUseAfterFreeErrors.m
Index: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td
+++ lib/StaticAnalyzer/Checkers/Checkers.td
@@ -440,6 +440,11 @@
HelpText<"Warn about Objective-C classes that lack a correct implementation of -dealloc">,
DescFile<"CheckObjCDealloc.cpp">;
+def ObjCSuperDeallocChecker : Checker<"SuperDealloc">,
+ HelpText<"Warn about use of 'self' after '[super dealloc]' in Objective-C "
+ "-dealloc methods">,
+ DescFile<"ObjCSuperDeallocChecker.cpp">;
+
def InstanceVariableInvalidation : Checker<"InstanceVariableInvalidation">,
HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">,
DescFile<"IvarInvalidationChecker.cpp">;
Index: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
===================================================================
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
@@ -0,0 +1,246 @@
+//===- ObjCSuperDeallocChecker.cpp - Check use of [super dealloc] message -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This defines ObjCSuperDeallocChecker, a builtin check that checks for the
+// correct use of the [super dealloc] message in -dealloc in MRR mode.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class SuperDeallocState {
+
+ SymbolRef SuperDeallocSymbol;
+ unsigned BlockID;
+ const StackFrameContext *SFC;
+
+public:
+ SuperDeallocState(SymbolRef S, unsigned B, const StackFrameContext *SFC)
+ : SuperDeallocSymbol(S), BlockID(B), SFC(SFC) {}
+
+ SymbolRef getSuperDeallocSymbol() const { return SuperDeallocSymbol; }
+ const StackFrameContext *getStackFrameContext() const { return SFC; }
+
+ bool operator==(const SuperDeallocState &X) const {
+ return BlockID == X.BlockID && SFC == X.SFC &&
+ SuperDeallocSymbol == X.SuperDeallocSymbol;
+ }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddPointer(SuperDeallocSymbol);
+ ID.AddInteger(BlockID);
+ ID.AddPointer(SFC);
+ }
+};
+
+class SuperDeallocBRVisitor
+ : public BugReporterVisitorImpl<SuperDeallocBRVisitor> {
+
+ SymbolRef SuperDeallocSymbol;
+ const StackFrameContext *SFC;
+ bool Satisfied;
+
+public:
+ SuperDeallocBRVisitor(SymbolRef SuperDeallocSymbol,
+ const StackFrameContext *SFC)
+ : SuperDeallocSymbol(SuperDeallocSymbol), SFC(SFC), Satisfied(false) {}
+
+ PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.Add(SuperDeallocSymbol);
+ ID.Add(SFC);
+ }
+};
+
+class ObjCSuperDeallocChecker
+ : public Checker<check::PostObjCMessage, check::PreObjCMessage> {
+
+ mutable IdentifierInfo *IIdealloc, *IINSObject;
+ mutable Selector SELdealloc;
+
+ std::unique_ptr<BugType> DoubleSuperDeallocBugType;
+
+ void initIdentifierInfoAndSelectors(ASTContext &Ctx) const;
+
+ bool shouldRunOnFunctionOrMethod(const NamedDecl *ND) const;
+
+public:
+ ObjCSuperDeallocChecker();
+ void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
+ void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
+};
+
+} // end anonymous namespace
+
+// Remember if we called [super dealloc] previously.
+REGISTER_LIST_WITH_PROGRAMSTATE(CalledSuperDealloc, SuperDeallocState)
+
+PathDiagnosticPiece *SuperDeallocBRVisitor::VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC,
+ BugReport &BR) {
+ if (Satisfied)
+ return nullptr;
+
+ const Expr *E = nullptr;
+
+ if (Optional<PostStmt> P = Succ->getLocationAs<PostStmt>())
+ E = P->getStmtAs<ObjCMessageExpr>();
+
+ if (!E)
+ return nullptr;
+
+ ProgramStateRef State = Succ->getState();
+ SVal S = State->getSVal(E, Succ->getLocationContext());
+ if (SuperDeallocSymbol == S.getAsSymbol() && SFC == Succ->getStackFrame()) {
+ Satisfied = true;
+
+ // Construct a new PathDiagnosticPiece.
+ ProgramPoint P = Succ->getLocation();
+ PathDiagnosticLocation L =
+ PathDiagnosticLocation::create(P, BRC.getSourceManager());
+
+ if (!L.isValid() || !L.asLocation().isValid())
+ return nullptr;
+
+ return new PathDiagnosticEventPiece(
+ L, "[super dealloc] originally called here");
+ }
+
+ return nullptr;
+}
+
+ObjCSuperDeallocChecker::ObjCSuperDeallocChecker()
+ : IIdealloc(nullptr), IINSObject(nullptr) {
+
+ DoubleSuperDeallocBugType.reset(
+ new BugType(this, "[super dealloc] should never be called twice",
+ categories::CoreFoundationObjectiveC));
+}
+
+void ObjCSuperDeallocChecker::checkPostObjCMessage(const ObjCMethodCall &M,
+ CheckerContext &C) const {
+ initIdentifierInfoAndSelectors(C.getASTContext());
+
+ // FIXME: A callback should disable checkers at the start of functions.
+ if (!shouldRunOnFunctionOrMethod(
+ dyn_cast<NamedDecl>(C.getCurrentAnalysisDeclContext()->getDecl())))
+ return;
+
+ // Check for [super dealloc] method call.
+ if (M.getOriginExpr()->getReceiverKind() == ObjCMessageExpr::SuperInstance &&
+ M.getSelector() == SELdealloc) {
+ ProgramStateRef State = C.getState();
+ SVal V = State->getSVal(M.getOriginExpr(), C.getLocationContext());
+ State = State->add<CalledSuperDealloc>(
+ SuperDeallocState(V.getAsSymbol(), C.getBlockID(), C.getStackFrame()));
+ C.addTransition(State);
+ }
+}
+
+void ObjCSuperDeallocChecker::checkPreObjCMessage(const ObjCMethodCall &M,
+ CheckerContext &C) const {
+ initIdentifierInfoAndSelectors(C.getASTContext());
+
+ // If [super dealloc] has not been called, there is nothing to do.
+ ProgramStateRef State = C.getState();
+ CalledSuperDeallocTy SDStateList = State->get<CalledSuperDealloc>();
+ if (SDStateList.isEmpty())
+ return;
+
+ assert(++SDStateList.begin() == SDStateList.end() &&
+ "There should only ever be one SuperDeallocState in the list "
+ "because a second one causes a sink node to be created.");
+
+ // Check for a duplicate [super dealloc] method call.
+ if (M.getOriginExpr()->getReceiverKind() == ObjCMessageExpr::SuperInstance &&
+ M.getSelector() == SELdealloc) {
+
+ // We have reached a bug that likely causes a crash, so stop exploring the
+ // path by generating a sink.
+ ExplodedNode *ErrNode = C.generateSink();
+ // If we've already reached this node on another path, return.
+ if (!ErrNode)
+ return;
+
+ // Generate the report.
+ std::unique_ptr<BugReport> R(
+ new BugReport(*DoubleSuperDeallocBugType,
+ "[super dealloc] called a second time", ErrNode));
+ R->addRange(M.getOriginExpr()->getSourceRange());
+ const SuperDeallocState &SDState = SDStateList.getHead();
+ R->addVisitor(new SuperDeallocBRVisitor(SDState.getSuperDeallocSymbol(),
+ SDState.getStackFrameContext()));
+ C.emitReport(R.release());
+
+ return;
+ }
+}
+
+void
+ObjCSuperDeallocChecker::initIdentifierInfoAndSelectors(ASTContext &Ctx) const {
+
+ if (IIdealloc)
+ return;
+
+ IIdealloc = &Ctx.Idents.get("dealloc");
+ IINSObject = &Ctx.Idents.get("NSObject");
+
+ SELdealloc = Ctx.Selectors.getSelector(0, &IIdealloc);
+}
+
+// FIXME: A callback should disable checkers at the start of functions.
+bool ObjCSuperDeallocChecker::shouldRunOnFunctionOrMethod(
+ const NamedDecl *ND) const {
+
+ if (!ND)
+ return false;
+
+ const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(ND);
+ if (!MD)
+ return false;
+
+ // [super dealloc] only applies to NSObject subclasses.
+ for (ObjCInterfaceDecl *ID = MD->getClassInterface()->getSuperClass(); ID;
+ ID = ID->getSuperClass()) {
+
+ IdentifierInfo *II = ID->getIdentifier();
+
+ if (II == IINSObject)
+ return true;
+ }
+
+ return false;
+}
+
+//===----------------------------------------------------------------------===//
+// Checker Registration.
+//===----------------------------------------------------------------------===//
+
+void ento::registerObjCSuperDeallocChecker(CheckerManager &Mgr) {
+ const LangOptions &LangOpts = Mgr.getLangOpts();
+ if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount)
+ return;
+ Mgr.registerChecker<ObjCSuperDeallocChecker>();
+}
Index: test/Analysis/DeallocUseAfterFreeErrors.m
===================================================================
--- /dev/null
+++ test/Analysis/DeallocUseAfterFreeErrors.m
@@ -0,0 +1,203 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.SuperDealloc -verify %s
+
+#define nil ((id)0)
+
+typedef unsigned long NSUInteger;
+@protocol NSObject
+- (instancetype)retain;
+- (oneway void)release;
+@end
+
+@interface NSObject <NSObject> { }
+- (void)dealloc;
+- (instancetype)init;
+@end
+
+typedef struct objc_selector *SEL;
+
+//===------------------------------------------------------------------------===
+// <rdar://problem/6953275>
+// Check that 'self' is not referenced after calling '[super dealloc]'.
+
+@interface SuperDeallocThenReleaseIvarClass : NSObject {
+ NSObject *_ivar;
+}
+@end
+
+@implementation SuperDeallocThenReleaseIvarClass
+- (instancetype)initWithIvar:(NSObject *)ivar {
+ self = [super init];
+ if (!self)
+ return nil;
+ _ivar = [ivar retain];
+ return self;
+}
+- (void)dealloc {
+ [super dealloc];
+ [_ivar release];
+}
+@end
+
+@interface SuperDeallocThenAssignNilToIvarClass : NSObject {
+ NSObject *_delegate;
+}
+@end
+
+@implementation SuperDeallocThenAssignNilToIvarClass
+- (instancetype)initWithDelegate:(NSObject *)delegate {
+ self = [super init];
+ if (!self)
+ return nil;
+ _delegate = delegate;
+ return self;
+}
+- (void)dealloc {
+ [super dealloc];
+ _delegate = nil;
+}
+@end
+
+@interface SuperDeallocThenReleasePropertyClass : NSObject { }
+@property (retain) NSObject *ivar;
+@end
+
+@implementation SuperDeallocThenReleasePropertyClass
+- (instancetype)initWithProperty:(NSObject *)ivar {
+ self = [super init];
+ if (!self)
+ return nil;
+ self.ivar = ivar;
+ return self;
+}
+- (void)dealloc {
+ [super dealloc];
+ self.ivar = nil;
+}
+@end
+
+@interface SuperDeallocThenAssignNilToPropertyClass : NSObject { }
+@property (assign) NSObject *delegate;
+@end
+
+@implementation SuperDeallocThenAssignNilToPropertyClass
+- (instancetype)initWithDelegate:(NSObject *)delegate {
+ self = [super init];
+ if (!self)
+ return nil;
+ self.delegate = delegate;
+ return self;
+}
+- (void)dealloc {
+ [super dealloc];
+ self.delegate = nil;
+}
+@end
+
+@interface SuperDeallocThenCallInstanceMethodClass : NSObject { }
+- (void)_invalidate;
+@end
+
+@implementation SuperDeallocThenCallInstanceMethodClass
+- (void)_invalidate {
+}
+- (void)dealloc {
+ [super dealloc];
+ [self _invalidate];
+}
+@end
+
+@interface SuperDeallocThenCallNonObjectiveCMethodClass : NSObject { }
+@end
+
+static void _invalidate(NSObject *object) {
+ (void)object;
+}
+
+@implementation SuperDeallocThenCallNonObjectiveCMethodClass
+- (void)dealloc {
+ [super dealloc];
+ _invalidate(self);
+}
+@end
+
+@interface TwoSuperDeallocCallsClass : NSObject {
+ NSObject *_ivar;
+}
+- (void)_invalidate;
+@end
+
+@implementation TwoSuperDeallocCallsClass
+- (void)_invalidate {
+}
+- (void)dealloc {
+ if (_ivar) {
+ [_ivar release];
+ [super dealloc];
+ return;
+ }
+ [super dealloc];
+ [self _invalidate];
+}
+@end
+
+@interface MissingReturnCausesDoubleSuperDeallocClass : NSObject {
+ NSObject *_ivar;
+}
+@end
+
+@implementation MissingReturnCausesDoubleSuperDeallocClass
+- (void)dealloc {
+ if (_ivar) {
+ [_ivar release];
+ [super dealloc];
+ //return;
+ }
+ [super dealloc]; // expected-warning{{[super dealloc] called a second time}}
+}
+@end
+
+@interface SuperDeallocInOtherMethodClass : NSObject {
+ NSObject *_ivar;
+}
+- (void)_cleanup;
+@end
+
+@implementation SuperDeallocInOtherMethodClass
+- (void)_cleanup {
+ [_ivar release];
+ [super dealloc];
+}
+- (void)dealloc {
+ [self _cleanup];
+ [super dealloc]; // expected-warning{{[super dealloc] called a second time}}
+}
+@end
+
+// A class that contains an ivar of itself can generate a false positive that
+// [super dealloc] is called twice if each object instance is not tracked
+// separately by the checker, and if custom reference counting is implemented,
+// as happens when _OBJC_SUPPORTED_INLINE_REFCNT_WITH_DEALLOC2MAIN is used.
+// This is just a simple approximation to trigger the bug.
+@class ClassWithOwnIvarInstanceClass;
+@interface ClassWithOwnIvarInstanceClass : NSObject {
+ ClassWithOwnIvarInstanceClass *_ivar;
+ NSUInteger _retainCount;
+}
+@end
+
+@implementation ClassWithOwnIvarInstanceClass
+- (instancetype)retain {
+ ++_retainCount;
+ return self;
+}
+- (oneway void)release {
+ --_retainCount;
+ if (!_retainCount)
+ [self dealloc];
+}
+- (void)dealloc {
+ [_ivar release];
+ [super dealloc]; // expected-warning{{[super dealloc] called a second time}}
+ // FIXME: This is a false-positive since the checker does not track 'self' values for different objects separately.
+}
+@end
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
