- Updated help text in `Checkers.td` to be more generic.
- Fix false positive with classes using custom retain counting and containing
an ivar that's the same type as the class. Added test case for this condition.
- Extracted duplicate code into isSuperDeallocMessage().
- Small tweaks to variable names.
- Add section headers for [super dealloc] test cases to explain what each one
is testing.
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,10 @@
HelpText<"Warn about Objective-C classes that lack a correct implementation of -dealloc">,
DescFile<"CheckObjCDealloc.cpp">;
+def ObjCSuperDeallocChecker : Checker<"SuperDealloc">,
+ HelpText<"Warn about improper use of '[super dealloc]' in Objective-C">,
+ 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,259 @@
+//===- ObjCSuperDeallocChecker.cpp - Check correct use of [super dealloc] -===//
+//
+// 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 SuperDeallocSymbol == X.SuperDeallocSymbol && BlockID == X.BlockID &&
+ SFC == X.SFC;
+ }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddPointer(SuperDeallocSymbol);
+ ID.AddInteger(BlockID);
+ ID.AddPointer(SFC);
+ }
+};
+
+class SuperDeallocBRVisitor
+ : public BugReporterVisitorImpl<SuperDeallocBRVisitor> {
+
+ SymbolRef SelfSymbol;
+ SymbolRef SuperDeallocSymbol;
+ const StackFrameContext *SFC;
+ bool Satisfied;
+
+public:
+ SuperDeallocBRVisitor(SymbolRef SelfSymbol, const SuperDeallocState &SDState)
+ : SelfSymbol(SelfSymbol),
+ SuperDeallocSymbol(SDState.getSuperDeallocSymbol()),
+ SFC(SDState.getStackFrameContext()), Satisfied(false) {}
+
+ PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.Add(SelfSymbol);
+ 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 isSuperDeallocMessage(const ObjCMethodCall &M) 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 by key 'self' SymbolRef.
+REGISTER_MAP_WITH_PROGRAMSTATE(CalledSuperDealloc, SymbolRef, SuperDeallocState)
+
+PathDiagnosticPiece *SuperDeallocBRVisitor::VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC,
+ BugReport &BR) {
+ if (Satisfied)
+ return nullptr;
+
+ const ObjCMessageExpr *E = nullptr;
+ if (Optional<PostStmt> P = Succ->getLocationAs<PostStmt>())
+ E = P->getStmtAs<ObjCMessageExpr>();
+ if (!E)
+ return nullptr;
+
+ ProgramStateRef State = Succ->getState();
+ const LocationContext *LocCtx = Succ->getLocationContext();
+ CallEventManager &CEMgr = State->getStateManager().getCallEventManager();
+ CallEventRef<ObjCMethodCall> Call = CEMgr.getObjCMethodCall(E, State, LocCtx);
+ SVal SelfVal = Call->getSelfSVal();
+ SVal SuperDeallocVal = State->getSVal(E, LocCtx);
+
+ if (SelfSymbol == SelfVal.getAsSymbol() &&
+ SuperDeallocSymbol == SuperDeallocVal.getAsSymbol() &&
+ SFC == Succ->getStackFrame()) {
+
+ Satisfied = true;
+
+ 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 (isSuperDeallocMessage(M)) {
+ ProgramStateRef State = C.getState();
+ SymbolRef SelfSymbol = M.getSelfSVal().getAsSymbol();
+ SymbolRef MethodSymbol =
+ State->getSVal(M.getOriginExpr(), C.getLocationContext()).getAsSymbol();
+
+ State = State->set<CalledSuperDealloc>(
+ SelfSymbol,
+ SuperDeallocState(MethodSymbol, 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();
+ SymbolRef SelfSymbol = M.getSelfSVal().getAsSymbol();
+ const SuperDeallocState *SDState = State->get<CalledSuperDealloc>(SelfSymbol);
+ if (!SDState)
+ return;
+
+ // Check for a duplicate [super dealloc] method call.
+ if (isSuperDeallocMessage(M)) {
+ // 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> BR(
+ new BugReport(*DoubleSuperDeallocBugType,
+ "[super dealloc] called a second time", ErrNode));
+ BR->addRange(M.getOriginExpr()->getSourceRange());
+ BR->addVisitor(new SuperDeallocBRVisitor(SelfSymbol, *SDState));
+ C.emitReport(BR.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);
+}
+
+bool
+ObjCSuperDeallocChecker::isSuperDeallocMessage(const ObjCMethodCall &M) const {
+ return M.getOriginExpr()->getReceiverKind() ==
+ ObjCMessageExpr::SuperInstance &&
+ M.getSelector() == SELdealloc;
+}
+
+// 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,236 @@
+// 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
+
+//===------------------------------------------------------------------------===
+// Warn about calling [super dealloc] twice due to missing return statement.
+
+@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
+
+//===------------------------------------------------------------------------===
+// Warn about calling [super dealloc] twice in two different methods.
+
+@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
+
+//===------------------------------------------------------------------------===
+// Do not warn about calling [super dealloc] recursively for different objects
+// of the same type with custom retain counting.
+//
+// A class that contains an ivar of itself with custom retain counting (such
+// as provided by _OBJC_SUPPORTED_INLINE_REFCNT_WITH_DEALLOC2MAIN) can generate
+// a false positive that [super dealloc] is called twice if each object instance
+// is not tracked separately by the checker. This test case is just a simple
+// approximation to trigger the false positive.
+
+@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]; // no warning: different instances of same class
+}
+@end
+
+//===------------------------------------------------------------------------===
+// Do not warn about calling [super dealloc] twice if +dealloc is a class
+// method.
+
+@interface SuperDeallocClassMethodIgnoredClass : NSObject { }
++ (void)dealloc;
+@end
+
+@implementation SuperDeallocClassMethodIgnoredClass
++ (void)dealloc { }
+@end
+
+@interface SuperDeallocClassMethodIgnoredSubClass : NSObject { }
++ (void)dealloc;
+@end
+
+@implementation SuperDeallocClassMethodIgnoredSubClass
++ (void)dealloc {
+ [super dealloc];
+ [super dealloc]; // no warning: class method
+}
+@end
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
