Szelethus created this revision.
Szelethus added reviewers: NoQ, martong, balazske, xazax.hun, rnkovacs,
baloghadamsoftware, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso,
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D80901: [analyzer][NFC] Change checker
dependency unit tests to check for the registration order.
Szelethus added a child revision: D78126: [analyzer][NFC] Don't allow
dependency checkers to emit diagnostics.
Checker dependencies were added D54438 <https://reviews.llvm.org/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
<https://reviews.llvm.org/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 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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 <https://reviews.llvm.org/D75360>. A
lengthier one is in D58065 <https://reviews.llvm.org/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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80905
Files:
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/weak-dependencies.c
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
clang/utils/TableGen/ClangSACheckersEmitter.cpp
Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp
===================================================================
--- clang/utils/TableGen/ClangSACheckersEmitter.cpp
+++ 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)
Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===================================================================
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -171,6 +171,298 @@
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
Index: clang/test/Analysis/weak-dependencies.c
===================================================================
--- /dev/null
+++ 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]}}
+}
Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -130,6 +130,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
@@ -238,6 +242,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) \
@@ -253,12 +262,31 @@
#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
@@ -280,77 +308,123 @@
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;
-/// Collects dependenies in \p enabledCheckers. Return None on failure.
-LLVM_NODISCARD
-static llvm::Optional<CheckerRegistry::CheckerInfoSet>
-collectDependencies(const CheckerRegistry::CheckerInfo &checker,
- const CheckerManager &Mgr) {
+ CheckerInfoSet Deps;
- 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,
- 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 "
@@ -361,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);
}
}
@@ -369,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,
@@ -514,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)
@@ -550,7 +636,7 @@
continue;
}
- auto PackageIt =
+ const auto *PackageIt =
llvm::find(Packages, PackageInfo(SuppliedCheckerOrPackage));
if (PackageIt != Packages.end()) {
isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedCheckerOrPackage,
Index: clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ 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<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
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ 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<Boolean,
"DisplayLoadedSummaries",
@@ -372,6 +372,7 @@
"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"
Index: clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
+++ clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
@@ -112,6 +112,8 @@
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,10 @@
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.
+/// 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 +137,22 @@
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
+/// for checkers that would warn for the same bug, like non-null parameter bugs
+/// are detected by NonNullParamChecker due to the nonnull attribute, and
+/// StdLibraryFunctionsChecker as it models standard functions. 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.
+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; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
