[analyzer] Migrate NSErrorChecker and DereferenceChecker to CheckerV2.

They cooperate in that NSErrorChecker listens for ImplicitNullDerefEvent events that
DereferenceChecker can dispatch.
ImplicitNullDerefEvent is when we dereferenced a location that may be null.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@126659 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/StaticAnalyzer/Core/CheckerV2.h

@@ -15,6 +15,7 @@
 #define LLVM_CLANG_SA_CORE_CHECKERV2
 
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "llvm/Support/Casting.h"
 
 namespace clang {
@@ -358,6 +359,14 @@ public:
   }
 };
 
+/// \brief We dereferenced a location that may be null.
+struct ImplicitNullDerefEvent {
+  SVal Location;
+  bool IsLoad;
+  ExplodedNode *SinkNode;
+  BugReporter *BR;
+};
+
 } // end ento namespace
 
 } // end clang namespace

lib/StaticAnalyzer/Checkers/Checkers.td

@@ -70,11 +70,19 @@ def ObjCMethSigsChecker : Checker<"MethodSigs">,
 def ObjCUnusedIvarsChecker : Checker<"UnusedIvars">,
   HelpText<"Warn about private ivars that are never used">,
   DescFile<"ObjCUnusedIVarsChecker.cpp">;
+ 
+def NSErrorChecker : Checker<"NSError">,
+  HelpText<"Check usage of NSError** parameters">,
+  DescFile<"NSErrorChecker.cpp">;
 
 } // end "cocoa"
 
 let ParentPackage = Core in {
 
+def DereferenceChecker : Checker<"Deref">,
+  HelpText<"Check for null pointers at loads and stores">,
+  DescFile<"DereferenceChecker.cpp">;
+
 def CallAndMessageChecker : Checker<"CallAndMsg">,
   HelpText<"Check for errors of call and objc message expressions">,
   DescFile<"CallAndMessageChecker.cpp">;
@@ -162,6 +170,11 @@ def CFRetainReleaseChecker : Checker<"CFRetainRelease">,
   HelpText<"Check for null arguments to CFRetain/CFRelease">,
   DescFile<"BasicObjCFoundationChecks.cpp">;
 
+def CFErrorChecker : Checker<"CFError">,
+  InPackage<MacOSX>,
+  HelpText<"Check usage of CFErrorRef* parameters">,
+  DescFile<"NSErrorChecker.cpp">;
+
 def LLVMConventionsChecker : Checker<"Conventions">,
   InPackage<LLVM>,
   HelpText<"Check code for LLVM codebase conventions">,

lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp

@@ -12,51 +12,31 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "InternalChecks.h"
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/CheckerV2.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
-#include "clang/StaticAnalyzer/Checkers/DereferenceChecker.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/Checker.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
-class DereferenceChecker : public Checker {
-  BuiltinBug *BT_null;
-  BuiltinBug *BT_undef;
-  llvm::SmallVector<ExplodedNode*, 2> ImplicitNullDerefNodes;
-public:
-  DereferenceChecker() : BT_null(0), BT_undef(0) {}
-  static void *getTag() { static int tag = 0; return &tag; }
-  void visitLocation(CheckerContext &C, const Stmt *S, SVal location,
-                     bool isLoad);
+class DereferenceChecker
+    : public CheckerV2< check::Location,
+                        EventDispatcher<ImplicitNullDerefEvent> > {
+  mutable llvm::OwningPtr<BuiltinBug> BT_null;
+  mutable llvm::OwningPtr<BuiltinBug> BT_undef;
 
-  std::pair<ExplodedNode * const*, ExplodedNode * const*>
-  getImplicitNodes() const {
-    return std::make_pair(ImplicitNullDerefNodes.data(),
-                          ImplicitNullDerefNodes.data() +
-                          ImplicitNullDerefNodes.size());
-  }
-  void AddDerefSource(llvm::raw_ostream &os,
-                      llvm::SmallVectorImpl<SourceRange> &Ranges,
-                      const Expr *Ex, bool loadedFrom = false);
+public:
+  void checkLocation(SVal location, bool isLoad, CheckerContext &C) const;
+
+  static void AddDerefSource(llvm::raw_ostream &os,
+                             llvm::SmallVectorImpl<SourceRange> &Ranges,
+                             const Expr *Ex, bool loadedFrom = false);
 };
 } // end anonymous namespace
 
-void ento::RegisterDereferenceChecker(ExprEngine &Eng) {
-  Eng.registerCheck(new DereferenceChecker());
-}
-
-std::pair<ExplodedNode * const *, ExplodedNode * const *>
-ento::GetImplicitNullDereferences(ExprEngine &Eng) {
-  DereferenceChecker *checker = Eng.getChecker<DereferenceChecker>();
-  if (!checker)
-    return std::make_pair((ExplodedNode * const *) 0,
-                          (ExplodedNode * const *) 0);
-  return checker->getImplicitNodes();
-}
-
 void DereferenceChecker::AddDerefSource(llvm::raw_ostream &os,
                                      llvm::SmallVectorImpl<SourceRange> &Ranges,
                                         const Expr *Ex,
@@ -85,13 +65,13 @@ void DereferenceChecker::AddDerefSource(llvm::raw_ostream &os,
   }
 }
 
-void DereferenceChecker::visitLocation(CheckerContext &C, const Stmt *S,
-                                       SVal l, bool isLoad) {
+void DereferenceChecker::checkLocation(SVal l, bool isLoad,
+                                       CheckerContext &C) const {
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
     if (ExplodedNode *N = C.generateSink()) {
       if (!BT_undef)
-        BT_undef = new BuiltinBug("Dereference of undefined pointer value");
+        BT_undef.reset(new BuiltinBug("Dereference of undefined pointer value"));
 
       EnhancedBugReport *report =
         new EnhancedBugReport(*BT_undef, BT_undef->getDescription(), N);
@@ -108,6 +88,7 @@ void DereferenceChecker::visitLocation(CheckerContext &C, const Stmt *S,
   if (!isa<Loc>(location))
     return;
 
+  const Stmt *S = C.getStmt();
   const GRState *state = C.getState();
   const GRState *notNullState, *nullState;
   llvm::tie(notNullState, nullState) = state->assume(location);
@@ -123,7 +104,7 @@ void DereferenceChecker::visitLocation(CheckerContext &C, const Stmt *S,
       // We know that 'location' cannot be non-null.  This is what
       // we call an "explicit" null dereference.
       if (!BT_null)
-        BT_null = new BuiltinBug("Dereference of null pointer");
+        BT_null.reset(new BuiltinBug("Dereference of null pointer"));
 
       llvm::SmallString<100> buf;
       llvm::SmallVector<SourceRange, 2> Ranges;
@@ -195,11 +176,17 @@ void DereferenceChecker::visitLocation(CheckerContext &C, const Stmt *S,
       // Otherwise, we have the case where the location could either be
       // null or not-null.  Record the error node as an "implicit" null
       // dereference.
-      if (ExplodedNode *N = C.generateSink(nullState))
-        ImplicitNullDerefNodes.push_back(N);
+      if (ExplodedNode *N = C.generateSink(nullState)) {
+        ImplicitNullDerefEvent event = { l, isLoad, N, &C.getBugReporter() };
+        dispatchEvent(event);
+      }
     }
   }
 
   // From this point forward, we know that the location is not null.
   C.addTransition(notNullState);
 }
+
+void ento::registerDereferenceChecker(CheckerManager &mgr) {
+  mgr.registerChecker<DereferenceChecker>();
+}

lib/StaticAnalyzer/Checkers/ExprEngine.cpp

@@ -308,22 +308,6 @@ void ExprEngine::CheckerVisitBind(const Stmt *StoreE, ExplodedNodeSet &Dst,
 // Engine construction and deletion.
 //===----------------------------------------------------------------------===//
 
-static void RegisterInternalChecks(ExprEngine &Eng) {
-  // Register internal "built-in" BugTypes with the BugReporter. These BugTypes
-  // are different than what probably many checks will do since they don't
-  // create BugReports on-the-fly but instead wait until ExprEngine finishes
-  // analyzing a function.  Generation of BugReport objects is done via a call
-  // to 'FlushReports' from BugReporter.
-  // The following checks do not need to have their associated BugTypes
-  // explicitly registered with the BugReporter.  If they issue any BugReports,
-  // their associated BugType will get registered with the BugReporter
-  // automatically.  Note that the check itself is owned by the ExprEngine
-  // object.
-  // CallAndMessageChecker should be registered before AttrNonNullChecker,
-  // where we assume arguments are not undefined.
-  RegisterDereferenceChecker(Eng);
-}
-
 ExprEngine::ExprEngine(AnalysisManager &mgr, TransferFuncs *tf)
   : AMgr(mgr),
     Engine(*this),
@@ -338,8 +322,6 @@ ExprEngine::ExprEngine(AnalysisManager &mgr, TransferFuncs *tf)
     NSExceptionII(NULL), NSExceptionInstanceRaiseSelectors(NULL),
     RaiseSel(GetNullarySelector("raise", getContext())),
     BR(mgr, *this), TF(tf) {
-  // Register internal checks.
-  RegisterInternalChecks(*this);
 
   // FIXME: Eventually remove the TF object entirely.
   TF->RegisterChecks(*this);

lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp

@@ -1,4 +1,4 @@
-//=- NSErrorCheckerer.cpp - Coding conventions for uses of NSError -*- C++ -*-==//
+//=- NSErrorChecker.cpp - Coding conventions for uses of NSError -*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -15,11 +15,12 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/StaticAnalyzer/Checkers/LocalCheckers.h"
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/CheckerV2.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/GRStateTrait.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "clang/StaticAnalyzer/Checkers/DereferenceChecker.h"
-#include "BasicObjCFoundationChecks.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/SmallVector.h"
@@ -27,142 +28,248 @@
 using namespace clang;
 using namespace ento;
 
+static bool IsNSError(const ParmVarDecl *PD, IdentifierInfo *II);
+static bool IsCFError(const ParmVarDecl *PD, IdentifierInfo *II);
+
+//===----------------------------------------------------------------------===//
+// NSErrorMethodChecker
+//===----------------------------------------------------------------------===//
+
 namespace {
-class NSErrorChecker : public BugType {
-  const Decl &CodeDecl;
-  const bool isNSErrorWarning;
-  IdentifierInfo * const II;
-  ExprEngine &Eng;
-
-  void CheckSignature(const ObjCMethodDecl& MD, QualType& ResultTy,
-                      llvm::SmallVectorImpl<VarDecl*>& ErrorParams);
-
-  void CheckSignature(const FunctionDecl& MD, QualType& ResultTy,
-                      llvm::SmallVectorImpl<VarDecl*>& ErrorParams);
-
-  bool CheckNSErrorArgument(QualType ArgTy);
-  bool CheckCFErrorArgument(QualType ArgTy);
-
-  void CheckParamDeref(const VarDecl *V, const LocationContext *LC,
-                       const GRState *state, BugReporter& BR);
-
-  void EmitRetTyWarning(BugReporter& BR, const Decl& CodeDecl);
+class NSErrorMethodChecker
+    : public CheckerV2< check::ASTDecl<ObjCMethodDecl> > {
+  mutable IdentifierInfo *II;
 
 public:
-  NSErrorChecker(const Decl &D, bool isNSError, ExprEngine& eng)
-    : BugType(isNSError ? "NSError** null dereference"
-                        : "CFErrorRef* null dereference",
-              "Coding conventions (Apple)"),
-    CodeDecl(D),
-    isNSErrorWarning(isNSError),
-    II(&eng.getContext().Idents.get(isNSErrorWarning ? "NSError":"CFErrorRef")),
-    Eng(eng) {}
+  NSErrorMethodChecker() : II(0) { }
 
-  void FlushReports(BugReporter& BR);
+  void checkASTDecl(const ObjCMethodDecl *D,
+                    AnalysisManager &mgr, BugReporter &BR) const;
+};
+}
+
+void NSErrorMethodChecker::checkASTDecl(const ObjCMethodDecl *D,
+                                        AnalysisManager &mgr,
+                                        BugReporter &BR) const {
+  if (!D->isThisDeclarationADefinition())
+    return;
+  if (!D->getResultType()->isVoidType())
+    return;
+
+  if (!II)
+    II = &D->getASTContext().Idents.get("NSError"); 
+
+  bool hasNSError = false;
+  for (ObjCMethodDecl::param_iterator
+         I = D->param_begin(), E = D->param_end(); I != E; ++I)  {
+    if (IsNSError(*I, II)) {
+      hasNSError = true;
+      break;
+    }
+  }
+
+  if (hasNSError) {
+    const char *err = "Method accepting NSError** "
+        "should have a non-void return value to indicate whether or not an "
+        "error occurred";
+    BR.EmitBasicReport("Bad return type when passing NSError**",
+                       "Coding conventions (Apple)", err, D->getLocation());
+  }
+}
+
+//===----------------------------------------------------------------------===//
+// CFErrorFunctionChecker
+//===----------------------------------------------------------------------===//
+
+namespace {
+class CFErrorFunctionChecker
+    : public CheckerV2< check::ASTDecl<FunctionDecl> > {
+  mutable IdentifierInfo *II;
+
+public:
+  CFErrorFunctionChecker() : II(0) { }
+
+  void checkASTDecl(const FunctionDecl *D,
+                    AnalysisManager &mgr, BugReporter &BR) const;
+};
+}
+
+void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D,
+                                        AnalysisManager &mgr,
+                                        BugReporter &BR) const {
+  if (!D->isThisDeclarationADefinition())
+    return;
+  if (!D->getResultType()->isVoidType())
+    return;
+
+  if (!II)
+    II = &D->getASTContext().Idents.get("CFErrorRef"); 
+
+  bool hasCFError = false;
+  for (FunctionDecl::param_const_iterator
+         I = D->param_begin(), E = D->param_end(); I != E; ++I)  {
+    if (IsCFError(*I, II)) {
+      hasCFError = true;
+      break;
+    }
+  }
+
+  if (hasCFError) {
+    const char *err = "Function accepting CFErrorRef* "
+        "should have a non-void return value to indicate whether or not an "
+        "error occurred";
+    BR.EmitBasicReport("Bad return type when passing CFErrorRef*",
+                       "Coding conventions (Apple)", err, D->getLocation());
+  }
+}
+
+//===----------------------------------------------------------------------===//
+// NSOrCFErrorDerefChecker
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+class NSErrorDerefBug : public BugType {
+public:
+  NSErrorDerefBug() : BugType("NSError** null dereference",
+                              "Coding conventions (Apple)") {}
 };
 
-} // end anonymous namespace
+class CFErrorDerefBug : public BugType {
+public:
+  CFErrorDerefBug() : BugType("CFErrorRef* null dereference",
+                              "Coding conventions (Apple)") {}
+};
 
-void ento::RegisterNSErrorChecks(BugReporter& BR, ExprEngine &Eng,
-                                  const Decl &D) {
-  BR.Register(new NSErrorChecker(D, true, Eng));
-  BR.Register(new NSErrorChecker(D, false, Eng));
 }
 
-void NSErrorChecker::FlushReports(BugReporter& BR) {
-  // Get the analysis engine and the exploded analysis graph.
-  ExplodedGraph& G = Eng.getGraph();
+namespace {
+class NSOrCFErrorDerefChecker
+    : public CheckerV2< check::Location,
+                        check::Event<ImplicitNullDerefEvent> > {
+  mutable IdentifierInfo *NSErrorII, *CFErrorII;
+public:
+  bool ShouldCheckNSError, ShouldCheckCFError;
+  NSOrCFErrorDerefChecker() : NSErrorII(0), CFErrorII(0),
+                              ShouldCheckNSError(0), ShouldCheckCFError(0) { }
 
-  // Get the ASTContext, which is useful for querying type information.
-  ASTContext &Ctx = BR.getContext();
+  void checkLocation(SVal loc, bool isLoad, CheckerContext &C) const;
+  void checkEvent(ImplicitNullDerefEvent event) const;
+};
+}
 
-  QualType ResultTy;
-  llvm::SmallVector<VarDecl*, 5> ErrorParams;
+namespace { struct NSErrorOut {}; }
+namespace { struct CFErrorOut {}; }
 
-  if (const ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(&CodeDecl))
-    CheckSignature(*MD, ResultTy, ErrorParams);
-  else if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(&CodeDecl))
-    CheckSignature(*FD, ResultTy, ErrorParams);
-  else
+typedef llvm::ImmutableMap<SymbolRef, unsigned> ErrorOutFlag;
+
+namespace clang {
+namespace ento {
+  template <>
+  struct GRStateTrait<NSErrorOut> : public GRStatePartialTrait<ErrorOutFlag> {  
+    static void *GDMIndex() { static int index = 0; return &index; }
+  };
+  template <>
+  struct GRStateTrait<CFErrorOut> : public GRStatePartialTrait<ErrorOutFlag> {  
+    static void *GDMIndex() { static int index = 0; return &index; }
+  };
+}
+}
+
+template <typename T>
+static bool hasFlag(SVal val, const GRState *state) {
+  if (SymbolRef sym = val.getAsSymbol())
+    if (const unsigned *attachedFlags = state->get<T>(sym))
+      return *attachedFlags;
+  return false;
+}
+
+template <typename T>
+static void setFlag(const GRState *state, SVal val, CheckerContext &C) {
+  // We tag the symbol that the SVal wraps.
+  if (SymbolRef sym = val.getAsSymbol())
+    C.addTransition(state->set<T>(sym, true));
+}
+
+void NSOrCFErrorDerefChecker::checkLocation(SVal loc, bool isLoad,
+                                            CheckerContext &C) const {
+  if (!isLoad)
+    return;
+  if (loc.isUndef() || !isa<Loc>(loc))
     return;
 
-  if (ErrorParams.empty())
+  const GRState *state = C.getState();
+
+  // If we are loading from NSError**/CFErrorRef* parameter, mark the resulting
+  // SVal so that we can later check it when handling the
+  // ImplicitNullDerefEvent event.
+  // FIXME: Cumbersome! Maybe add hook at construction of SVals at start of
+  // function ?
+  
+  const VarDecl *VD = loc.getAsVarDecl();
+  if (!VD) return;
+  const ParmVarDecl *PD = dyn_cast<ParmVarDecl>(VD);
+  if (!PD) return;
+
+  if (!NSErrorII)
+    NSErrorII = &PD->getASTContext().Idents.get("NSError");
+  if (!CFErrorII)
+    CFErrorII = &PD->getASTContext().Idents.get("CFErrorRef");
+
+  if (ShouldCheckNSError && IsNSError(PD, NSErrorII)) {
+    setFlag<NSErrorOut>(state, state->getSVal(cast<Loc>(loc)), C);
+    return;
+  }
+
+  if (ShouldCheckCFError && IsCFError(PD, CFErrorII)) {
+    setFlag<CFErrorOut>(state, state->getSVal(cast<Loc>(loc)), C);
+    return;
+  }
+}
+
+void NSOrCFErrorDerefChecker::checkEvent(ImplicitNullDerefEvent event) const {
+  if (event.IsLoad)
     return;
 
-  if (ResultTy == Ctx.VoidTy) EmitRetTyWarning(BR, CodeDecl);
+  SVal loc = event.Location;
+  const GRState *state = event.SinkNode->getState();
+  BugReporter &BR = *event.BR;
 
-  for (ExplodedGraph::roots_iterator RI=G.roots_begin(), RE=G.roots_end();
-       RI!=RE; ++RI) {
-    // Scan the parameters for an implicit null dereference.
-    for (llvm::SmallVectorImpl<VarDecl*>::iterator I=ErrorParams.begin(),
-          E=ErrorParams.end(); I!=E; ++I)
-        CheckParamDeref(*I, (*RI)->getLocationContext(), (*RI)->getState(), BR);
-  }
-}
+  bool isNSError = hasFlag<NSErrorOut>(loc, state);
+  bool isCFError = false;
+  if (!isNSError)
+    isCFError = hasFlag<CFErrorOut>(loc, state);
 
-void NSErrorChecker::EmitRetTyWarning(BugReporter& BR, const Decl& CodeDecl) {
-  std::string sbuf;
-  llvm::raw_string_ostream os(sbuf);
+  if (!(isNSError || isCFError))
+    return;
 
-  if (isa<ObjCMethodDecl>(CodeDecl))
-    os << "Method";
+  // Storing to possible null NSError/CFErrorRef out parameter.
+
+  // Emit an error.
+  std::string err;
+  llvm::raw_string_ostream os(err);
+    os << "Potential null dereference.  According to coding standards ";
+
+  if (isNSError)
+    os << "in 'Creating and Returning NSError Objects' the parameter '";
   else
-    os << "Function";
+    os << "documented in CoreFoundation/CFError.h the parameter '";
 
-  os << " accepting ";
-  os << (isNSErrorWarning ? "NSError**" : "CFErrorRef*");
-  os << " should have a non-void return value to indicate whether or not an "
-        "error occurred";
+  os  << "' may be null.";
 
-  BR.EmitBasicReport(isNSErrorWarning
-                     ? "Bad return type when passing NSError**"
-                     : "Bad return type when passing CFError*",
-                     getCategory(), os.str(),
-                     CodeDecl.getLocation());
+  BugType *bug = 0;
+  if (isNSError)
+    bug = new NSErrorDerefBug();
+  else
+    bug = new CFErrorDerefBug();
+  EnhancedBugReport *report = new EnhancedBugReport(*bug, os.str(),
+                                                    event.SinkNode);
+  BR.EmitReport(report);
 }
 
-void
-NSErrorChecker::CheckSignature(const ObjCMethodDecl& M, QualType& ResultTy,
-                             llvm::SmallVectorImpl<VarDecl*>& ErrorParams) {
+static bool IsNSError(const ParmVarDecl *PD, IdentifierInfo *II) {
 
-  ResultTy = M.getResultType();
-
-  for (ObjCMethodDecl::param_iterator I=M.param_begin(),
-       E=M.param_end(); I!=E; ++I)  {
-
-    QualType T = (*I)->getType();
-
-    if (isNSErrorWarning) {
-      if (CheckNSErrorArgument(T)) ErrorParams.push_back(*I);
-    }
-    else if (CheckCFErrorArgument(T))
-      ErrorParams.push_back(*I);
-  }
-}
-
-void
-NSErrorChecker::CheckSignature(const FunctionDecl& F, QualType& ResultTy,
-                             llvm::SmallVectorImpl<VarDecl*>& ErrorParams) {
-
-  ResultTy = F.getResultType();
-
-  for (FunctionDecl::param_const_iterator I = F.param_begin(),
-                                          E = F.param_end(); I != E; ++I)  {
-
-    QualType T = (*I)->getType();
-
-    if (isNSErrorWarning) {
-      if (CheckNSErrorArgument(T)) ErrorParams.push_back(*I);
-    }
-    else if (CheckCFErrorArgument(T))
-      ErrorParams.push_back(*I);
-  }
-}
-
-
-bool NSErrorChecker::CheckNSErrorArgument(QualType ArgTy) {
-
-  const PointerType* PPT = ArgTy->getAs<PointerType>();
+  const PointerType* PPT = PD->getType()->getAs<PointerType>();
   if (!PPT)
     return false;
 
@@ -181,9 +288,8 @@ bool NSErrorChecker::CheckNSErrorArgument(QualType ArgTy) {
   return false;
 }
 
-bool NSErrorChecker::CheckCFErrorArgument(QualType ArgTy) {
-
-  const PointerType* PPT = ArgTy->getAs<PointerType>();
+static bool IsCFError(const ParmVarDecl *PD, IdentifierInfo *II) {
+  const PointerType* PPT = PD->getType()->getAs<PointerType>();
   if (!PPT) return false;
 
   const TypedefType* TT = PPT->getPointeeType()->getAs<TypedefType>();
@@ -192,47 +298,16 @@ bool NSErrorChecker::CheckCFErrorArgument(QualType ArgTy) {
   return TT->getDecl()->getIdentifier() == II;
 }
 
-void NSErrorChecker::CheckParamDeref(const VarDecl *Param,
-                                   const LocationContext *LC,
-                                   const GRState *rootState,
-                                   BugReporter& BR) {
-
-  SVal ParamL = rootState->getLValue(Param, LC);
-  const MemRegion* ParamR = cast<loc::MemRegionVal>(ParamL).getRegionAs<VarRegion>();
-  assert (ParamR && "Parameters always have VarRegions.");
-  SVal ParamSVal = rootState->getSVal(ParamR);
-
-  // FIXME: For now assume that ParamSVal is symbolic.  We need to generalize
-  // this later.
-  SymbolRef ParamSym = ParamSVal.getAsLocSymbol();
-  if (!ParamSym)
-    return;
-
-  // Iterate over the implicit-null dereferences.
-  ExplodedNode *const* I,  *const* E;
-  llvm::tie(I, E) = GetImplicitNullDereferences(Eng);
-  for ( ; I != E; ++I) {
-    const GRState *state = (*I)->getState();
-    SVal location = state->getSVal((*I)->getLocationAs<StmtPoint>()->getStmt());
-    if (location.getAsSymbol() != ParamSym)
-      continue;
-
-    // Emit an error.
-    std::string sbuf;
-    llvm::raw_string_ostream os(sbuf);
-      os << "Potential null dereference.  According to coding standards ";
-
-    if (isNSErrorWarning)
-      os << "in 'Creating and Returning NSError Objects' the parameter '";
-    else
-      os << "documented in CoreFoundation/CFError.h the parameter '";
-
-    os << Param << "' may be null.";
-
-    BugReport *report = new BugReport(*this, os.str(), *I);
-    // FIXME: Notable symbols are now part of the report.  We should
-    //  add support for notable symbols in BugReport.
-    //    BR.addNotableSymbol(SV->getSymbol());
-    BR.EmitReport(report);
-  }
+void ento::registerNSErrorChecker(CheckerManager &mgr) {
+  mgr.registerChecker<NSErrorMethodChecker>();
+  NSOrCFErrorDerefChecker *
+    checker = mgr.registerChecker<NSOrCFErrorDerefChecker>();
+  checker->ShouldCheckNSError = true;
+}
+
+void ento::registerCFErrorChecker(CheckerManager &mgr) {
+  mgr.registerChecker<CFErrorFunctionChecker>();
+  NSOrCFErrorDerefChecker *
+    checker = mgr.registerChecker<NSOrCFErrorDerefChecker>();
+  checker->ShouldCheckCFError = true;
 }

lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

@@ -339,8 +339,6 @@ static void ActionExprEngine(AnalysisConsumer &C, AnalysisManager& mgr,
     return;
   ExprEngine Eng(mgr, TF.take());
 
-  RegisterNSErrorChecks(Eng.getBugReporter(), Eng, *D);
-
   // Set the graph auditor.
   llvm::OwningPtr<ExplodedNode::Auditor> Auditor;
   if (mgr.shouldVisualizeUbigraph()) {

test/Analysis/CheckNSError.m

@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=basic -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=basic -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=range -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=range -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,cocoa.NSError,macosx.CFError -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=basic -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,cocoa.NSError,macosx.CFError -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=basic -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,cocoa.NSError,macosx.CFError -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=range -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,cocoa.NSError,macosx.CFError -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=range -verify %s
 
 
 typedef signed char BOOL;

test/Analysis/bstring.c

@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core.experimental.CString -analyzer-check-objc-mem -analyzer-store=region -verify %s
-// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core.experimental.CString -analyzer-check-objc-mem -analyzer-store=region -verify %s
-// RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core.experimental.CString -analyzer-check-objc-mem -analyzer-store=region -verify %s
-// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=core.experimental.CString -analyzer-check-objc-mem -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,core.experimental.CString -analyzer-check-objc-mem -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core,core.experimental.CString -analyzer-check-objc-mem -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core,core.experimental.CString -analyzer-check-objc-mem -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,core.experimental.CString -analyzer-check-objc-mem -analyzer-store=region -verify %s
 
 //===----------------------------------------------------------------------===
 // Declarations

test/Analysis/complex.c

@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=basic -verify -Wno-unreachable-code -ffreestanding %s
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=range -verify -Wno-unreachable-code -ffreestanding %s
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=basic -verify -Wno-unreachable-code -ffreestanding %s
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=range -verify -Wno-unreachable-code -ffreestanding %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=basic -verify -Wno-unreachable-code -ffreestanding %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=range -verify -Wno-unreachable-code -ffreestanding %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=basic -verify -Wno-unreachable-code -ffreestanding %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=range -verify -Wno-unreachable-code -ffreestanding %s
 
 #include <stdint.h>
 

test/Analysis/constant-folding.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core.experimental.UnreachableCode -analyzer-check-objc-mem -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,core.experimental.UnreachableCode -analyzer-check-objc-mem -verify %s
 
 // Trigger a warning if the analyzer reaches this point in the control flow.
 #define WARN ((void)*(char*)0)

test/Analysis/dtor.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store region -analyzer-inline-call -cfg-add-implicit-dtors -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-check-objc-mem -analyzer-store region -analyzer-inline-call -cfg-add-implicit-dtors -verify %s
 
 class A {
 public:

test/Analysis/flat-store.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=flat -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-check-objc-mem -analyzer-store=flat -verify %s
 #define FAIL ((void)*(char*)0)
 struct simple { int x; };
 

test/Analysis/idempotent-operations-limited-loops.c

@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-checker=core.experimental.IdempotentOps -analyzer-max-loop 3 -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-checker=core.experimental.IdempotentOps -analyzer-max-loop 4 -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-checker=core.experimental.IdempotentOps %s -verify
+// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-checker=core,core.experimental.IdempotentOps -analyzer-max-loop 3 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-checker=core,core.experimental.IdempotentOps -analyzer-max-loop 4 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-checker=core,core.experimental.IdempotentOps %s -verify
 
 void always_warning() { int *p = 0; *p = 0xDEADBEEF; } // expected-warning{{Dereference of null pointer (loaded from variable 'p')}}
 

test/Analysis/inline.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-inline-call -analyzer-store region -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-check-objc-mem -analyzer-inline-call -analyzer-store region -verify %s
 
 int test1_f1() {
   int y = 1;

test/Analysis/misc-ps-basic-store.m

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core.experimental -analyzer-check-objc-mem -analyzer-store=basic -verify -fblocks %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,core.experimental -analyzer-check-objc-mem -analyzer-store=basic -verify -fblocks %s
 
 //---------------------------------------------------------------------------
 // Test case 'checkaccess_union' differs for region store and basic store.

test/Analysis/misc-ps-region-store.cpp

@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core.experimental -analyzer-check-objc-mem -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core.experimental -analyzer-check-objc-mem -analyzer-store=region -verify -fblocks   -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core,core.experimental -analyzer-check-objc-mem -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,core.experimental -analyzer-check-objc-mem -analyzer-store=region -verify -fblocks   -analyzer-opt-analyze-nested-blocks %s
 
 // Test basic handling of references.
 char &test1_aux();

test/Analysis/override-werror.c

@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core.experimental -analyzer-check-objc-mem -Werror %s -analyzer-store=basic -verify
-// RUN: %clang_cc1 -analyze -analyzer-checker=core.experimental -analyzer-check-objc-mem -Werror %s -analyzer-store=region -verify
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,core.experimental -analyzer-check-objc-mem -Werror %s -analyzer-store=basic -verify
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,core.experimental -analyzer-check-objc-mem -Werror %s -analyzer-store=region -verify
 
 // This test case illustrates that using '-analyze' overrides the effect of
 // -Werror.  This allows basic warnings not to interfere with producing

test/Analysis/plist-output-alternate.m

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core.experimental -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-output=plist -o - %s | FileCheck %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,core.experimental -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-output=plist -o - %s | FileCheck %s
 
 void test_null_init(void) {
   int *p = 0;

test/Analysis/retain-release-region-store.m

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store=region -analyzer-max-loop 6 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-check-objc-mem -analyzer-store=region -analyzer-max-loop 6 -verify %s
 
 //===----------------------------------------------------------------------===//
 // The following code is reduced using delta-debugging from

test/Analysis/string.c

@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core.experimental.CString,core.experimental.UnreachableCode -analyzer-check-objc-mem -analyzer-store=region -verify %s
-// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core.experimental.CString,core.experimental.UnreachableCode -analyzer-check-objc-mem -analyzer-store=region -verify %s
-// RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core.experimental.CString,core.experimental.UnreachableCode -analyzer-check-objc-mem -analyzer-store=region -verify %s
-// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=core.experimental.CString,core.experimental.UnreachableCode -analyzer-check-objc-mem -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,core.experimental.CString,core.experimental.UnreachableCode -analyzer-check-objc-mem -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core,core.experimental.CString,core.experimental.UnreachableCode -analyzer-check-objc-mem -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core,core.experimental.CString,core.experimental.UnreachableCode -analyzer-check-objc-mem -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,core.experimental.CString,core.experimental.UnreachableCode -analyzer-check-objc-mem -analyzer-store=region -verify %s
 
 //===----------------------------------------------------------------------===
 // Declarations

test/Analysis/unreachable-code-path.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=DeadStores,core.experimental.UnreachableCode -analyzer-check-objc-mem -verify -analyzer-opt-analyze-nested-blocks -Wno-unused-value %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,DeadStores,core.experimental.UnreachableCode -analyzer-check-objc-mem -verify -analyzer-opt-analyze-nested-blocks -Wno-unused-value %s
 
 extern void foo(int a);
 

test/Coverage/html-diagnostics.c

@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-check-objc-mem -o %t %s
+// RUN: %clang_cc1 -analyze -analyzer-output=html -analyzer-checker=core -analyzer-check-objc-mem -o %t %s
 // RUN: cat %t/*.html | FileCheck %s
 
 // CHECK: <h3>Annotated Source Code</h3>