Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, szepet, whisperity.
TL;DR: `CheckerOptInfo` feels very much out of place in
`CheckerRegistration.cpp`, so I moved it to `CheckerRegistry.h`.
Details:
While on the quest of fixing checker options the same way I cleaned up
non-checker options, although I'm making good progress, I ran into a wall: In
order to emit warnings on incorrect checker options, we need to ensure that
checkers actually acquire their options properly -- but, I unearthed a huge bug
in checker registration: dependencies are currently implemented in a way that
breaks the already very fragile registration infrastructure.
Here is where the problem really originates from: this is a snippet from
`CheckerRegistry::initializeManager`.
// Initialize the CheckerManager with all enabled checkers.
for (const auto *i :enabledCheckers) {
checkerMgr.setCurrentCheckName(CheckName(i->FullName));
i->Initialize(checkerMgr);
}
Note that `Initialize` is a function pointer to `register##CHECKERNAME`, like
`registerInnerPointerChecker`. Since for each register function call the
current checker name is only set once, when `InnerPointerChecker` attempts to
also register it's dependent `MallocChecker`, both it and `MallocChecker` will
receive the `cplusplus.InnerPointer` name. This results in `MallocChecker`
trying to acquire it's `Optimistic` option as
`cplusplus.InnerPointerChecker:Optimistic`.
Clearly, this problem has to be solved at a higher level -- it makes no sense
to be affect other checkers in a registry function. Since I'll already make
changes to how checker registration works, I'll probably tune some things for
the current C++ standard, add much needed documentation, and try to make the
code //a lot less confusing//.
Repository:
rC Clang
https://reviews.llvm.org/D54397
Files:
include/clang/StaticAnalyzer/Core/CheckerOptInfo.h
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
lib/StaticAnalyzer/Core/CheckerRegistry.cpp
lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -17,7 +17,6 @@
#include "clang/StaticAnalyzer/Checkers/ClangCheckers.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/CheckerOptInfo.h"
#include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
#include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
#include "llvm/ADT/SmallVector.h"
@@ -102,44 +101,23 @@
<< pluginAPIVersion;
}
-static SmallVector<CheckerOptInfo, 8>
-getCheckerOptList(const AnalyzerOptions &opts) {
- SmallVector<CheckerOptInfo, 8> checkerOpts;
- for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
- const std::pair<std::string, bool> &opt = opts.CheckersControlList[i];
- checkerOpts.push_back(CheckerOptInfo(opt.first, opt.second));
- }
- return checkerOpts;
-}
-
std::unique_ptr<CheckerManager> ento::createCheckerManager(
ASTContext &context,
AnalyzerOptions &opts,
ArrayRef<std::string> plugins,
ArrayRef<std::function<void(CheckerRegistry &)>> checkerRegistrationFns,
DiagnosticsEngine &diags) {
auto checkerMgr = llvm::make_unique<CheckerManager>(context, opts);
- SmallVector<CheckerOptInfo, 8> checkerOpts = getCheckerOptList(opts);
-
ClangCheckerRegistry allCheckers(plugins, &diags);
for (const auto &Fn : checkerRegistrationFns)
Fn(allCheckers);
- allCheckers.initializeManager(*checkerMgr, checkerOpts);
+ allCheckers.initializeManager(*checkerMgr, opts, diags);
allCheckers.validateCheckerOptions(opts, diags);
checkerMgr->finishedCheckerRegistration();
- for (unsigned i = 0, e = checkerOpts.size(); i != e; ++i) {
- if (checkerOpts[i].isUnclaimed()) {
- diags.Report(diag::err_unknown_analyzer_checker)
- << checkerOpts[i].getName();
- diags.Report(diag::note_suggest_disabling_all_checkers);
- }
-
- }
-
return checkerMgr;
}
@@ -155,8 +133,7 @@
const AnalyzerOptions &opts) {
out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
- SmallVector<CheckerOptInfo, 8> checkerOpts = getCheckerOptList(opts);
- ClangCheckerRegistry(plugins).printList(out, checkerOpts);
+ ClangCheckerRegistry(plugins).printList(out, opts);
}
void ento::printAnalyzerConfigList(raw_ostream &out) {
Index: lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -12,7 +12,6 @@
#include "clang/Basic/LLVM.h"
#include "clang/Frontend/FrontendDiagnostic.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/CheckerOptInfo.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
@@ -30,6 +29,41 @@
using CheckerInfoSet = llvm::SetVector<const CheckerRegistry::CheckerInfo *>;
+namespace {
+/// Represents a request to include or exclude a checker or package from a
+/// specific analysis run.
+///
+/// \sa CheckerRegistry::initializeManager
+class CheckerOptInfo {
+ StringRef Name;
+ bool Enable;
+ bool Claimed;
+
+public:
+ CheckerOptInfo(StringRef name, bool enable)
+ : Name(name), Enable(enable), Claimed(false) { }
+
+ StringRef getName() const { return Name; }
+ bool isEnabled() const { return Enable; }
+ bool isDisabled() const { return !isEnabled(); }
+
+ bool isClaimed() const { return Claimed; }
+ bool isUnclaimed() const { return !isClaimed(); }
+ void claim() { Claimed = true; }
+};
+
+} // end of anonymous namespace
+
+static SmallVector<CheckerOptInfo, 8>
+getCheckerOptList(const AnalyzerOptions &opts) {
+ SmallVector<CheckerOptInfo, 8> checkerOpts;
+ for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
+ const std::pair<std::string, bool> &opt = opts.CheckersControlList[i];
+ checkerOpts.push_back(CheckerOptInfo(opt.first, opt.second));
+ }
+ return checkerOpts;
+}
+
static bool checkerNameLT(const CheckerRegistry::CheckerInfo &a,
const CheckerRegistry::CheckerInfo &b) {
return a.FullName < b.FullName;
@@ -52,6 +86,7 @@
return false;
}
+/// Collects the checkers for the supplied \p opt option into \p collected.
static void collectCheckers(const CheckerRegistry::CheckerInfoList &checkers,
const llvm::StringMap<size_t> &packageSizes,
CheckerOptInfo &opt, CheckerInfoSet &collected) {
@@ -101,20 +136,31 @@
}
void CheckerRegistry::initializeManager(CheckerManager &checkerMgr,
- SmallVectorImpl<CheckerOptInfo> &opts) const {
+ const AnalyzerOptions &Opts,
+ DiagnosticsEngine &diags) const {
// Sort checkers for efficient collection.
llvm::sort(Checkers, checkerNameLT);
+ llvm::SmallVector<CheckerOptInfo, 8> checkerOpts = getCheckerOptList(Opts);
// Collect checkers enabled by the options.
CheckerInfoSet enabledCheckers;
- for (auto &i : opts)
+ for (auto &i : checkerOpts)
collectCheckers(Checkers, Packages, i, enabledCheckers);
// Initialize the CheckerManager with all enabled checkers.
for (const auto *i :enabledCheckers) {
checkerMgr.setCurrentCheckName(CheckName(i->FullName));
i->Initialize(checkerMgr);
}
+
+ for (unsigned i = 0, e = checkerOpts.size(); i != e; ++i) {
+ if (checkerOpts[i].isUnclaimed()) {
+ diags.Report(diag::err_unknown_analyzer_checker)
+ << checkerOpts[i].getName();
+ diags.Report(diag::note_suggest_disabling_all_checkers);
+ }
+
+ }
}
void CheckerRegistry::validateCheckerOptions(const AnalyzerOptions &opts,
@@ -176,13 +222,14 @@
}
}
-void CheckerRegistry::printList(
- raw_ostream &out, SmallVectorImpl<CheckerOptInfo> &opts) const {
+void CheckerRegistry::printList(raw_ostream &out,
+ const AnalyzerOptions &opts) const {
llvm::sort(Checkers, checkerNameLT);
+ llvm::SmallVector<CheckerOptInfo, 8> checkerOpts = getCheckerOptList(opts);
// Collect checkers enabled by the options.
CheckerInfoSet enabledCheckers;
- for (auto &i : opts)
+ for (auto &i : checkerOpts)
collectCheckers(Checkers, Packages, i, enabledCheckers);
for (const auto *i : enabledCheckers)
Index: include/clang/StaticAnalyzer/Core/CheckerRegistry.h
===================================================================
--- include/clang/StaticAnalyzer/Core/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Core/CheckerRegistry.h
@@ -73,8 +73,6 @@
namespace ento {
-class CheckerOptInfo;
-
/// Manages a set of available checkers for running a static analysis.
/// The checkers are organized into packages by full name, where including
/// a package will recursively include all subpackages and checkers within it.
@@ -123,18 +121,17 @@
/// all checkers specified by the given CheckerOptInfo list. The order of this
/// list is significant; later options can be used to reverse earlier ones.
/// This can be used to exclude certain checkers in an included package.
- void initializeManager(CheckerManager &mgr,
- SmallVectorImpl<CheckerOptInfo> &opts) const;
+ void initializeManager(CheckerManager &mgr, const AnalyzerOptions &Opts,
+ DiagnosticsEngine &diags) const;
/// Check if every option corresponds to a specific checker or package.
void validateCheckerOptions(const AnalyzerOptions &opts,
DiagnosticsEngine &diags) const;
/// Prints the name and description of all checkers in this registry.
/// This output is not intended to be machine-parseable.
void printHelp(raw_ostream &out, size_t maxNameChars = 30) const;
- void printList(raw_ostream &out,
- SmallVectorImpl<CheckerOptInfo> &opts) const;
+ void printList(raw_ostream &out, const AnalyzerOptions &opts) const;
private:
mutable CheckerInfoList Checkers;
Index: include/clang/StaticAnalyzer/Core/CheckerOptInfo.h
===================================================================
--- include/clang/StaticAnalyzer/Core/CheckerOptInfo.h
+++ /dev/null
@@ -1,44 +0,0 @@
-//===--- CheckerOptInfo.h - Specifies which checkers to use -----*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_CHECKEROPTINFO_H
-#define LLVM_CLANG_STATICANALYZER_CORE_CHECKEROPTINFO_H
-
-#include "clang/Basic/LLVM.h"
-#include "llvm/ADT/StringRef.h"
-
-namespace clang {
-namespace ento {
-
-/// Represents a request to include or exclude a checker or package from a
-/// specific analysis run.
-///
-/// \sa CheckerRegistry::initializeManager
-class CheckerOptInfo {
- StringRef Name;
- bool Enable;
- bool Claimed;
-
-public:
- CheckerOptInfo(StringRef name, bool enable)
- : Name(name), Enable(enable), Claimed(false) { }
-
- StringRef getName() const { return Name; }
- bool isEnabled() const { return Enable; }
- bool isDisabled() const { return !isEnabled(); }
-
- bool isClaimed() const { return Claimed; }
- bool isUnclaimed() const { return !isClaimed(); }
- void claim() { Claimed = true; }
-};
-
-} // end namespace ento
-} // end namespace clang
-
-#endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
