[clang] e22f1c0 - [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order
Kirstóf Umann via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 12 05:10:19 PDT 2020
Author: Kirstóf Umann
Date: 2020-06-12T14:08:38+02:00
New Revision: e22f1c02a27f4471af1b9ae3aa6d8324b86ab2d0
URL: https://github.com/llvm/llvm-project/commit/e22f1c02a27f4471af1b9ae3aa6d8324b86ab2d0
DIFF: https://github.com/llvm/llvm-project/commit/e22f1c02a27f4471af1b9ae3aa6d8324b86ab2d0.diff
LOG: [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
Added:
clang/test/Analysis/weak-dependencies.c
Modified:
clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
clang/test/Analysis/analyzer-enabled-checkers.c
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
clang/utils/TableGen/ClangSACheckersEmitter.cpp
Removed:
################################################################################
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<string name = ""> {
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;
@@ -122,8 +124,13 @@ 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 "
@@ -133,6 +140,24 @@ 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; }
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<Boolean,
"DisplayLoadedSummaries",
@@ -372,6 +372,7 @@ def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
"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"
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 @@ class CheckerRegistry {
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 @@ class CheckerRegistry {
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 @@ class CheckerRegistry {
/// 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
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<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
@@ -281,77 +308,123 @@ CheckerRegistry::CheckerRegistry(
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) {
+ 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 <typename IsEnabledFn>
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<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 "
@@ -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<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
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)
More information about the cfe-commits
mailing list