diff --git a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td --- a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td @@ -112,6 +112,8 @@ 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 @@ 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 @@ 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 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -347,7 +347,7 @@ def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">, HelpText<"Improve modeling of the C standard library functions">, - Dependencies<[NonNullParamChecker, CallAndMessageModeling]>, + Dependencies<[CallAndMessageModeling]>, CheckerOptions<[ CmdLineOption, 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 --- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h @@ -170,6 +170,7 @@ 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 @@ 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 @@ /// 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 --- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -131,6 +131,10 @@ 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 @@ #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 @@ #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 @@ 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); - - 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 -collectDependencies(const CheckerRegistry::CheckerInfo &checker, - const CheckerManager &Mgr) { + collectWeakDependencies(Checker.WeakDependencies, Mgr, Deps, IsEnabled); - 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; + if (!collectStrongDependencies(Checker.Dependencies, Mgr, Deps, + IsEnabledFromCmdLine)) { + // If we failed to enable any of the dependencies, don't enable this + // checker. + continue; + } - return Ret; + // Note that set_union also preserves the order of insertion. + EnabledCheckers.set_union(Deps); + EnabledCheckers.insert(&Checker); + } } +template 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 @@ 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 @@ 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 @@ 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 @@ 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 --- 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 --- /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 --- a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp +++ b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp @@ -171,6 +171,296 @@ 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 --- a/clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ b/clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -282,6 +282,31 @@ 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)