Skip to content

Commit

Permalink
[analyzer] Fix an assertion failure if plugins added dependencies
Browse files Browse the repository at this point in the history
Ideally, there is no reason behind not being able to depend on checkers that
come from a different plugin (or on builtin checkers) -- however, this is only
possible if all checkers are added to the registry before resolving checker
dependencies. Since I used a binary search in my addDependency method, this also
resulted in an assertion failure (due to CheckerRegistry::Checkers not being
sorted), since the function used by plugins to register their checkers
(clang_registerCheckers) calls addDependency.

This patch resolves this issue by only noting which dependencies have to
established when addDependency is called, and resolves them at a later stage
when no more checkers are added to the registry, by which point
CheckerRegistry::Checkers is already sorted.

Differential Revision: https://reviews.llvm.org/D59461

llvm-svn: 358750
  • Loading branch information
Kristof Umann committed Apr 19, 2019
1 parent 99f641c commit cd3f147
Showing 3 changed files with 43 additions and 10 deletions.
6 changes: 6 additions & 0 deletions clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
Original file line number Diff line number Diff line change
@@ -195,6 +195,12 @@ class CheckerRegistry {
CheckerInfoList Checkers;
llvm::StringMap<size_t> PackageSizes;

/// Contains all (Dependendent checker, Dependency) pairs. We need this, as
/// we'll resolve dependencies after all checkers were added first.
llvm::SmallVector<std::pair<StringRef, StringRef>, 0> Dependencies;

void resolveDependencies();

DiagnosticsEngine &Diags;
AnalyzerOptions &AnOpts;
const LangOptions &LangOpts;
30 changes: 20 additions & 10 deletions clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
Original file line number Diff line number Diff line change
@@ -177,6 +177,8 @@ CheckerRegistry::CheckerRegistry(
#undef CHECKER_DEPENDENCY
#undef GET_CHECKER_DEPENDENCIES

resolveDependencies();

// Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
// command line.
for (const std::pair<std::string, bool> &Opt : AnOpts.CheckersControlList) {
@@ -278,18 +280,26 @@ void CheckerRegistry::addChecker(InitializationFunction Rfn,
}
}

void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
auto CheckerIt = binaryFind(Checkers, FullName);
assert(CheckerIt != Checkers.end() && CheckerIt->FullName == FullName &&
"Failed to find the checker while attempting to set up its "
"dependencies!");
void CheckerRegistry::resolveDependencies() {
for (const std::pair<StringRef, StringRef> &Entry : Dependencies) {
auto CheckerIt = binaryFind(Checkers, Entry.first);
assert(CheckerIt != Checkers.end() && CheckerIt->FullName == Entry.first &&
"Failed to find the checker while attempting to set up its "
"dependencies!");

auto DependencyIt = binaryFind(Checkers, Dependency);
assert(DependencyIt != Checkers.end() &&
DependencyIt->FullName == Dependency &&
"Failed to find the dependency of a checker!");
auto DependencyIt = binaryFind(Checkers, Entry.second);
assert(DependencyIt != Checkers.end() &&
DependencyIt->FullName == Entry.second &&
"Failed to find the dependency of a checker!");

CheckerIt->Dependencies.emplace_back(&*DependencyIt);
}

CheckerIt->Dependencies.emplace_back(&*DependencyIt);
Dependencies.clear();
}

void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
Dependencies.emplace_back(FullName, Dependency);
}

void CheckerRegistry::initializeManager(CheckerManager &CheckerMgr) const {
17 changes: 17 additions & 0 deletions clang/test/Analysis/checker-dependencies.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
// RUN: %clang_analyze_cc1 %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=nullability.NullReturnedFromNonnull

// RUN: %clang_analyze_cc1 %s \
// RUN: -analyzer-checker=osx.cocoa.RetainCount \
// RUN: -analyzer-list-enabled-checkers \
// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-ENABLED

// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCountBase
// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCount

// RUN: %clang_analyze_cc1 %s \
// RUN: -analyzer-checker=osx.cocoa.RetainCount \
// RUN: -analyzer-disable-checker=osx.cocoa.RetainCountBase \
// RUN: -analyzer-list-enabled-checkers \
// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-DISABLED

// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCountBase
// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCount

0 comments on commit cd3f147

Please sign in to comment.