Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -56,6 +56,7 @@ ObjCContainersChecker.cpp ObjCMissingSuperCallChecker.cpp ObjCSelfInitChecker.cpp + ObjCSuperDeallocChecker.cpp ObjCUnusedIVarsChecker.cpp PaddingChecker.cpp PointerArithChecker.cpp Index: lib/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- lib/StaticAnalyzer/Checkers/Checkers.td +++ lib/StaticAnalyzer/Checkers/Checkers.td @@ -513,6 +513,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,194 @@ +//===- 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 SuperDeallocBRVisitor + : public BugReporterVisitorImpl { + + SymbolRef SelfSymbol; + bool Satisfied; + +public: + SuperDeallocBRVisitor(SymbolRef SelfSymbol) + : SelfSymbol(SelfSymbol), + 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); + } +}; + +class ObjCSuperDeallocChecker + : public Checker { + + mutable IdentifierInfo *IIdealloc, *IINSObject; + mutable Selector SELdealloc; + + std::unique_ptr DoubleSuperDeallocBugType; + + void initIdentifierInfoAndSelectors(ASTContext &Ctx) const; + + bool isSuperDeallocMessage(const ObjCMethodCall &M) 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_SET_WITH_PROGRAMSTATE(CalledSuperDealloc, SymbolRef) + +PathDiagnosticPiece *SuperDeallocBRVisitor::VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, + BugReporterContext &BRC, + BugReport &BR) { + if (Satisfied) + return nullptr; + + const ObjCMessageExpr *E = nullptr; + if (Optional P = Succ->getLocationAs()) + E = P->getStmtAs(); + if (!E) + return nullptr; + + ProgramStateRef State = Succ->getState(); + const LocationContext *LocCtx = Succ->getLocationContext(); + CallEventManager &CEMgr = State->getStateManager().getCallEventManager(); + CallEventRef Call = CEMgr.getObjCMethodCall(E, State, LocCtx); + SVal SelfVal = Call->getSelfSVal(); + + if (SelfSymbol == SelfVal.getAsSymbol()) { + 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 { + // Check for [super dealloc] method call. + if (!isSuperDeallocMessage(M)) + return; + + ProgramStateRef State = C.getState(); + SymbolRef SelfSymbol = M.getSelfSVal().getAsSymbol(); + assert(SelfSymbol && "No self symbol in call to [super dealloc]?"); + + State = State->add(SelfSymbol); + C.addTransition(State); +} + +void ObjCSuperDeallocChecker::checkPreObjCMessage(const ObjCMethodCall &M, + CheckerContext &C) const { + if (!isSuperDeallocMessage(M)) + return; + + ProgramStateRef State = C.getState(); + SymbolRef SelfSymbol = M.getSelfSVal().getAsSymbol(); + bool AlreadyCalled = State->contains(SelfSymbol); + + // 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; + + // Generate the report. + std::unique_ptr BR( + new BugReport(*DoubleSuperDeallocBugType, + "[super dealloc] called a second time", ErrNode)); + BR->addRange(M.getOriginExpr()->getSourceRange()); + BR->addVisitor(llvm::make_unique(SelfSymbol)); + C.emitReport(std::move(BR)); + + 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 { + if (M.getOriginExpr()->getReceiverKind() != ObjCMessageExpr::SuperInstance) + return false; + + ASTContext &Ctx = M.getState()->getStateManager().getContext(); + initIdentifierInfoAndSelectors(Ctx); + + return M.getSelector() == SELdealloc; +} + +//===----------------------------------------------------------------------===// +// Checker Registration. +//===----------------------------------------------------------------------===// + +void ento::registerObjCSuperDeallocChecker(CheckerManager &Mgr) { + const LangOptions &LangOpts = Mgr.getLangOpts(); + if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount) + return; + Mgr.registerChecker(); +} Index: test/Analysis/DeallocUseAfterFreeErrors.m =================================================================== --- /dev/null +++ test/Analysis/DeallocUseAfterFreeErrors.m @@ -0,0 +1,275 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.osx.cocoa.SuperDealloc,debug.ExprInspection -analyzer-output=text -verify %s + +void clang_analyzer_warnIfReached(); + +#define nil ((id)0) + +typedef unsigned long NSUInteger; +@protocol NSObject +- (instancetype)retain; +- (oneway void)release; +@end + +@interface NSObject { } +- (void)dealloc; +- (instancetype)init; +@end + +typedef struct objc_selector *SEL; + +//===------------------------------------------------------------------------=== +// +// 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) { // expected-note {{Taking true branch}} + [_ivar release]; + [super dealloc]; // expected-note {{[super dealloc] originally called here}} + // return; + } + [super dealloc]; // expected-warning{{[super dealloc] called a second time}} // expected-note {{[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]; // expected-note {{[super dealloc] originally called here}} +} +- (void)dealloc { + [self _cleanup]; // expected-note {{Calling '_cleanup'}} expected-note {{Returning from '_cleanup'}} + [super dealloc]; // expected-warning {{[super dealloc] called a second time}} expected-note {{[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 + +//===------------------------------------------------------------------------=== +// Do not warn about calling [super dealloc] twice if when the analyzer has +// inlined the call to its super deallocator. + +@interface SuperClassCallingSuperDealloc : NSObject +@end + +@implementation SuperClassCallingSuperDealloc +- (void)dealloc; { + [super dealloc]; +} +@end + +@interface SubclassCallingSuperDealloc : SuperClassCallingSuperDealloc +@end + +@implementation SubclassCallingSuperDealloc +- (void)dealloc; { + [super dealloc]; +} +@end + +//===------------------------------------------------------------------------=== +// Treat calling [super dealloc] twice as as a sink. + +@interface CallingSuperDeallocTwiceIsSink : NSObject +@end + +@implementation CallingSuperDeallocTwiceIsSink +- (void)dealloc; { + [super dealloc]; // expected-note {{[super dealloc] originally called here}} + [super dealloc]; // expected-warning {{[super dealloc] called a second time}} expected-note {{[super dealloc] called a second time}} + + clang_analyzer_warnIfReached(); // no-warning +} +@end