From e22f1c02a27f4471af1b9ae3aa6d8324b86ab2d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirst=C3=B3f=20Umann?= Date: Wed, 27 May 2020 12:29:47 +0200 Subject: [PATCH] [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order 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. [1] https://www.youtube.com/watch?v=eqKeqHRAhQM Differential Revision: https://reviews.llvm.org/D80905 --- .../StaticAnalyzer/Checkers/CheckerBase.td | 29 +- .../clang/StaticAnalyzer/Checkers/Checkers.td | 3 +- .../StaticAnalyzer/Frontend/CheckerRegistry.h | 11 +- .../Frontend/CheckerRegistry.cpp | 175 ++++++++--- .../test/Analysis/analyzer-enabled-checkers.c | 2 +- clang/test/Analysis/weak-dependencies.c | 15 + .../RegisterCustomCheckersTest.cpp | 290 ++++++++++++++++++ .../utils/TableGen/ClangSACheckersEmitter.cpp | 25 ++ 8 files changed, 499 insertions(+), 51 deletions(-) create mode 100644 clang/test/Analysis/weak-dependencies.c diff --git a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td index 6625d79559f5..98d26aaa637d 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td @@ -112,6 +112,8 @@ class Checker { list CheckerOptions; // This field is optional. list Dependencies; + // This field is optional. + list WeakDependencies; bits<2> Documentation; Package ParentPackage; bit Hidden = 0; @@ -122,8 +124,13 @@ class CheckerOptions opts> { list 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 " @@ -133,6 +140,24 @@ class Dependencies Deps = []> { list 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 Deps = []> { + list 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; } diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 2d69d8f34420..dff8f387c87f 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -347,7 +347,7 @@ let ParentPackage = APIModeling in { def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">, HelpText<"Improve modeling of the C standard library functions">, - Dependencies<[NonNullParamChecker, CallAndMessageModeling]>, + Dependencies<[CallAndMessageModeling]>, CheckerOptions<[ CmdLineOption, "such as whether the parameter of isalpha is in the range [0, 255] " "or is EOF.">, Dependencies<[StdCLibraryFunctionsChecker]>, + WeakDependencies<[NonNullParamChecker]>, Documentation; } // end "alpha.apiModeling" diff --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h index c3494d0ebeef..7b72fae0fefe 100644 --- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h @@ -170,6 +170,7 @@ public: StateFromCmdLine State = StateFromCmdLine::State_Unspecified; ConstCheckerInfoList Dependencies; + ConstCheckerInfoList WeakDependencies; bool isEnabled(const CheckerManager &mgr) const { return State == StateFromCmdLine::State_Enabled && ShouldRegister(mgr); @@ -255,10 +256,14 @@ public: 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 @@ -322,7 +327,9 @@ private: /// Contains all (Dependendent checker, Dependency) pairs. We need this, as /// we'll resolve dependencies after all checkers were added first. llvm::SmallVector, 0> Dependencies; - void resolveDependencies(); + llvm::SmallVector, 0> WeakDependencies; + + template void resolveDependencies(); /// Contains all (FullName, CmdLineOption) pairs. Similarly to dependencies, /// we only modify the actual CheckerInfo and PackageInfo objects once all diff --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp index 401cd1d57bb3..ce5e0497435c 100644 --- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -131,6 +131,10 @@ CheckerRegistry::CheckerInfo::dumpToStream(llvm::raw_ostream &Out) const { 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 @@ -239,6 +243,11 @@ CheckerRegistry::CheckerRegistry( #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) \ @@ -254,12 +263,30 @@ CheckerRegistry::CheckerRegistry( #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(); + resolveDependencies(); + + 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 @@ -281,77 +308,123 @@ CheckerRegistry::CheckerRegistry( validateCheckerOptions(); } -/// Collects dependenies in \p enabledCheckers. Return None on failure. -LLVM_NODISCARD -static llvm::Optional -collectDependencies(const CheckerRegistry::CheckerInfo &checker, - const CheckerManager &Mgr); +//===----------------------------------------------------------------------===// +// Dependency resolving. +//===----------------------------------------------------------------------===// + +template +static bool +collectStrongDependencies(const CheckerRegistry::ConstCheckerInfoList &Deps, + const CheckerManager &Mgr, + CheckerRegistry::CheckerInfoSet &Ret, + IsEnabledFn IsEnabled); + +/// Collects weak dependencies in \p enabledCheckers. +template +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 Deps = collectDependencies(Checker, Mgr); + CheckerInfoSet Deps; + if (!collectStrongDependencies(Checker.Dependencies, Mgr, Deps, + IsEnabledFromCmdLine)) { + // If we failed to enable any of the dependencies, don't enable this + // checker. + continue; + } - if (!Deps) { + Tmp.insert(Deps.begin(), Deps.end()); + + // Enable the checker. + Tmp.insert(&Checker); + } + + // 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; + + collectWeakDependencies(Checker.WeakDependencies, Mgr, Deps, IsEnabled); + + 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); - - // Enable the checker. + EnabledCheckers.set_union(Deps); EnabledCheckers.insert(&Checker); } } -/// Collects dependencies in \p ret, returns false on failure. +template static bool -collectDependenciesImpl(const CheckerRegistry::ConstCheckerInfoList &Deps, - const CheckerManager &Mgr, - CheckerRegistry::CheckerInfoSet &Ret); - -/// Collects dependenies in \p enabledCheckers. Return None on failure. -LLVM_NODISCARD -static llvm::Optional -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; - - return Ret; -} - -static bool -collectDependenciesImpl(const CheckerRegistry::ConstCheckerInfoList &Deps, - const CheckerManager &Mgr, - CheckerRegistry::CheckerInfoSet &Ret) { +collectStrongDependencies(const CheckerRegistry::ConstCheckerInfoList &Deps, + const CheckerManager &Mgr, + 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 &Entry : Dependencies) { +template +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 void CheckerRegistry::resolveDependencies() { + for (const std::pair &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 " @@ -362,7 +435,10 @@ void CheckerRegistry::resolveDependencies() { DependencyIt->FullName == Entry.second && "Failed to find the dependency of a checker!"); - CheckerIt->Dependencies.emplace_back(&*DependencyIt); + if (IsWeak) + CheckerIt->WeakDependencies.emplace_back(&*DependencyIt); + else + CheckerIt->Dependencies.emplace_back(&*DependencyIt); } } @@ -370,6 +446,15 @@ 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, @@ -515,7 +600,7 @@ isOptionContainedIn(const CheckerRegistry::CmdLineOptionList &OptionList, 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) @@ -551,7 +636,7 @@ void CheckerRegistry::validateCheckerOptions() const { continue; } - auto PackageIt = + const auto *PackageIt = llvm::find(Packages, PackageInfo(SuppliedCheckerOrPackage)); if (PackageIt != Packages.end()) { isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedCheckerOrPackage, diff --git a/clang/test/Analysis/analyzer-enabled-checkers.c b/clang/test/Analysis/analyzer-enabled-checkers.c index 028754f0fbed..7c00e78c16ac 100644 --- a/clang/test/Analysis/analyzer-enabled-checkers.c +++ b/clang/test/Analysis/analyzer-enabled-checkers.c @@ -6,7 +6,6 @@ // 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 @@ -15,6 +14,7 @@ // 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 diff --git a/clang/test/Analysis/weak-dependencies.c b/clang/test/Analysis/weak-dependencies.c new file mode 100644 index 000000000000..a2179ecaff57 --- /dev/null +++ b/clang/test/Analysis/weak-dependencies.c @@ -0,0 +1,15 @@ +// 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]}} +} diff --git a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp index c88dfca224b3..30a4efc06a4a 100644 --- a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp +++ b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp @@ -171,6 +171,296 @@ TEST(RegisterDeps, UnsatisfiedDependency) { EXPECT_TRUE(runCheckerOnCode("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( + "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( + "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( + "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( + "void f() {int i;}", Diags)); + EXPECT_EQ(Diags, + "custom.RegistrationOrder:custom.Dep\ncustom.RegistrationOrder\n"); + Diags.clear(); + + EXPECT_TRUE( + runCheckerOnCode("void f() {int i;}", Diags)); + EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.WeakDep2\ncustom." + "Dep\ncustom.RegistrationOrder\n"); + Diags.clear(); + + EXPECT_TRUE( + runCheckerOnCode("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("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("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( + "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( + "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( + "void f() {int i;}", Diags)); + EXPECT_EQ(Diags, + "custom.RegistrationOrder:custom.Dep\ncustom.RegistrationOrder\n"); + Diags.clear(); + + EXPECT_TRUE( + runCheckerOnCode( + "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 diff --git a/clang/utils/TableGen/ClangSACheckersEmitter.cpp b/clang/utils/TableGen/ClangSACheckersEmitter.cpp index 50725ae08384..00d88274fc38 100644 --- a/clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ b/clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -282,6 +282,31 @@ void clang::EmitClangSACheckers(RecordKeeper &Records, raw_ostream &OS) { 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)