r261545 - [analyzer] Detect duplicate [super dealloc] calls

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Mon Feb 22 11:56:24 2016
New Revision: 261545

URL: http://llvm.org/viewvc/llvm-project?rev=261545&view=rev
Log:
[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

Added:
    cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
    cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
    cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=261545&r1=261544&r2=261545&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Mon Feb 22 11:56:24 2016
@@ -56,6 +56,7 @@ add_clang_library(clangStaticAnalyzerChe
   ObjCContainersChecker.cpp
   ObjCMissingSuperCallChecker.cpp
   ObjCSelfInitChecker.cpp
+  ObjCSuperDeallocChecker.cpp
   ObjCUnusedIVarsChecker.cpp
   PaddingChecker.cpp
   PointerArithChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=261545&r1=261544&r2=261545&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Mon Feb 22 11:56:24 2016
@@ -513,6 +513,10 @@ def ObjCDeallocChecker : Checker<"Deallo
   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">;

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp?rev=261545&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp Mon Feb 22 11:56:24 2016
@@ -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<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;
+
+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<SuperDeallocBRVisitor> {
+
+  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<CalledSuperDealloc>(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<BugReport> BR(
+      new BugReport(*DoubleSuperDeallocBugType,
+                    "[super dealloc] should not be called multiple times",
+                    ErrNode));
+  BR->addRange(M.getOriginExpr()->getSourceRange());
+  BR->addVisitor(llvm::make_unique<SuperDeallocBRVisitor>(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<CalledSuperDealloc>(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<CalledSuperDealloc>(ReceiverSymbol);
+  bool CalledBefore =
+      Pred->getState()->contains<CalledSuperDealloc>(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<ObjCSuperDeallocChecker>();
+}

Added: cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m?rev=261545&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m (added)
+++ cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m Mon Feb 22 11:56:24 2016
@@ -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;
+ at protocol NSObject
+- (instancetype)retain;
+- (oneway void)release;
+ at end
+
+ at interface NSObject <NSObject> { }
+- (void)dealloc;
+- (instancetype)init;
+ at end
+
+typedef struct objc_selector *SEL;
+
+//===------------------------------------------------------------------------===
+//  <rdar://problem/6953275>
+//  Check that 'self' is not referenced after calling '[super dealloc]'.
+
+ at interface SuperDeallocThenReleaseIvarClass : NSObject {
+  NSObject *_ivar;
+}
+ at end
+
+ at implementation SuperDeallocThenReleaseIvarClass
+- (instancetype)initWithIvar:(NSObject *)ivar {
+  self = [super init];
+  if (!self)
+    return nil;
+  _ivar = [ivar retain];
+  return self;
+}
+- (void)dealloc {
+  [super dealloc];
+  [_ivar release];
+}
+ at end
+
+ at interface SuperDeallocThenAssignNilToIvarClass : NSObject {
+  NSObject *_delegate;
+}
+ at end
+
+ at implementation SuperDeallocThenAssignNilToIvarClass
+- (instancetype)initWithDelegate:(NSObject *)delegate {
+  self = [super init];
+  if (!self)
+    return nil;
+  _delegate = delegate;
+  return self;
+}
+- (void)dealloc {
+  [super dealloc];
+  _delegate = nil;
+}
+ at end
+
+ at interface SuperDeallocThenReleasePropertyClass : NSObject { }
+ at property (retain) NSObject *ivar;
+ at end
+
+ at 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;
+}
+ at end
+
+ at interface SuperDeallocThenAssignNilToPropertyClass : NSObject { }
+ at property (assign) NSObject *delegate;
+ at end
+
+ at 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;
+}
+ at end
+
+ at interface SuperDeallocThenCallInstanceMethodClass : NSObject { }
+- (void)_invalidate;
+ at end
+
+ at implementation SuperDeallocThenCallInstanceMethodClass
+- (void)_invalidate {
+}
+- (void)dealloc {
+  [super dealloc];
+  [self _invalidate];
+}
+ at end
+
+ at interface SuperDeallocThenCallNonObjectiveCMethodClass : NSObject { }
+ at end
+
+static void _invalidate(NSObject *object) {
+  (void)object;
+}
+
+ at implementation SuperDeallocThenCallNonObjectiveCMethodClass
+- (void)dealloc {
+  [super dealloc];
+  _invalidate(self);
+}
+ at end
+
+ at interface TwoSuperDeallocCallsClass : NSObject {
+  NSObject *_ivar;
+}
+- (void)_invalidate;
+ at end
+
+ at implementation TwoSuperDeallocCallsClass
+- (void)_invalidate {
+}
+- (void)dealloc {
+  if (_ivar) {
+    [_ivar release];
+    [super dealloc];
+    return;
+  }
+  [super dealloc];
+  [self _invalidate];
+}
+ at end
+
+//===------------------------------------------------------------------------===
+// Warn about calling [super dealloc] twice due to missing return statement.
+
+ at interface MissingReturnCausesDoubleSuperDeallocClass : NSObject {
+  NSObject *_ivar;
+}
+ at end
+
+ at 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 at -1{{[super dealloc] should not be called multiple times}}
+}
+ at end
+
+//===------------------------------------------------------------------------===
+// Warn about calling [super dealloc] twice in two different methods.
+
+ at interface SuperDeallocInOtherMethodClass : NSObject {
+  NSObject *_ivar;
+}
+- (void)_cleanup;
+ at end
+
+ at implementation SuperDeallocInOtherMethodClass
+- (void)_cleanup {
+  [_ivar release];
+  [super dealloc]; // expected-note {{[super dealloc] called here}}
+}
+- (void)dealloc {
+  [self _cleanup]; // expected-note {{Calling '_cleanup'}}
+  //expected-note at -1 {{Returning from '_cleanup'}}
+  [super dealloc]; // expected-warning {{[super dealloc] should not be called multiple times}}
+  // expected-note at -1 {{[super dealloc] should not be called multiple times}}
+}
+ at 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.
+
+ at class ClassWithOwnIvarInstanceClass;
+ at interface ClassWithOwnIvarInstanceClass : NSObject {
+  ClassWithOwnIvarInstanceClass *_ivar;
+  NSUInteger _retainCount;
+}
+ at end
+
+ at 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
+}
+ at end
+
+//===------------------------------------------------------------------------===
+// Do not warn about calling [super dealloc] twice if +dealloc is a class
+// method.
+
+ at interface SuperDeallocClassMethodIgnoredClass : NSObject { }
++ (void)dealloc;
+ at end
+
+ at implementation SuperDeallocClassMethodIgnoredClass
++ (void)dealloc { }
+ at end
+
+ at interface SuperDeallocClassMethodIgnoredSubClass : NSObject { }
++ (void)dealloc;
+ at end
+
+ at implementation SuperDeallocClassMethodIgnoredSubClass
++ (void)dealloc {
+  [super dealloc];
+  [super dealloc]; // no warning: class method
+}
+ at end
+
+//===------------------------------------------------------------------------===
+// Do not warn about calling [super dealloc] twice if when the analyzer has
+// inlined the call to its super deallocator.
+
+ at interface SuperClassCallingSuperDealloc : NSObject
+ at end
+
+ at implementation SuperClassCallingSuperDealloc
+- (void)dealloc; {
+  [super dealloc];
+}
+ at end
+
+ at interface SubclassCallingSuperDealloc : SuperClassCallingSuperDealloc
+ at end
+
+ at implementation SubclassCallingSuperDealloc
+- (void)dealloc; {
+  [super dealloc];
+}
+ at end
+
+//===------------------------------------------------------------------------===
+// Treat calling [super dealloc] twice as as a sink.
+
+ at interface CallingSuperDeallocTwiceIsSink : NSObject
+ at end
+
+ at 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 at -1 {{[super dealloc] should not be called multiple times}}
+
+  clang_analyzer_warnIfReached(); // no-warning
+}
+ at end
+
+
+//===------------------------------------------------------------------------===
+// Test path notes with intervening method call on self.
+
+ at interface InterveningMethodCallOnSelf : NSObject
+ at end
+
+ at 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 at -1 {{[super dealloc] should not be called multiple times}}
+}
+ at end