Added static analysis check to see if a subclass of NSObject implements -dealloc, and whether or not that implementation calls [super dealloc].

llvm-svn: 53075

clang/Driver/AnalysisConsumer.cpp

@@ -17,7 +17,6 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
 #include "llvm/Support/Compiler.h"
-#include "llvm/ADT/ImmutableList.h"
 #include "llvm/ADT/OwningPtr.h"
 #include "clang/AST/CFG.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
@@ -25,6 +24,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/AST/TranslationUnit.h"
 #include "clang/Analysis/PathSensitive/BugReporter.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/LocalCheckers.h"
@@ -32,6 +32,8 @@
 #include "clang/Analysis/PathSensitive/GRExprEngine.h"
 #include "llvm/Support/Streams.h"
 
+#include <vector>
+
 using namespace clang;
 
   
@@ -53,11 +55,10 @@ namespace {
 namespace {
 
   class VISIBILITY_HIDDEN AnalysisConsumer : public ASTConsumer {
-    typedef llvm::ImmutableList<CodeAction> Actions;
+    typedef std::vector<CodeAction> Actions;
     Actions FunctionActions;
     Actions ObjCMethodActions;
-
-    Actions::Factory F;
+    Actions ObjCImplementationActions;
     
   public:
     const bool Visualize;
@@ -78,16 +79,19 @@ namespace {
                      const std::string& fname,
                      const std::string& htmldir,
                      bool visualize, bool trim, bool analyzeAll) 
-      : FunctionActions(F.GetEmptyList()), ObjCMethodActions(F.GetEmptyList()),
-        Visualize(visualize), TrimGraph(trim), LOpts(lopts), Diags(diags),
+      : Visualize(visualize), TrimGraph(trim), LOpts(lopts), Diags(diags),
         Ctx(0), PP(pp), PPF(ppf),
         HTMLDir(htmldir),
         FName(fname),
         AnalyzeAll(analyzeAll) {}
     
     void addCodeAction(CodeAction action) {
-      FunctionActions = F.Concat(action, FunctionActions);
-      ObjCMethodActions = F.Concat(action, ObjCMethodActions);      
+      FunctionActions.push_back(action);
+      ObjCMethodActions.push_back(action);
+    }
+    
+    void addObjCImplementationAction(CodeAction action) {
+      ObjCImplementationActions.push_back(action);
     }
     
     virtual void Initialize(ASTContext &Context) {
@@ -95,6 +99,8 @@ namespace {
     }
     
     virtual void HandleTopLevelDecl(Decl *D);
+    virtual void HandleTranslationUnit(TranslationUnit &TU);
+    
     void HandleCode(Decl* D, Stmt* Body, Actions actions);
   };
     
@@ -235,6 +241,18 @@ void AnalysisConsumer::HandleTopLevelDecl(Decl *D) {
   }
 }
 
+void AnalysisConsumer::HandleTranslationUnit(TranslationUnit& TU) {
+
+  if (ObjCImplementationActions.empty())
+    return;
+    
+  for (TranslationUnit::iterator I = TU.begin(), E = TU.end(); I!=E; ++I) {
+    
+    if (ObjCImplementationDecl* ID = dyn_cast<ObjCImplementationDecl>(*I))
+      HandleCode(ID, 0, ObjCImplementationActions);
+  }
+}
+
 void AnalysisConsumer::HandleCode(Decl* D, Stmt* Body, Actions actions) {
   
   // Don't run the actions if an error has occured with parsing the file.
@@ -259,7 +277,7 @@ void AnalysisConsumer::HandleCode(Decl* D, Stmt* Body, Actions actions) {
   // Dispatch on the actions.  
   for (Actions::iterator I = actions.begin(),
                          E = actions.end(); I != E; ++I)
-    ((*I).getHead())(mgr);  
+    (*I)(mgr);  
 }
 
 //===----------------------------------------------------------------------===//
@@ -351,6 +369,11 @@ static void ActionCFGView(AnalysisManager& mgr) {
   mgr.getCFG().viewCFG();  
 }
 
+static void ActionCheckObjCDealloc(AnalysisManager& mgr) {
+  BugReporter BR(mgr);
+  CheckObjCDealloc(cast<ObjCImplementationDecl>(mgr.getCodeDecl()), BR);  
+}
+
 //===----------------------------------------------------------------------===//
 // AnalysisConsumer creation.
 //===----------------------------------------------------------------------===//
@@ -401,6 +424,9 @@ ASTConsumer* clang::CreateAnalysisConsumer(Analyses* Beg, Analyses* End,
       default: break;
     }
   
+  // Checks we always perform:
+  C->addObjCImplementationAction(&ActionCheckObjCDealloc);  
+  
   return C.take();
 }
 

clang/include/clang/Analysis/LocalCheckers.h

@@ -28,6 +28,7 @@ class LangOptions;
 class ParentMap;
 class LiveVariables;
 class BugReporter;
+class ObjCImplementationDecl;
   
 void CheckDeadStores(LiveVariables& L, BugReporter& BR); 
   
@@ -39,6 +40,9 @@ GRTransferFuncs* MakeCFRefCountTF(ASTContext& Ctx, bool GCEnabled,
                                   bool StandardWarnings,
                                   const LangOptions& lopts); 
   
+void CheckObjCDealloc(ObjCImplementationDecl* D, BugReporter& BR);
+
+  
 } // end namespace clang
 
 #endif

clang/include/clang/Analysis/PathSensitive/BugReporter.h

@@ -24,7 +24,6 @@
 #include <vector>
 #include <list>
 
-
 namespace clang {
   
 class PathDiagnostic;
@@ -286,6 +285,15 @@ public:
   iterator end() { return Reports.end(); }
 };
   
+class SimpleBugType : public BugTypeCacheLocation {
+  const char* name;  
+public:
+  SimpleBugType(const char* n) : name(n) {}
+  
+  virtual const char* getName() const { return name; }
+  virtual const char* getDescription() const { return name; }
+};
+  
 } // end clang namespace
 
 #endif

clang/lib/Analysis/CheckObjCDealloc.cpp

@@ -0,0 +1,118 @@
+//==- CheckObjCDealloc.cpp - Check ObjC -dealloc implementation --*- C++ -*-==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines a DeadStores, a flow-sensitive checker that looks for
+//  stores to variables that are no longer live.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/LocalCheckers.h"
+#include "clang/Analysis/PathDiagnostic.h"
+#include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/AST/ExprObjC.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/DeclObjC.h"
+#include <sstream>
+
+using namespace clang;
+
+static bool scan_dealloc(Stmt* S, Selector Dealloc) {  
+  
+  if (ObjCMessageExpr* ME = dyn_cast<ObjCMessageExpr>(S))
+    if (ME->getSelector() == Dealloc)
+      if (Expr* Receiver = ME->getReceiver()->IgnoreParenCasts())
+        if (PreDefinedExpr* E = dyn_cast<PreDefinedExpr>(Receiver))
+          if (E->getIdentType() == PreDefinedExpr::ObjCSuper)
+            return true;
+
+  // Recurse to children.
+
+  for (Stmt::child_iterator I = S->child_begin(), E= S->child_end(); I!=E; ++I)
+    if (*I && scan_dealloc(*I, Dealloc))
+      return true;
+  
+  return false;
+}
+
+void clang::CheckObjCDealloc(ObjCImplementationDecl* D, BugReporter& BR) {
+
+  ASTContext& Ctx = BR.getContext();
+
+  // Determine if the class subclasses NSObject.
+  IdentifierInfo* NSObjectII = &Ctx.Idents.get("NSObject");
+  ObjCInterfaceDecl* ID = D->getClassInterface();
+  
+  for ( ; ID ; ID = ID->getSuperClass())
+    if (ID->getIdentifier() == NSObjectII)
+      break;
+  
+  if (!ID)
+    return;
+  
+  // Get the "dealloc" selector.
+  IdentifierInfo* II = &Ctx.Idents.get("dealloc");
+  Selector S = Ctx.Selectors.getSelector(0, &II);
+  
+  ObjCMethodDecl* MD = 0;
+  
+  // Scan the instance methods for "dealloc".
+  for (ObjCImplementationDecl::instmeth_iterator I = D->instmeth_begin(),
+       E = D->instmeth_end(); I!=E; ++I) {
+    
+    if ((*I)->getSelector() == S) {
+      MD = *I;
+      break;
+    }    
+  }
+  
+  if (!MD) { // No dealloc found.
+    
+    // FIXME: This code should be reduced to three lines if possible (Refactor).
+    SimpleBugType BT("missing -dealloc");
+    DiagCollector C(BT);
+    
+    std::ostringstream os;
+    os << "Objective-C class '" << D->getName()
+       << "' lacks a 'dealloc' instance method";
+    
+    Diagnostic& Diag = BR.getDiagnostic();    
+    Diag.Report(&C,
+                Ctx.getFullLoc(D->getLocStart()),
+                Diag.getCustomDiagID(Diagnostic::Warning, os.str().c_str()),
+                NULL, 0, NULL, 0);
+        
+    for (DiagCollector::iterator I = C.begin(), E = C.end(); I != E; ++I)
+      BR.EmitWarning(*I);
+    
+    return;
+  }
+  
+  // dealloc found.  Scan for missing [super dealloc].
+  if (MD->getBody() && !scan_dealloc(MD->getBody(), S)) {
+    
+    // FIXME: This code should be reduced to three lines if possible (Refactor).
+    SimpleBugType BT("missing [super dealloc]");
+    DiagCollector C(BT);
+    
+    std::ostringstream os;
+    os << "The 'dealloc' instance method in Objective-C class '" << D->getName()
+       << "' does not send a 'dealloc' message to it super class"
+           " (missing [super dealloc])";
+    
+    Diagnostic& Diag = BR.getDiagnostic();    
+    Diag.Report(&C,
+                Ctx.getFullLoc(MD->getLocStart()),
+                Diag.getCustomDiagID(Diagnostic::Warning, os.str().c_str()),
+                NULL, 0, NULL, 0);
+    
+    for (DiagCollector::iterator I = C.begin(), E = C.end(); I != E; ++I)
+      BR.EmitWarning(*I);
+  }    
+}
+

clang/lib/Analysis/DeadStores.cpp

@@ -143,23 +143,6 @@ public:
   
 } // end anonymous namespace
 
-//===----------------------------------------------------------------------===//
-// BugReporter-based invocation of the Dead-Stores checker.
-//===----------------------------------------------------------------------===//
-  
-namespace {
-  
-class SimpleBugType : public BugTypeCacheLocation {
-  const char* name;  
-public:
-  SimpleBugType(const char* n) : name(n) {}
-  
-  virtual const char* getName() const {
-    return name;
-  }
-};
-} // end anonymous namespace
-
 //===----------------------------------------------------------------------===//
 // Driver function to invoke the Dead-Stores checker on a CFG.
 //===----------------------------------------------------------------------===//

clang/test/Analysis/NSPanel.m

@@ -67,7 +67,8 @@ extern NSString *NSWindowDidBecomeKeyNotification;
 - (void)myMethod;
 - (void)myMethod2;
 @end
-@implementation MyClass
+
+@implementation MyClass // no-warning
 - (void)myMethod
 {
   NSPanel *panel = [[NSPanel alloc] initWithContentRect:NSMakeRect(0, 0, 200, 200) styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:(BOOL)1];

clang/test/Analysis/NSString.m

@@ -143,7 +143,7 @@ NSString* f10() {
 - (NSString*) getShared;
 + (C1*) sharedInstance;
 @end
-@implementation C1 : NSObject {}
+@implementation C1 : NSObject {} // expected-warning{{Objective-C class 'C1' lacks a 'dealloc' instance method}}
 - (NSString*) getShared {
   static NSString* s = 0;
   if (!s) s = [[NSString alloc] init];    
@@ -161,7 +161,7 @@ NSString* f10() {
 @interface SharedClass : NSObject
 + (id)sharedInstance;
 @end
-@implementation SharedClass
+@implementation SharedClass // expected-warning {{Objective-C class 'SharedClass' lacks a 'dealloc' instance method}}
 
 - (id)_init {
     if ((self = [super init])) {