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 { + + 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 { + + mutable IdentifierInfo *IIdealloc, *IINSObject; + mutable Selector SELdealloc; + + std::unique_ptr 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 P = Succ->getLocationAs()) + E = P->getStmtAs(); + + 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(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( + 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(); + 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 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(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(); +} 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 { } +- (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 + +@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