[analyzer] Fix an assertion failure if plugins added dependencies

[Szelethus]

Summary

Ideally, there is no reason behind not being able to depend on checkers that come from a different plugin (or on builtin checkers) -- however, this is only possible if all checkers are added to the registry before resolving checker dependencies. Since I used a binary search in my addDependency method, this also resulted in an assertion failure (due to CheckerRegistry::Checkers not being sorted), since the function used by plugins to register their checkers (clang_registerCheckers) calls addDependency.

This patch resolves this issue by only noting which dependencies have to established when addDependency is called, and resolves them at a later stage when no more checkers are added to the registry.

[Szelethus]

Don't let the testcase fool you, it's merely added because there wasn't any for that particular case, I still need to add the plugin, but that moves quite a few files around, so I might separate that to another patch.

edit: Added it in D59464.

[baloghadamsoftware]

This one seems straightforward.

include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

private:
   /// Return an iterator range of mutable CheckerInfos \p CmdLineArg applies to.
   /// For example, it'll return the checkers for the core package, if
   /// \p CmdLineArg is "core".
   CheckerInfoListRange getMutableCheckersForCmdLineArg(StringRef CmdLineArg);
 
   CheckerInfoList Checkers;
   llvm::StringMap<size_t> PackageSizes;
 
+  /// Contains all (Dependendent checker, Dependency) pairs. We need this, as
+  /// we'll resolve dependencies after all checkers were added first.
+  llvm::SmallVector<std::pair<StringRef, StringRef>, 0> Dependencies;
+
+  void resolveDependencies();
+
   DiagnosticsEngine &Diags;
   AnalyzerOptions &AnOpts;
   const LangOptions &LangOpts;
 };
 
 } // namespace ento
 
 } // namespace clang
 
 #endif // LLVM_CLANG_STATICANALYZER_CORE_CHECKERREGISTRY_H

lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

 
 #define CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY)                               \
   addDependency(FULLNAME, DEPENDENCY);
 
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER_DEPENDENCY
 #undef GET_CHECKER_DEPENDENCIES
 
+  resolveDependencies();
+
   // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
   // command line.
   for (const std::pair<std::string, bool> &Opt : AnOpts.CheckersControlList) {
     CheckerInfoListRange CheckerForCmdLineArg =
         getMutableCheckersForCmdLineArg(Opt.first);
 
     if (CheckerForCmdLineArg.begin() == CheckerForCmdLineArg.end()) {
       Diags.Report(diag::err_unknown_analyzer_checker) << Opt.first;
void CheckerRegistry::addChecker(InitializationFunction Rfn,
   StringRef PackageName, LeafName;
   std::tie(PackageName, LeafName) = Name.rsplit(PackageSeparator);
   while (!LeafName.empty()) {
     PackageSizes[PackageName] += 1;
     std::tie(PackageName, LeafName) = PackageName.rsplit(PackageSeparator);
   }
 }
 
-void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
-  auto CheckerIt = binaryFind(Checkers, FullName);
-  assert(CheckerIt != Checkers.end() && CheckerIt->FullName == FullName &&
+void CheckerRegistry::resolveDependencies() {
+  for (const std::pair<StringRef, StringRef> &Entry : Dependencies) {
+    auto CheckerIt = binaryFind(Checkers, Entry.first);
+    assert(CheckerIt != Checkers.end() && CheckerIt->FullName == Entry.first &&
            "Failed to find the checker while attempting to set up its "
            "dependencies!");
 
-  auto DependencyIt = binaryFind(Checkers, Dependency);
+    auto DependencyIt = binaryFind(Checkers, Entry.second);
     assert(DependencyIt != Checkers.end() &&
-         DependencyIt->FullName == Dependency &&
+           DependencyIt->FullName == Entry.second &&
            "Failed to find the dependency of a checker!");
 
     CheckerIt->Dependencies.emplace_back(&*DependencyIt);
   }
 
+  Dependencies.clear();
+}
+
+void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
+  Dependencies.emplace_back(FullName, Dependency);
+}
+
 void CheckerRegistry::initializeManager(CheckerManager &CheckerMgr) const {
   // Collect checkers enabled by the options.
   CheckerInfoSet enabledCheckers = getEnabledCheckers();
 
   // Initialize the CheckerManager with all enabled checkers.
   for (const auto *Checker : enabledCheckers) {
     CheckerMgr.setCurrentCheckName(CheckName(Checker->FullName));
     Checker->Initialize(CheckerMgr);

test/Analysis/checker-dependencies.c

 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=osx.cocoa.RetainCount \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-ENABLED
+
+// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCountBase
+// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCount
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=osx.cocoa.RetainCount \
+// RUN:   -analyzer-disable-checker=osx.cocoa.RetainCountBase \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-DISABLED
+
+// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCountBase
+// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCount