[analyzer] Reimplement dependencies between checkers

Unfortunately, up until now, the fact that certain checkers depended on one
another was known, but how these actually unfolded was hidden deep within the
implementation. For example, many checkers (like RetainCount, Malloc or CString)
modelled a certain functionality, and exposed certain reportable bug types to
the user. For example, while MallocChecker models many many different types of
memory handling, the actual "unix.MallocChecker" checker the user was exposed to
was merely and option to this modeling part.

Other than this being an ugly mess, this issue made resolving the checker naming
issue almost impossible. (The checker naming issue being that if a checker
registered more than one checker within its registry function, both checker
object recieved the same name) Also, if the user explicitly disabled a checker
that was a dependency of another that _was_ explicitly enabled, it implicitly,
without "telling" the user, reenabled it.

Clearly, changing this to a well structured, declarative form, where the
handling of dependencies are done on a higher level is very much preferred.

This patch, among the detailed things later, makes checkers declare their
dependencies within the TableGen file Checkers.td, and exposes the same
functionality to plugins and statically linked non-generated checkers through
CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies,
makes sure that checkers are added to CheckerManager in the correct order,
and makes sure that if a dependency is disabled, so will be every checker that
depends on it.

In detail:

* Add a new field to the Checker class in CheckerBase.td called Dependencies,
which is a list of Checkers.
* Move unix checkers before cplusplus, as there is no forward declaration in
tblgen :/
* Add the following new checkers:
  - StackAddrEscapeBase
  - StackAddrEscapeBase
  - CStringModeling
  - DynamicMemoryModeling (base of the MallocChecker family)
  - IteratorModeling (base of the IteratorChecker family)
  - ValistBase
  - SecuritySyntaxChecker (base of bcmp, bcopy, etc...)
  - NSOrCFErrorDerefChecker (base of NSErrorChecker and  CFErrorChecker)
  - IvarInvalidationModeling (base of IvarInvalidation checker family)
  - RetainCountBase (base of RetainCount and OSObjectRetainCount)
* Clear up and registry functions in MallocChecker, happily remove old FIXMEs.
* Add a new addDependency function to CheckerRegistry.
* Neatly format RUN lines in files I looked at while debugging.

Big thanks to Artem Degrachev for all the guidance through this project!

Differential Revision: https://reviews.llvm.org/D54438

llvm-svn: 352287

13 files changed, 169 insertions, 59 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 360891d8ecb..d37b2c57f58 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2475,6 +2475,14 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR,
   C.addTransition(state);
 }
 
+void ento::registerCStringModeling(CheckerManager &Mgr) {
+  Mgr.registerChecker<CStringChecker>();
+}
+
+bool ento::shouldRegisterCStringModeling(const LangOptions &LO) {
+  return true;
+}
+
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \
     CStringChecker *checker = mgr.registerChecker<CStringChecker>();           \
@@ -2490,7 +2498,3 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR,
   REGISTER_CHECKER(CStringOutOfBounds)
   REGISTER_CHECKER(CStringBufferOverlap)
 REGISTER_CHECKER(CStringNotNullTerm)
-
-  void ento::registerCStringCheckerBasic(CheckerManager &Mgr) {
-    Mgr.registerChecker<CStringChecker>();
-  }
diff --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index bb7c91cf572..4992ece1322 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -28,14 +28,6 @@ using namespace ento;
 
 namespace {
 
-struct ChecksFilter {
-  DefaultBool Check_CallAndMessageUnInitRefArg;
-  DefaultBool Check_CallAndMessageChecker;
-
-  CheckName CheckName_CallAndMessageUnInitRefArg;
-  CheckName CheckName_CallAndMessageChecker;
-};
-
 class CallAndMessageChecker
   : public Checker< check::PreStmt<CallExpr>,
                     check::PreStmt<CXXDeleteExpr>,
@@ -56,7 +48,8 @@ class CallAndMessageChecker
   mutable std::unique_ptr<BugType> BT_call_few_args;
 
 public:
-  ChecksFilter Filter;
+  DefaultBool Check_CallAndMessageUnInitRefArg;
+  CheckName CheckName_CallAndMessageUnInitRefArg;
 
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
   void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
@@ -151,7 +144,7 @@ bool CallAndMessageChecker::uninitRefOrPointer(
     CheckerContext &C, const SVal &V, SourceRange ArgRange, const Expr *ArgEx,
     std::unique_ptr<BugType> &BT, const ParmVarDecl *ParamDecl, const char *BD,
     int ArgumentNumber) const {
-  if (!Filter.Check_CallAndMessageUnInitRefArg)
+  if (!Check_CallAndMessageUnInitRefArg)
     return false;
 
   // No parameter declaration available, i.e. variadic function argument.
@@ -607,17 +600,21 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C,
   C.addTransition(state);
 }
 
-#define REGISTER_CHECKER(name)                                                 \
-  void ento::register##name(CheckerManager &mgr) {                             \
-    CallAndMessageChecker *Checker =                                           \
-        mgr.registerChecker<CallAndMessageChecker>();                          \
-    Checker->Filter.Check_##name = true;                                       \
-    Checker->Filter.CheckName_##name = mgr.getCurrentCheckName();              \
-  }                                                                            \
-                                                                               \
-  bool ento::shouldRegister##name(const LangOptions &LO) {                     \
-    return true;                                                               \
-  }
+void ento::registerCallAndMessageChecker(CheckerManager &mgr) {
+  mgr.registerChecker<CallAndMessageChecker>();
+}
+
+bool ento::shouldRegisterCallAndMessageChecker(const LangOptions &LO) {
+  return true;
+}
 
-REGISTER_CHECKER(CallAndMessageUnInitRefArg)
-REGISTER_CHECKER(CallAndMessageChecker)
+void ento::registerCallAndMessageUnInitRefArg(CheckerManager &mgr) {
+  CallAndMessageChecker *Checker =
+      mgr.registerChecker<CallAndMessageChecker>();
+  Checker->Check_CallAndMessageUnInitRefArg = true;
+  Checker->CheckName_CallAndMessageUnInitRefArg = mgr.getCurrentCheckName();
+}
+
+bool ento::shouldRegisterCallAndMessageUnInitRefArg(const LangOptions &LO) {
+  return true;
+}
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index f192dba2300..383005aa440 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -905,6 +905,14 @@ public:
 };
 }
 
+void ento::registerSecuritySyntaxChecker(CheckerManager &mgr) {
+  mgr.registerChecker<SecuritySyntaxChecker>();
+}
+
+bool ento::shouldRegisterSecuritySyntaxChecker(const LangOptions &LO) {
+  return true;
+}
+
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \
     SecuritySyntaxChecker *checker =                                           \
diff --git a/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h b/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
index 873352b3934..9642588d6a4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
+++ b/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
@@ -16,9 +16,6 @@ class CheckerManager;
 
 namespace ento {
 
-/// Register the checker which evaluates CString API calls.
-void registerCStringCheckerBasic(CheckerManager &Mgr);
-
 /// Register the part of MallocChecker connected to InnerPointerChecker.
 void registerInnerPointerCheckerAux(CheckerManager &Mgr);
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
index 75450271ed3..fbabe335958 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -2393,6 +2393,14 @@ bool compare(ProgramStateRef State, NonLoc NL1, NonLoc NL2,
 
 } // namespace
 
+void ento::registerIteratorModeling(CheckerManager &mgr) {
+  mgr.registerChecker<IteratorChecker>();
+}
+
+bool ento::shouldRegisterIteratorModeling(const LangOptions &LO) {
+  return true;
+}
+
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &Mgr) {                             \
     auto *checker = Mgr.registerChecker<IteratorChecker>();                    \
diff --git a/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
index 0dc78eb0c71..77ec8257989 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
@@ -735,6 +735,14 @@ public:
 };
 } // end anonymous namespace
 
+void ento::registerIvarInvalidationModeling(CheckerManager &mgr) {
+  mgr.registerChecker<IvarInvalidationChecker>();
+}
+
+bool ento::shouldRegisterIvarInvalidationModeling(const LangOptions &LO) {
+  return true;
+}
+
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \
     IvarInvalidationChecker *checker =                                         \
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 24e34dc3816..016aa636f10 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3086,44 +3086,27 @@ markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
 } // end namespace ento
 } // end namespace clang
 
-void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) {
-  registerCStringCheckerBasic(mgr);
-  MallocChecker *checker = mgr.registerChecker<MallocChecker>();
-  checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
-      "Optimistic", false, checker);
-  checker->ChecksEnabled[MallocChecker::CK_NewDeleteLeaksChecker] = true;
-  checker->CheckNames[MallocChecker::CK_NewDeleteLeaksChecker] =
-      mgr.getCurrentCheckName();
-  // We currently treat NewDeleteLeaks checker as a subchecker of NewDelete
-  // checker.
-  if (!checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker]) {
-    checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker] = true;
-    // FIXME: This does not set the correct name, but without this workaround
-    //        no name will be set at all.
-    checker->CheckNames[MallocChecker::CK_NewDeleteChecker] =
-        mgr.getCurrentCheckName();
-  }
-}
-
-bool ento::shouldRegisterNewDeleteLeaksChecker(const LangOptions &LO) {
-  return true;
-}
-
 // Intended to be used in InnerPointerChecker to register the part of
 // MallocChecker connected to it.
 void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) {
-    registerCStringCheckerBasic(mgr);
     MallocChecker *checker = mgr.registerChecker<MallocChecker>();
     checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
-        "Optimistic", false, checker);
+                                                  "Optimistic", false, checker);
     checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true;
     checker->CheckNames[MallocChecker::CK_InnerPointerChecker] =
         mgr.getCurrentCheckName();
 }
 
+void ento::registerDynamicMemoryModeling(CheckerManager &mgr) {
+  mgr.registerChecker<MallocChecker>();
+}
+
+bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) {
+  return true;
+}
+
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &mgr) {                             \
-    registerCStringCheckerBasic(mgr);                                          \
     MallocChecker *checker = mgr.registerChecker<MallocChecker>();             \
     checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(  \
         "Optimistic", false, checker);                                         \
@@ -3137,4 +3120,5 @@ void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) {
 
 REGISTER_CHECKER(MallocChecker)
 REGISTER_CHECKER(NewDeleteChecker)
+REGISTER_CHECKER(NewDeleteLeaksChecker)
 REGISTER_CHECKER(MismatchedDeallocatorChecker)
diff --git a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
index d92c0f09cb6..3520b12933e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -307,6 +307,14 @@ static bool IsCFError(QualType T, IdentifierInfo *II) {
   return TT->getDecl()->getIdentifier() == II;
 }
 
+void ento::registerNSOrCFErrorDerefChecker(CheckerManager &mgr) {
+  mgr.registerChecker<NSOrCFErrorDerefChecker>();
+}
+
+bool ento::shouldRegisterNSOrCFErrorDerefChecker(const LangOptions &LO) {
+  return true;
+}
+
 void ento::registerNSErrorChecker(CheckerManager &mgr) {
   mgr.registerChecker<NSErrorMethodChecker>();
   NSOrCFErrorDerefChecker *checker =
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 50f0edd2ebc..7d86f67090c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -1191,6 +1191,14 @@ void NullabilityChecker::printState(raw_ostream &Out, ProgramStateRef State,
   }
 }
 
+void ento::registerNullabilityBase(CheckerManager &mgr) {
+  mgr.registerChecker<NullabilityChecker>();
+}
+
+bool ento::shouldRegisterNullabilityBase(const LangOptions &LO) {
+  return true;
+}
+
 #define REGISTER_CHECKER(name, trackingRequired)                               \
   void ento::register##name##Checker(CheckerManager &mgr) {                    \
     NullabilityChecker *checker = mgr.registerChecker<NullabilityChecker>();   \
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index e67cdda4df8..5d8e7a83e3d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1455,6 +1455,14 @@ void RetainCountChecker::printState(raw_ostream &Out, ProgramStateRef State,
 // Checker registration.
 //===----------------------------------------------------------------------===//
 
+void ento::registerRetainCountBase(CheckerManager &Mgr) {
+  Mgr.registerChecker<RetainCountChecker>();
+}
+
+bool ento::shouldRegisterRetainCountBase(const LangOptions &LO) {
+  return true;
+}
+
 void ento::registerRetainCountChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.registerChecker<RetainCountChecker>();
   Chk->TrackObjCAndCFObjects = true;
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 4a9dbe46505..f6bb7375d17 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -359,6 +359,14 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
   }
 }
 
+void ento::registerStackAddrEscapeBase(CheckerManager &mgr) {
+  mgr.registerChecker<StackAddrEscapeChecker>();
+}
+
+bool ento::shouldRegisterStackAddrEscapeBase(const LangOptions &LO) {
+  return true;
+}
+
 #define REGISTER_CHECKER(name) \
   void ento::register##name(CheckerManager &Mgr) { \
     StackAddrEscapeChecker *Chk = \
diff --git a/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
index b47eadba8e6..d3eb5063120 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -399,6 +399,14 @@ std::shared_ptr<PathDiagnosticPiece> ValistChecker::ValistBugVisitor::VisitNode(
   return std::make_shared<PathDiagnosticEventPiece>(Pos, Msg, true);
 }
 
+void ento::registerValistBase(CheckerManager &mgr) {
+  mgr.registerChecker<ValistChecker>();
+}
+
+bool ento::shouldRegisterValistBase(const LangOptions &LO) {
+  return true;
+}
+
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name##Checker(CheckerManager &mgr) {                    \
     ValistChecker *checker = mgr.registerChecker<ValistChecker>();             \
diff --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
index 8850b2895c9..68776d94209 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -151,6 +151,15 @@ CheckerRegistry::CheckerRegistry(
   // that's ASCIIbetically last?
   llvm::sort(Checkers, checkerNameLT);
 
+#define GET_CHECKER_DEPENDENCIES
+
+#define CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY)                               \
+  addDependency(FULLNAME, DEPENDENCY);
+
+#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#undef CHECKER_DEPENDENCY
+#undef GET_CHECKER_DEPENDENCIES
+
   // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
   // command line.
   for (const std::pair<std::string, bool> &opt : AnOpts.CheckersControlList) {
@@ -169,13 +178,69 @@ CheckerRegistry::CheckerRegistry(
   }
 }
 
+/// Collects dependencies in \p ret, returns false on failure.
+static bool collectDependenciesImpl(
+                              const CheckerRegistry::ConstCheckerInfoList &deps,
+                              const LangOptions &LO,
+                              CheckerRegistry::CheckerInfoSet &ret);
+
+/// Collects dependenies in \p enabledCheckers. Return None on failure.
+LLVM_NODISCARD
+static llvm::Optional<CheckerRegistry::CheckerInfoSet> collectDependencies(
+     const CheckerRegistry::CheckerInfo &checker, const LangOptions &LO) {
+
+  CheckerRegistry::CheckerInfoSet ret;
+  // Add dependencies to the enabled checkers only if all of them can be
+  // enabled.
+  if (!collectDependenciesImpl(checker.Dependencies, LO, ret))
+    return None;
+
+  return ret;
+}
+
+static bool collectDependenciesImpl(
+                              const CheckerRegistry::ConstCheckerInfoList &deps,
+                              const LangOptions &LO,
+                              CheckerRegistry::CheckerInfoSet &ret) {
+
+  for (const CheckerRegistry::CheckerInfo *dependency : deps) {
+
+    if (dependency->isDisabled(LO))
+      return false;
+
+    // Collect dependencies recursively.
+    if (!collectDependenciesImpl(dependency->Dependencies, LO, ret))
+      return false;
+
+    ret.insert(dependency);
+  }
+
+  return true;
+}
+
 CheckerRegistry::CheckerInfoSet CheckerRegistry::getEnabledCheckers() const {
 
   CheckerInfoSet enabledCheckers;
 
   for (const CheckerInfo &checker : Checkers) {
-    if (checker.isEnabled(LangOpts))
-      enabledCheckers.insert(&checker);
+    if (!checker.isEnabled(LangOpts))
+      continue;
+
+    // Recursively enable it's dependencies.
+    llvm::Optional<CheckerInfoSet> deps =
+        collectDependencies(checker, LangOpts);
+
+    if (!deps) {
+      // If we failed to enable any of the dependencies, don't enable this
+      // checker.
+      continue;
+    }
+
+    // Note that set_union also preserves the order of insertion.
+    enabledCheckers.set_union(*deps);
+
+    // Enable the checker.
+    enabledCheckers.insert(&checker);
   }
 
   return enabledCheckers;
@@ -196,7 +261,6 @@ void CheckerRegistry::addChecker(InitializationFunction Rfn,
 }
 
 void CheckerRegistry::initializeManager(CheckerManager &checkerMgr) const {
-
   // Collect checkers enabled by the options.
   CheckerInfoSet enabledCheckers = getEnabledCheckers();