Skip to content

Commit

Permalink
[Analyzer] Bugfix for CheckerRegistry
Browse files Browse the repository at this point in the history
`CheckerRegistry` registers a checker either if it is excplicitly
enabled or it is a dependency of an explicitly enabled checker and is
not explicitly disabled. In both cases it is also important that the
checker should be registered (`shoudRegister`//XXX//`()` returns true).

Currently there is a bug here: if the dependenct checker is not
explicitly disabled it is registered regardless of whether it should
be registered. This patch fixes this bug.

Differential Revision: https://reviews.llvm.org/D75842
  • Loading branch information
Adam Balogh committed Mar 19, 2020
1 parent 06c810b commit 6cff2e9
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
Expand Up @@ -167,7 +167,7 @@ class CheckerRegistry {
}

bool isDisabled(const LangOptions &LO) const {
return State == StateFromCmdLine::State_Disabled && ShouldRegister(LO);
return State == StateFromCmdLine::State_Disabled || !ShouldRegister(LO);
}

// Since each checker must have a different full name, we can identify
Expand Down
60 changes: 60 additions & 0 deletions clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
Expand Up @@ -81,6 +81,66 @@ TEST(RegisterCustomCheckers, CheckLocationIncDec) {
runCheckerOnCode<addLocIncDecChecker>("void f() { int *p; (*p)++; }"));
}

//===----------------------------------------------------------------------===//
// Unsatisfied checker dependency
//===----------------------------------------------------------------------===//

class PrerequisiteChecker : public Checker<check::ASTCodeBody> {
public:
void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
BugReporter &BR) const {
BR.EmitBasicReport(D, this, "Prerequisite", categories::LogicError,
"This is the prerequisite checker",
PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
}
};

void registerPrerequisiteChecker(CheckerManager &mgr) {
mgr.registerChecker<PrerequisiteChecker>();
}

bool shouldRegisterPrerequisiteChecker(const LangOptions &LO) {
return false;
}

class DependentChecker : public Checker<check::ASTCodeBody> {
public:
void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
BugReporter &BR) const {
BR.EmitBasicReport(D, this, "Dependent", categories::LogicError,
"This is the Dependent Checker",
PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
}
};

void registerDependentChecker(CheckerManager &mgr) {
mgr.registerChecker<DependentChecker>();
}

bool shouldRegisterDependentChecker(const LangOptions &LO) {
return true;
}

void addDependentChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"custom.Dependent", true}};
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
Registry.addChecker(registerPrerequisiteChecker,
shouldRegisterPrerequisiteChecker,
"custom.Prerequisite", "Description", "", false);
Registry.addChecker(registerDependentChecker,
shouldRegisterDependentChecker,
"custom.Dependent", "Description", "", false);
Registry.addDependency("custom.Dependent", "custom.Prerequisite");
});
}

TEST(RegisterDependentCheckers, RegisterChecker) {
std::string Diags;
EXPECT_TRUE(runCheckerOnCode<addDependentChecker>("void f() {;}", Diags));
EXPECT_EQ(Diags, "");
}

} // namespace
} // namespace ento
} // namespace clang

0 comments on commit 6cff2e9

Please sign in to comment.