From: Devin Coughlin Date: Mon, 22 Feb 2016 17:56:24 +0000 (+0000) Subject: [analyzer] Detect duplicate [super dealloc] calls X-Git-Tag: llvmorg-3.9.0-rc1~13547 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=eb6673cfed481ec3200331998f042a5beb5cfae9;p=platform%2Fupstream%2Fllvm.git [analyzer] Detect duplicate [super dealloc] calls Add an alpha path checker that warns about duplicate calls to [super dealloc]. This will form the foundation of a checker that will detect uses of 'self' after calling [super dealloc]. Part of rdar://problem/6953275. Based on a patch by David Kilzer! Differential Revision: http://reviews.llvm.org/D5238 llvm-svn: 261545 --- diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 58ff48d..e1d98cf0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -56,6 +56,7 @@ add_clang_library(clangStaticAnalyzerCheckers ObjCContainersChecker.cpp ObjCMissingSuperCallChecker.cpp ObjCSelfInitChecker.cpp + ObjCSuperDeallocChecker.cpp ObjCUnusedIVarsChecker.cpp PaddingChecker.cpp PointerArithChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/clang/lib/StaticAnalyzer/Checkers/Checkers.td index 8133d29..7748d9c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/lib/StaticAnalyzer/Checkers/Checkers.td @@ -513,6 +513,10 @@ def ObjCDeallocChecker : Checker<"Dealloc">, 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">; diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp new file mode 100644 index 0000000..d1d6ef3 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp @@ -0,0 +1,197 @@ +//===- 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 warns when +// [super dealloc] is called twice on the same instance 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 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 whether [super dealloc] has previously been called on the +// a SymbolRef for the receiver. +REGISTER_SET_WITH_PROGRAMSTATE(CalledSuperDealloc, SymbolRef) + +class SuperDeallocBRVisitor final + : public BugReporterVisitorImpl { + + SymbolRef ReceiverSymbol; + bool Satisfied; + +public: + SuperDeallocBRVisitor(SymbolRef ReceiverSymbol) + : ReceiverSymbol(ReceiverSymbol), + Satisfied(false) {} + + PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, + BugReporterContext &BRC, + BugReport &BR) override; + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.Add(ReceiverSymbol); + } +}; + +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]?"); + + bool AlreadyCalled = State->contains(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; + + // Generate the report. + std::unique_ptr BR( + new BugReport(*DoubleSuperDeallocBugType, + "[super dealloc] should not be called multiple times", + ErrNode)); + BR->addRange(M.getOriginExpr()->getSourceRange()); + BR->addVisitor(llvm::make_unique(ReceiverSymbol)); + C.emitReport(std::move(BR)); + + return; +} + +void ObjCSuperDeallocChecker::checkPostObjCMessage(const ObjCMethodCall &M, + CheckerContext &C) const { + // Check for [super dealloc] method call. + if (!isSuperDeallocMessage(M)) + return; + + ProgramStateRef State = C.getState(); + SymbolRef ReceiverSymbol = M.getSelfSVal().getAsSymbol(); + assert(ReceiverSymbol && "No receiver symbol at call to [super dealloc]?"); + + // We add this transition in checkPostObjCMessage to avoid warning when + // we inline a call to [super dealloc] where the inlined call itself + // calls [super dealloc]. + State = State->add(ReceiverSymbol); + C.addTransition(State); +} + +ObjCSuperDeallocChecker::ObjCSuperDeallocChecker() + : IIdealloc(nullptr), IINSObject(nullptr) { + + DoubleSuperDeallocBugType.reset( + new BugType(this, "[super dealloc] should not be called more than once", + categories::CoreFoundationObjectiveC)); +} + +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; +} + +PathDiagnosticPiece *SuperDeallocBRVisitor::VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, + BugReporterContext &BRC, + BugReport &BR) { + if (Satisfied) + return nullptr; + + ProgramStateRef State = Succ->getState(); + + bool CalledNow = + Succ->getState()->contains(ReceiverSymbol); + bool CalledBefore = + Pred->getState()->contains(ReceiverSymbol); + + // Is Succ the node on which the analyzer noted that [super dealloc] was + // called on ReceiverSymbol? + if (CalledNow && !CalledBefore) { + 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] called here"); + } + + return nullptr; +} + +//===----------------------------------------------------------------------===// +// Checker Registration. +//===----------------------------------------------------------------------===// + +void ento::registerObjCSuperDeallocChecker(CheckerManager &Mgr) { + const LangOptions &LangOpts = Mgr.getLangOpts(); + if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount) + return; + Mgr.registerChecker(); +} diff --git a/clang/test/Analysis/DeallocUseAfterFreeErrors.m b/clang/test/Analysis/DeallocUseAfterFreeErrors.m new file mode 100644 index 0000000..75c2a1e --- /dev/null +++ b/clang/test/Analysis/DeallocUseAfterFreeErrors.m @@ -0,0 +1,298 @@ +// 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] called here}} + // return; + } + [super dealloc]; // expected-warning{{[super dealloc] should not be called multiple times}} + // expected-note@-1{{[super dealloc] should not be called multiple times}} +} +@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] called here}} +} +- (void)dealloc { + [self _cleanup]; // expected-note {{Calling '_cleanup'}} + //expected-note@-1 {{Returning from '_cleanup'}} + [super dealloc]; // expected-warning {{[super dealloc] should not be called multiple times}} + // expected-note@-1 {{[super dealloc] should not be called multiple times}} +} +@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] called here}} + [super dealloc]; // expected-warning {{[super dealloc] should not be called multiple times}} + // expected-note@-1 {{[super dealloc] should not be called multiple times}} + + clang_analyzer_warnIfReached(); // no-warning +} +@end + + +//===------------------------------------------------------------------------=== +// Test path notes with intervening method call on self. + +@interface InterveningMethodCallOnSelf : NSObject +@end + +@implementation InterveningMethodCallOnSelf +- (void)anotherMethod { +} + +- (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@-1 {{[super dealloc] should not be called multiple times}} +} +@end