[analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

[Szelethus]

Summary

Checker dependencies were added D54438 to solve a bug where the checker names were incorrectly registered, for example, InnerPointerChecker would incorrectly emit diagnostics under the name MallocChecker, or vice versa [1]. Since the system over the course of about a year matured, our expectations of what a role of a dependency and a dependent checker should be crystallized a bit more -- D77474 and its summary, as well as a variety of patches in the stack demonstrates how we try to keep dependencies to play a purely modeling role. In fact, D78126 outright forbids diagnostics under a dependency checkers name.

These dependencies ensured the registration order and enabling only when all dependencies are satisfied. This was a very "strong" contract however, that doesn't fit the dependency added in D79420. As its summary suggests, this relation is directly in between diagnostics, not modeling -- we'd prefer a more specific warning over a general one.

To support this, I added a new dependency kind, weak dependencies. These are not as strict of a contract, they only express a preference in registration order. If a weak dependency isn't satisfied, the checker may still be enabled, but if it is, checker registration, and transitively, checker callback evaluation order is ensured.

If you are not familiar with the TableGen changes, a rather short description can be found in the summary of D75360. A lengthier one is in D58065. Of course, this is a rather rarely touched part of the analyzer and I'm happy to spill the beans if any part of it is unclear!

[1] https://www.youtube.com/watch?v=eqKeqHRAhQM

[NoQ]

I think this patch is a fairly historic moment to celebratte.

checker callback evaluation order is ensured

And it will be the same for all callbacks, right? Like, we're not doing this thing yet where it intentionally goes like

chk1->checkPreCall();
chk2->checkPreCall();
chk2->checkPostCall();
chk1->checkPostCall();

?

clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td

144–145

I wouldn't mind having predictable callback evaluation order for modeling as well. What's causing you to drop this scenario?

[Szelethus]

checker callback evaluation order is ensured

And it will be the same for all callbacks, right? Like, we're not doing this thing yet where it intentionally goes like

chk1->checkPreCall();
chk2->checkPreCall();
chk2->checkPostCall();
chk1->checkPostCall();

?

It should work for all callbacks. It'd be a nightmare to test and assert, but when CheckerManager registers a checker, all of its callbacks are registered to the appropriate container, which isn't sorted.

NoQ added a reviewer: vsavchenko.

Woops O:)

[Szelethus]

Mind that strong dependencies also ensure the order of evaluation, but that is a guarantee (either the dependency will be evaluated before the dependency, or none of them will be evaluated), and this patch is about preference (if both of them are enabled, the dependency will be evaluated first).

[NoQ]

+Nithin because it may be relevant to the smart pointer checker if inter-checker communication turns out to be necessary (eg., with the move checker).

[Szelethus]

In D80905#2068320, @NoQ wrote:

+Nithin because it may be relevant to the smart pointer checker if inter-checker communication turns out to be necessary (eg., with the move checker).

If checkers rely on one another, weak dependencies aren't the answer, but rather the guarantee strong dependencies grant. Weak dependencies could be useful however if reports from the checker would be hidden by, say, NullDereferenceChecker.

[Szelethus]

Gentle ping

clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td

144–145

D80905#2066219

[NoQ]

clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td

144–145

Ok, so do i understand it correctly that the only reason you're omitting the "and modeling" clause here is because in your ultimate vision (which i completely agree with) checkers either do modeling or checking, and modeling-checkers never need to be disabled to begin with, so weak dependencies are useless for them, which is why this whole patch is entirely about diagnostics?

[Szelethus]

clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td

144–145

Pretty much, yes. Its not just useless -- I don't think establishing a preferred evaluation order over guaranteed for modeling checkers could ever be correct. Not only that, I think adding new kinds of dependencies must be very well justified to counterweight the extra complexity, they should be intuitive and "just work". There would be a lot of corner cases if these dependencies could be too intertwined, and not only would they be a mess to implement, it could be confusing on the checker developer's end.

[martong]

Thank you Kristof for working on this. Looks good to me as it is!

clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td

 ///     HelpText<"Check for dereferences of null pointers">;
 class Checker<string name = ""> {
   string              CheckerName = name;
   string              HelpText;
   // This field is optional.
   list<CmdLineOption> CheckerOptions;
   // This field is optional.
   list<Checker>       Dependencies;
+  // This field is optional.
+  list<Checker>       WeakDependencies;
   bits<2>             Documentation;
   Package             ParentPackage;
   bit                 Hidden = 0;
 }
 
 /// Describes a list of checker options.
 class CheckerOptions<list<CmdLineOption> opts> {
   list<CmdLineOption> CheckerOptions = opts;
 }
 
-/// Describes dependencies in between checkers. For example, InnerPointerChecker
-/// relies on information MallocBase gathers.
+/// Describes (strong) dependencies in between checkers. This is important for
+/// modeling checkers, for example, MallocBase depends on the proper modeling of
+/// string operations, so it depends on CStringBase. A checker may only be
+/// enabled if none of its dependencies (transitively) is disabled. Dependencies
+/// are always registered before the dependent checker, and its checker
+/// callbacks are also evaluated sooner.
+/// One may only depend on a purely modeling checker (that emits no diagnostis).
 /// Example:
 ///   def InnerPointerChecker : Checker<"InnerPointer">,
 ///     HelpText<"Check for inner pointers of C++ containers used after "
 ///              "re/deallocation">,
 ///     Dependencies<[MallocBase]>;
 class Dependencies<list<Checker> Deps = []> {
   list<Checker> Dependencies = Deps;
 }
 
+/// Describes preferred registration and evaluation order in between checkers.
+/// Unlike strong dependencies, this expresses dependencies in between
+/// diagnostics, and *not* modeling. In the case of an unsatisfied (disabled)
+/// weak dependency, the dependent checker might still be registered. If the
+/// weak dependency is satisfied, it'll be registered, and its checker
+/// callbacks will be evaluated before the dependent checker. This can be used
+/// to ensure that a more specific warning would be displayed in place of a
+/// generic one, should multiple checkers detect the same bug. For example,
+/// non-null parameter bugs are detected by NonNullParamChecker due to the
+/// nonnull attribute, and StdLibraryFunctionsChecker as it models standard
+/// functions, and the former is the more specific one. While freeing a
+/// dangling pointer is a bug, if it is also a double free, we would like to
+/// recognize it as such first and foremost. This works best for fatal error
+/// node generation, otherwise both warnings may be present and in any order.
+class WeakDependencies<list<Checker> Deps = []> {
+  list<Checker> WeakDependencies = Deps;
+}
+
 /// Marks a checker or a package hidden. Hidden entries are meant for developers
 /// only, and aren't exposed to end users.
 class Hidden { bit Hidden = 1; }

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

 //===----------------------------------------------------------------------===//
 // APIModeling.
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = APIModeling in {
 
 def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
   HelpText<"Improve modeling of the C standard library functions">,
-  Dependencies<[NonNullParamChecker, CallAndMessageModeling]>,
+  Dependencies<[CallAndMessageModeling]>,
   CheckerOptions<[
     CmdLineOption<Boolean,
                   "DisplayLoadedSummaries",
                   "If set to true, the checker displays the found summaries "
                   "for the given translation unit.",
                   "false",
                   Released>
   ]>,
   Documentation<NotDocumented>;
 
 def TrustNonnullChecker : Checker<"TrustNonnull">,
   HelpText<"Trust that returns from framework methods annotated with _Nonnull "
            "are not null">,
   Documentation<NotDocumented>;
 
 } // end "apiModeling"
 
 let ParentPackage = APIModelingAlpha in {
 
 def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
   HelpText<"Check constraints of arguments of C standard library functions, "
            "such as whether the parameter of isalpha is in the range [0, 255] "
            "or is EOF.">,
   Dependencies<[StdCLibraryFunctionsChecker]>,
+  WeakDependencies<[NonNullParamChecker]>,
   Documentation<NotDocumented>;
 
 } // end "alpha.apiModeling"
 
 //===----------------------------------------------------------------------===//
 // Evaluate "builtin" functions.
 //===----------------------------------------------------------------------===//
 

clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

struct CheckerInfo {
     StringRef FullName;
     StringRef Desc;
     StringRef DocumentationUri;
     CmdLineOptionList CmdLineOptions;
     bool IsHidden = false;
     StateFromCmdLine State = StateFromCmdLine::State_Unspecified;
 
     ConstCheckerInfoList Dependencies;
+    ConstCheckerInfoList WeakDependencies;
 
     bool isEnabled(const CheckerManager &mgr) const {
       return State == StateFromCmdLine::State_Enabled && ShouldRegister(mgr);
     }
 
     bool isDisabled(const CheckerManager &mgr) const {
       return State == StateFromCmdLine::State_Disabled || !ShouldRegister(mgr);
     }
void addChecker(StringRef FullName, StringRef Desc, StringRef DocsUri,
                   bool IsHidden = false) {
     // Avoid MSVC's Compiler Error C2276:
     // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
     addChecker(&CheckerRegistry::initializeManager<CheckerManager, T>,
                &CheckerRegistry::returnTrue<T>, FullName, Desc, DocsUri,
                IsHidden);
   }
 
-  /// Makes the checker with the full name \p fullName depends on the checker
+  /// Makes the checker with the full name \p fullName depend on the checker
   /// called \p dependency.
   void addDependency(StringRef FullName, StringRef Dependency);
 
+  /// Makes the checker with the full name \p fullName weak depend on the
+  /// checker called \p dependency.
+  void addWeakDependency(StringRef FullName, StringRef Dependency);
+
   /// Registers an option to a given checker. A checker option will always have
   /// the following format:
   ///   CheckerFullName:OptionName=Value
   /// And can be specified from the command line like this:
   ///   -analyzer-config CheckerFullName:OptionName=Value
   ///
   /// Options for unknown checkers, or unknown options for a given checker, or
   /// invalid value types for that given option are reported as an error in
private:
   PackageInfoList Packages;
   /// Used for couting how many checkers belong to a certain package in the
   /// \c Checkers field. For convenience purposes.
   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();
+  llvm::SmallVector<std::pair<StringRef, StringRef>, 0> WeakDependencies;
+
+  template <bool IsWeak> void resolveDependencies();
 
   /// Contains all (FullName, CmdLineOption) pairs. Similarly to dependencies,
   /// we only modify the actual CheckerInfo and PackageInfo objects once all
   /// of them have been added.
   llvm::SmallVector<std::pair<StringRef, CmdLineOption>, 0> PackageOptions;
   llvm::SmallVector<std::pair<StringRef, CmdLineOption>, 0> CheckerOptions;
 
   void resolveCheckerAndPackageOptions();

clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

for (const CmdLineOption &Option : CmdLineOptions) {
     Out << "    ";
     Option.dumpToStream(Out);
     Out << '\n';
   }
   Out << "  Dependencies:\n";
   for (const CheckerInfo *Dependency : Dependencies) {
     Out << "  " << Dependency->FullName << '\n';
   }
+  Out << "  Weak dependencies:\n";
+  for (const CheckerInfo *Dependency : WeakDependencies) {
+    Out << "    " << Dependency->FullName << '\n';
+  }
 }
 
 LLVM_DUMP_METHOD void
 CheckerRegistry::PackageInfo::dumpToStream(llvm::raw_ostream &Out) const {
   Out << FullName << "\n";
   Out << "  Options:\n";
   for (const CmdLineOption &Option : CmdLineOptions) {
     Out << "    ";
#undef GET_PACKAGES
   llvm::sort(Packages, PackageNameLT{});
   llvm::sort(Checkers, CheckerNameLT{});
 
 #define GET_CHECKER_DEPENDENCIES
 
 #define CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY)                               \
   addDependency(FULLNAME, DEPENDENCY);
 
+#define GET_CHECKER_WEAK_DEPENDENCIES
+
+#define CHECKER_WEAK_DEPENDENCY(FULLNAME, DEPENDENCY)                          \
+  addWeakDependency(FULLNAME, DEPENDENCY);
+
 #define GET_CHECKER_OPTIONS
 #define CHECKER_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL,             \
                        DEVELOPMENT_STATUS, IS_HIDDEN)                          \
   addCheckerOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC,                 \
                    DEVELOPMENT_STATUS, IS_HIDDEN);
 
 #define GET_PACKAGE_OPTIONS
 #define PACKAGE_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL,             \
                        DEVELOPMENT_STATUS, IS_HIDDEN)                          \
   addPackageOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC,                 \
                    DEVELOPMENT_STATUS, IS_HIDDEN);
 
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER_DEPENDENCY
 #undef GET_CHECKER_DEPENDENCIES
+#undef CHECKER_WEAK_DEPENDENCY
+#undef GET_CHECKER_WEAK_DEPENDENCIES
 #undef CHECKER_OPTION
 #undef GET_CHECKER_OPTIONS
 #undef PACKAGE_OPTION
 #undef GET_PACKAGE_OPTIONS
 
-  resolveDependencies();
+  resolveDependencies<true>();
+  resolveDependencies<false>();
+
+  for (auto &DepPair : Dependencies) {
+    for (auto &WeakDepPair : WeakDependencies) {
+      // Some assertions to enforce that strong dependencies are relations in
+      // between purely modeling checkers, and weak dependencies are about
+      // diagnostics.
+      assert(WeakDepPair != DepPair &&
+             "A checker cannot strong and weak depend on the same checker!");
+      assert(WeakDepPair.first != DepPair.second &&
+             "A strong dependency mustn't have weak dependencies!");
+      assert(WeakDepPair.second != DepPair.second &&
+             "A strong dependency mustn't be a weak dependency as well!");
+    }
+  }
+
   resolveCheckerAndPackageOptions();
 
   // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
   // command line.
   for (const std::pair<std::string, bool> &Opt : AnOpts.CheckersAndPackages) {
     CheckerInfoListRange CheckerForCmdLineArg =
         getMutableCheckersForCmdLineArg(Opt.first);
 
     if (CheckerForCmdLineArg.begin() == CheckerForCmdLineArg.end()) {
       Diags.Report(diag::err_unknown_analyzer_checker_or_package) << Opt.first;
       Diags.Report(diag::note_suggest_disabling_all_checkers);
     }
 
     for (CheckerInfo &checker : CheckerForCmdLineArg) {
       checker.State = Opt.second ? StateFromCmdLine::State_Enabled
                                  : StateFromCmdLine::State_Disabled;
     }
   }
   validateCheckerOptions();
 }
 
-/// Collects dependenies in \p enabledCheckers. Return None on failure.
-LLVM_NODISCARD
-static llvm::Optional<CheckerRegistry::CheckerInfoSet>
-collectDependencies(const CheckerRegistry::CheckerInfo &checker,
-                    const CheckerManager &Mgr);
+//===----------------------------------------------------------------------===//
+// Dependency resolving.
+//===----------------------------------------------------------------------===//
+
+template <typename IsEnabledFn>
+static bool
+collectStrongDependencies(const CheckerRegistry::ConstCheckerInfoList &Deps,
+                          const CheckerManager &Mgr,
+                          CheckerRegistry::CheckerInfoSet &Ret,
+                          IsEnabledFn IsEnabled);
+
+/// Collects weak dependencies in \p enabledCheckers.
+template <typename IsEnabledFn>
+static void
+collectWeakDependencies(const CheckerRegistry::ConstCheckerInfoList &Deps,
+                        const CheckerManager &Mgr,
+                        CheckerRegistry::CheckerInfoSet &Ret,
+                        IsEnabledFn IsEnabled);
 
 void CheckerRegistry::initializeRegistry(const CheckerManager &Mgr) {
+  // First, we calculate the list of enabled checkers as specified by the
+  // invocation. Weak dependencies will not enable their unspecified strong
+  // depenencies, but its only after resolving strong dependencies for all
+  // checkers when we know whether they will be enabled.
+  CheckerInfoSet Tmp;
+  auto IsEnabledFromCmdLine = [&](const CheckerInfo *Checker) {
+    return !Checker->isDisabled(Mgr);
+  };
   for (const CheckerInfo &Checker : Checkers) {
     if (!Checker.isEnabled(Mgr))
       continue;
 
-    // Recursively enable its dependencies.
-    llvm::Optional<CheckerInfoSet> Deps = collectDependencies(Checker, Mgr);
-
-    if (!Deps) {
+    CheckerInfoSet Deps;
+    if (!collectStrongDependencies(Checker.Dependencies, Mgr, Deps,
+                                   IsEnabledFromCmdLine)) {
       // 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);
+    Tmp.insert(Deps.begin(), Deps.end());
 
     // Enable the checker.
-    EnabledCheckers.insert(&Checker);
-  }
+    Tmp.insert(&Checker);
   }
 
-/// Collects dependencies in \p ret, returns false on failure.
-static bool
-collectDependenciesImpl(const CheckerRegistry::ConstCheckerInfoList &Deps,
-                        const CheckerManager &Mgr,
-                        CheckerRegistry::CheckerInfoSet &Ret);
+  // Calculate enabled checkers with the correct registration order. As this is
+  // done recursively, its arguably cheaper, but for sure less error prone to
+  // recalculate from scratch.
+  auto IsEnabled = [&](const CheckerInfo *Checker) {
+    return llvm::is_contained(Tmp, Checker);
+  };
+  for (const CheckerInfo &Checker : Checkers) {
+    if (!Checker.isEnabled(Mgr))
+      continue;
+
+    CheckerInfoSet Deps;
 
-/// Collects dependenies in \p enabledCheckers. Return None on failure.
-LLVM_NODISCARD
-static llvm::Optional<CheckerRegistry::CheckerInfoSet>
-collectDependencies(const CheckerRegistry::CheckerInfo &checker,
-                    const CheckerManager &Mgr) {
-
-  CheckerRegistry::CheckerInfoSet Ret;
-  // Add dependencies to the enabled checkers only if all of them can be
-  // enabled.
-  if (!collectDependenciesImpl(checker.Dependencies, Mgr, Ret))
-    return None;
+    collectWeakDependencies(Checker.WeakDependencies, Mgr, Deps, IsEnabled);
 
-  return Ret;
+    if (!collectStrongDependencies(Checker.Dependencies, Mgr, Deps,
+                                   IsEnabledFromCmdLine)) {
+      // 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);
+    EnabledCheckers.insert(&Checker);
+  }
+}
+
+template <typename IsEnabledFn>
 static bool
-collectDependenciesImpl(const CheckerRegistry::ConstCheckerInfoList &Deps,
+collectStrongDependencies(const CheckerRegistry::ConstCheckerInfoList &Deps,
                           const CheckerManager &Mgr,
-                        CheckerRegistry::CheckerInfoSet &Ret) {
+                          CheckerRegistry::CheckerInfoSet &Ret,
+                          IsEnabledFn IsEnabled) {
 
   for (const CheckerRegistry::CheckerInfo *Dependency : Deps) {
-
-    if (Dependency->isDisabled(Mgr))
+    if (!IsEnabled(Dependency))
       return false;
 
     // Collect dependencies recursively.
-    if (!collectDependenciesImpl(Dependency->Dependencies, Mgr, Ret))
+    if (!collectStrongDependencies(Dependency->Dependencies, Mgr, Ret,
+                                   IsEnabled))
       return false;
-
     Ret.insert(Dependency);
   }
 
   return true;
 }
 
-void CheckerRegistry::resolveDependencies() {
-  for (const std::pair<StringRef, StringRef> &Entry : Dependencies) {
+template <typename IsEnabledFn>
+static void
+collectWeakDependencies(const CheckerRegistry::ConstCheckerInfoList &WeakDeps,
+                        const CheckerManager &Mgr,
+                        CheckerRegistry::CheckerInfoSet &Ret,
+                        IsEnabledFn IsEnabled) {
+
+  for (const CheckerRegistry::CheckerInfo *Dependency : WeakDeps) {
+    // Don't enable this checker if strong dependencies are unsatisfied, but
+    // assume that weak dependencies are transitive.
+    collectWeakDependencies(Dependency->WeakDependencies, Mgr, Ret, IsEnabled);
+
+    if (IsEnabled(Dependency) &&
+        collectStrongDependencies(Dependency->Dependencies, Mgr, Ret,
+                                  IsEnabled))
+      Ret.insert(Dependency);
+  }
+}
+
+template <bool IsWeak> void CheckerRegistry::resolveDependencies() {
+  for (const std::pair<StringRef, StringRef> &Entry :
+       (IsWeak ? WeakDependencies : 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, Entry.second);
     assert(DependencyIt != Checkers.end() &&
            DependencyIt->FullName == Entry.second &&
            "Failed to find the dependency of a checker!");
 
+    if (IsWeak)
+      CheckerIt->WeakDependencies.emplace_back(&*DependencyIt);
+    else
       CheckerIt->Dependencies.emplace_back(&*DependencyIt);
   }
 }
 
 void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
   Dependencies.emplace_back(FullName, Dependency);
 }
 
+void CheckerRegistry::addWeakDependency(StringRef FullName,
+                                        StringRef Dependency) {
+  WeakDependencies.emplace_back(FullName, Dependency);
+}
+
+//===----------------------------------------------------------------------===//
+// Checker option resolving and validating.
+//===----------------------------------------------------------------------===//
+
 /// Insert the checker/package option to AnalyzerOptions' config table, and
 /// validate it, if the user supplied it on the command line.
 static void insertAndValidate(StringRef FullName,
                               const CheckerRegistry::CmdLineOption &Option,
                               AnalyzerOptions &AnOpts,
                               DiagnosticsEngine &Diags) {
 
   std::string FullOption = (FullName + ":" + Option.OptionName).str();
if (!AnOpts.ShouldEmitErrorsOnInvalidConfigValue)
     return;
 
   using CmdLineOption = CheckerRegistry::CmdLineOption;
 
   auto SameOptName = [SuppliedOption](const CmdLineOption &Opt) {
     return Opt.OptionName == SuppliedOption;
   };
 
-  auto OptionIt = llvm::find_if(OptionList, SameOptName);
+  const auto *OptionIt = llvm::find_if(OptionList, SameOptName);
 
   if (OptionIt == OptionList.end()) {
     Diags.Report(diag::err_analyzer_checker_option_unknown)
         << SuppliedChecker << SuppliedOption;
     return;
   }
 }
 
for (const auto &Config : AnOpts.Config) {
     auto CheckerIt =
         llvm::find(Checkers, CheckerInfo(SuppliedCheckerOrPackage));
     if (CheckerIt != Checkers.end()) {
       isOptionContainedIn(CheckerIt->CmdLineOptions, SuppliedCheckerOrPackage,
                           SuppliedOption, AnOpts, Diags);
       continue;
     }
 
-    auto PackageIt =
+    const auto *PackageIt =
         llvm::find(Packages, PackageInfo(SuppliedCheckerOrPackage));
     if (PackageIt != Packages.end()) {
       isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedCheckerOrPackage,
                           SuppliedOption, AnOpts, Diags);
       continue;
     }
 
     Diags.Report(diag::err_unknown_analyzer_checker_or_package)

clang/test/Analysis/analyzer-enabled-checkers.c

 // RUN: %clang --analyze %s --target=x86_64-pc-linux-gnu \
 // RUN:   -Xclang -analyzer-list-enabled-checkers \
 // RUN:   -Xclang -analyzer-display-progress \
 // RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
 // RUN:                       --implicit-check-not=\.
 
 // CHECK:      OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
-// CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
 // CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
+// CHECK-NEXT: core.NonNullParamChecker
 // CHECK-NEXT: core.NonnilStringConstants
 // CHECK-NEXT: core.NullDereference
 // CHECK-NEXT: core.StackAddrEscapeBase
 // CHECK-NEXT: core.StackAddressEscape
 // CHECK-NEXT: core.UndefinedBinaryOperatorResult
 // CHECK-NEXT: core.VLASize
 // CHECK-NEXT: core.builtin.BuiltinFunctions
 // CHECK-NEXT: core.builtin.NoReturnFunctions

clang/test/Analysis/weak-dependencies.c

+// RUN: %clang_analyze_cc1 %s -verify \
+// RUN:   -analyzer-checker=alpha.apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=core
+
+typedef __typeof(sizeof(int)) size_t;
+
+struct FILE;
+typedef struct FILE FILE;
+
+size_t fread(void *restrict, size_t, size_t, FILE *restrict) __attribute__((nonnull(1)));
+
+void f(FILE *F) {
+  int *p = 0;
+  fread(p, sizeof(int), 5, F); // expected-warning{{Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]}}
+}

clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

void addDep(AnalysisASTConsumer &AnalysisConsumer,
   });
 }
 
 TEST(RegisterDeps, UnsatisfiedDependency) {
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCode<addDep>("void f() {int i;}", Diags));
   EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.RegistrationOrder\n");
 }
+
+//===----------------------------------------------------------------------===//
+// Weak checker dependencies.
+//===----------------------------------------------------------------------===//
+
+UNITTEST_CHECKER(WeakDep, "Weak")
+
+void addWeakDepCheckerBothEnabled(AnalysisASTConsumer &AnalysisConsumer,
+                                  AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.WeakDep", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+void addWeakDepCheckerBothEnabledSwitched(AnalysisASTConsumer &AnalysisConsumer,
+                                          AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.WeakDep", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addWeakDependency("custom.WeakDep", "custom.Dep");
+  });
+}
+
+void addWeakDepCheckerDepDisabled(AnalysisASTConsumer &AnalysisConsumer,
+                                  AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.WeakDep", false},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+void addWeakDepCheckerDepUnspecified(AnalysisASTConsumer &AnalysisConsumer,
+                                     AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+UNITTEST_CHECKER(WeakDep2, "Weak2")
+UNITTEST_CHECKER(Dep2, "Dep2")
+
+void addWeakDepHasWeakDep(AnalysisASTConsumer &AnalysisConsumer,
+                          AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.WeakDep", true},
+                                {"custom.WeakDep2", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addStrongDep(Registry);
+    addWeakDep(Registry);
+    addWeakDep2(Registry);
+    addDep(Registry);
+    addDep2(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+    Registry.addWeakDependency("custom.WeakDep", "custom.WeakDep2");
+  });
+}
+
+void addWeakDepTransitivity(AnalysisASTConsumer &AnalysisConsumer,
+                            AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.WeakDep", false},
+                                {"custom.WeakDep2", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addStrongDep(Registry);
+    addWeakDep(Registry);
+    addWeakDep2(Registry);
+    addDep(Registry);
+    addDep2(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+    Registry.addWeakDependency("custom.WeakDep", "custom.WeakDep2");
+  });
+}
+
+TEST(RegisterDeps, SimpleWeakDependency) {
+  std::string Diags;
+  EXPECT_TRUE(runCheckerOnCode<addWeakDepCheckerBothEnabled>(
+      "void f() {int i;}", Diags));
+  EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.WeakDep\ncustom."
+                   "Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+
+  // Mind that AnalyzerOption listed the enabled checker list in the same order,
+  // but the dependencies are switched.
+  EXPECT_TRUE(runCheckerOnCode<addWeakDepCheckerBothEnabledSwitched>(
+      "void f() {int i;}", Diags));
+  EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.Dep\ncustom."
+                   "RegistrationOrder\ncustom.WeakDep\n");
+  Diags.clear();
+
+  // Weak dependencies dont prevent dependent checkers from being enabled.
+  EXPECT_TRUE(runCheckerOnCode<addWeakDepCheckerDepDisabled>(
+      "void f() {int i;}", Diags));
+  EXPECT_EQ(Diags,
+            "custom.RegistrationOrder:custom.Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+
+  // Nor will they be enabled just because a dependent checker is.
+  EXPECT_TRUE(runCheckerOnCode<addWeakDepCheckerDepUnspecified>(
+      "void f() {int i;}", Diags));
+  EXPECT_EQ(Diags,
+            "custom.RegistrationOrder:custom.Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+
+  EXPECT_TRUE(
+      runCheckerOnCode<addWeakDepTransitivity>("void f() {int i;}", Diags));
+  EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.WeakDep2\ncustom."
+                   "Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+
+  EXPECT_TRUE(
+      runCheckerOnCode<addWeakDepHasWeakDep>("void f() {int i;}", Diags));
+  EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.WeakDep2\ncustom."
+                   "WeakDep\ncustom.Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+}
+
+//===----------------------------------------------------------------------===//
+// Interaction of weak and regular checker dependencies.
+//===----------------------------------------------------------------------===//
+
+void addWeakDepHasStrongDep(AnalysisASTConsumer &AnalysisConsumer,
+                            AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.StrongDep", true},
+                                {"custom.WeakDep", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addStrongDep(Registry);
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addDependency("custom.WeakDep", "custom.StrongDep");
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+void addWeakDepAndStrongDep(AnalysisASTConsumer &AnalysisConsumer,
+                            AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.StrongDep", true},
+                                {"custom.WeakDep", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addStrongDep(Registry);
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addDependency("custom.Dep", "custom.StrongDep");
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+void addDisabledWeakDepHasStrongDep(AnalysisASTConsumer &AnalysisConsumer,
+                                    AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.StrongDep", true},
+                                {"custom.WeakDep", false},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addStrongDep(Registry);
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addDependency("custom.WeakDep", "custom.StrongDep");
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+void addDisabledWeakDepHasUnspecifiedStrongDep(
+    AnalysisASTConsumer &AnalysisConsumer, AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.WeakDep", false},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addStrongDep(Registry);
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addDependency("custom.WeakDep", "custom.StrongDep");
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+void addWeakDepHasDisabledStrongDep(AnalysisASTConsumer &AnalysisConsumer,
+                                    AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.StrongDep", false},
+                                {"custom.WeakDep", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addStrongDep(Registry);
+    addWeakDep(Registry);
+    addDep(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addDependency("custom.WeakDep", "custom.StrongDep");
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+void addWeakDepHasUnspecifiedButLaterEnabledStrongDep(
+    AnalysisASTConsumer &AnalysisConsumer, AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
+                                {"custom.Dep2", true},
+                                {"custom.WeakDep", true},
+                                {"custom.RegistrationOrder", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([=](CheckerRegistry &Registry) {
+    addStrongDep(Registry);
+    addWeakDep(Registry);
+    addDep(Registry);
+    addDep2(Registry);
+    addCheckerRegistrationOrderPrinter(Registry);
+    Registry.addDependency("custom.WeakDep", "custom.StrongDep");
+    Registry.addDependency("custom.Dep2", "custom.StrongDep");
+    Registry.addWeakDependency("custom.Dep", "custom.WeakDep");
+  });
+}
+
+TEST(RegisterDeps, DependencyInteraction) {
+  std::string Diags;
+  EXPECT_TRUE(
+      runCheckerOnCode<addWeakDepHasStrongDep>("void f() {int i;}", Diags));
+  EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.StrongDep\ncustom."
+                   "WeakDep\ncustom.Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+
+  // Weak dependencies are registered before strong dependencies. This is most
+  // important for purely diagnostic checkers that are implemented as a part of
+  // purely modeling checkers, becuse the checker callback order will have to be
+  // established in between the modeling portion and the weak dependency.
+  EXPECT_TRUE(
+      runCheckerOnCode<addWeakDepAndStrongDep>("void f() {int i;}", Diags));
+  EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.WeakDep\ncustom."
+                   "StrongDep\ncustom.Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+
+  // If a weak dependency is disabled, the checker itself can still be enabled.
+  EXPECT_TRUE(runCheckerOnCode<addDisabledWeakDepHasStrongDep>(
+      "void f() {int i;}", Diags));
+  EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.Dep\ncustom."
+                   "RegistrationOrder\ncustom.StrongDep\n");
+  Diags.clear();
+
+  // If a weak dependency is disabled, the checker itself can still be enabled,
+  // but it shouldn't enable a strong unspecified dependency.
+  EXPECT_TRUE(runCheckerOnCode<addDisabledWeakDepHasUnspecifiedStrongDep>(
+      "void f() {int i;}", Diags));
+  EXPECT_EQ(Diags,
+            "custom.RegistrationOrder:custom.Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+
+  // A strong dependency of a weak dependency is disabled, so neither of them
+  // should be enabled.
+  EXPECT_TRUE(runCheckerOnCode<addWeakDepHasDisabledStrongDep>(
+      "void f() {int i;}", Diags));
+  EXPECT_EQ(Diags,
+            "custom.RegistrationOrder:custom.Dep\ncustom.RegistrationOrder\n");
+  Diags.clear();
+
+  EXPECT_TRUE(
+      runCheckerOnCode<addWeakDepHasUnspecifiedButLaterEnabledStrongDep>(
+          "void f() {int i;}", Diags));
+  EXPECT_EQ(Diags,
+            "custom.RegistrationOrder:custom.StrongDep\ncustom.WeakDep\ncustom."
+            "Dep\ncustom.Dep2\ncustom.RegistrationOrder\n");
+  Diags.clear();
+}
 } // namespace
 } // namespace ento
 } // namespace clang

clang/utils/TableGen/ClangSACheckersEmitter.cpp

for (const Record *Dependency :
       OS << '\"';
       OS.write_escaped(getCheckerFullName(Dependency)) << '\"';
       OS << ")\n";
     }
   }
   OS << "\n"
         "#endif // GET_CHECKER_DEPENDENCIES\n";
 
+  // Emit weak dependencies.
+  //
+  // CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY)
+  //   - FULLNAME: The full name of the checker that is supposed to be
+  //     registered first.
+  //   - DEPENDENCY: The full name of the checker FULLNAME weak depends on.
+  OS << "\n"
+        "#ifdef GET_CHECKER_WEAK_DEPENDENCIES\n";
+  for (const Record *Checker : checkers) {
+    if (Checker->isValueUnset("WeakDependencies"))
+      continue;
+
+    for (const Record *Dependency :
+         Checker->getValueAsListOfDefs("WeakDependencies")) {
+      OS << "CHECKER_WEAK_DEPENDENCY(";
+      OS << '\"';
+      OS.write_escaped(getCheckerFullName(Checker)) << "\", ";
+      OS << '\"';
+      OS.write_escaped(getCheckerFullName(Dependency)) << '\"';
+      OS << ")\n";
+    }
+  }
+  OS << "\n"
+        "#endif // GET_CHECKER_WEAK_DEPENDENCIES\n";
+
   // Emit a package option.
   //
   // CHECKER_OPTION(OPTIONTYPE, CHECKERNAME, OPTIONNAME, DESCRIPTION, DEFAULT)
   //   - OPTIONTYPE: Type of the option, whether it's integer or boolean etc.
   //                 This is important for validating user input. Note that
   //                 it's a string, rather than an actual type: since we can
   //                 load checkers runtime, we can't use template hackery for
   //                 sorting this out compile-time.