llvm-project/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

773 lines
28 KiB
C++
Raw Normal View History

//===- CheckerRegistry.cpp - Maintains all available checkers -------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LLVM.h"
#include "clang/Driver/DriverDiagnostic.h"
#include "clang/Frontend/FrontendDiagnostic.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/DynamicLibrary.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
using namespace clang;
using namespace ento;
using llvm::sys::DynamicLibrary;
//===----------------------------------------------------------------------===//
// Utilities.
//===----------------------------------------------------------------------===//
using RegisterCheckersFn = void (*)(CheckerRegistry &);
static bool isCompatibleAPIVersion(const char *VersionString) {
// If the version string is null, its not an analyzer plugin.
if (!VersionString)
return false;
// For now, none of the static analyzer API is considered stable.
// Versions must match exactly.
return strcmp(VersionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
}
namespace {
template <class T> struct FullNameLT {
bool operator()(const T &Lhs, const T &Rhs) {
return Lhs.FullName < Rhs.FullName;
}
};
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
using PackageNameLT = FullNameLT<CheckerRegistry::PackageInfo>;
using CheckerNameLT = FullNameLT<CheckerRegistry::CheckerInfo>;
} // end of anonymous namespace
template <class CheckerOrPackageInfoList>
static std::conditional_t<std::is_const<CheckerOrPackageInfoList>::value,
typename CheckerOrPackageInfoList::const_iterator,
typename CheckerOrPackageInfoList::iterator>
binaryFind(CheckerOrPackageInfoList &Collection, StringRef FullName) {
using CheckerOrPackage = typename CheckerOrPackageInfoList::value_type;
using CheckerOrPackageFullNameLT = FullNameLT<CheckerOrPackage>;
assert(llvm::is_sorted(Collection, CheckerOrPackageFullNameLT{}) &&
"In order to efficiently gather checkers/packages, this function "
"expects them to be already sorted!");
return llvm::lower_bound(Collection, CheckerOrPackage(FullName),
CheckerOrPackageFullNameLT{});
}
static constexpr char PackageSeparator = '.';
static bool isInPackage(const CheckerRegistry::CheckerInfo &Checker,
StringRef PackageName) {
// Does the checker's full name have the package as a prefix?
if (!Checker.FullName.startswith(PackageName))
return false;
// Is the package actually just the name of a specific checker?
if (Checker.FullName.size() == PackageName.size())
return true;
// Is the checker in the package (or a subpackage)?
if (Checker.FullName[PackageName.size()] == PackageSeparator)
return true;
return false;
}
//===----------------------------------------------------------------------===//
// Methods of CmdLineOption, PackageInfo and CheckerInfo.
//===----------------------------------------------------------------------===//
LLVM_DUMP_METHOD void
CheckerRegistry::CmdLineOption::dumpToStream(llvm::raw_ostream &Out) const {
// The description can be just checked in Checkers.inc, the point here is to
// debug whether we succeeded in parsing it.
Out << OptionName << " (" << OptionType << ", "
<< (IsHidden ? "hidden, " : "") << DevelopmentStatus << ") default: \""
<< DefaultValStr;
}
static StringRef toString(CheckerRegistry::StateFromCmdLine Kind) {
switch (Kind) {
case CheckerRegistry::StateFromCmdLine::State_Disabled:
return "Disabled";
case CheckerRegistry::StateFromCmdLine::State_Enabled:
return "Enabled";
case CheckerRegistry::StateFromCmdLine::State_Unspecified:
return "Unspecified";
}
llvm_unreachable("Unhandled CheckerRegistry::StateFromCmdLine enum");
}
LLVM_DUMP_METHOD void
CheckerRegistry::CheckerInfo::dumpToStream(llvm::raw_ostream &Out) const {
// The description can be just checked in Checkers.inc, the point here is to
// debug whether we succeeded in parsing it. Same with documentation uri.
Out << FullName << " (" << toString(State) << (IsHidden ? ", hidden" : "")
<< ")\n";
Out << " Options:\n";
for (const CmdLineOption &Option : CmdLineOptions) {
Out << " ";
Option.dumpToStream(Out);
Out << '\n';
}
Out << " Dependencies:\n";
for (const CheckerInfo *Dependency : Dependencies) {
Out << " " << Dependency->FullName << '\n';
}
[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
2020-05-27 18:29:47 +08:00
Out << " Weak dependencies:\n";
for (const CheckerInfo *Dependency : WeakDependencies) {
Out << " " << Dependency->FullName << '\n';
}
}
LLVM_DUMP_METHOD void
CheckerRegistry::PackageInfo::dumpToStream(llvm::raw_ostream &Out) const {
Out << FullName << "\n";
Out << " Options:\n";
for (const CmdLineOption &Option : CmdLineOptions) {
Out << " ";
Option.dumpToStream(Out);
Out << '\n';
}
}
//===----------------------------------------------------------------------===//
// Methods of CheckerRegistry.
//===----------------------------------------------------------------------===//
CheckerRegistry::CheckerInfoListRange
CheckerRegistry::getMutableCheckersForCmdLineArg(StringRef CmdLineArg) {
auto It = binaryFind(Checkers, CmdLineArg);
if (!isInPackage(*It, CmdLineArg))
return {Checkers.end(), Checkers.end()};
// See how large the package is.
// If the package doesn't exist, assume the option refers to a single
// checker.
size_t Size = 1;
llvm::StringMap<size_t>::const_iterator PackageSize =
PackageSizes.find(CmdLineArg);
if (PackageSize != PackageSizes.end())
Size = PackageSize->getValue();
return {It, It + Size};
}
CheckerRegistry::CheckerRegistry(
ArrayRef<std::string> Plugins, DiagnosticsEngine &Diags,
AnalyzerOptions &AnOpts,
ArrayRef<std::function<void(CheckerRegistry &)>> CheckerRegistrationFns)
: Diags(Diags), AnOpts(AnOpts) {
// Register builtin checkers.
#define GET_CHECKERS
#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) \
addChecker(register##CLASS, shouldRegister##CLASS, FULLNAME, HELPTEXT, \
DOC_URI, IS_HIDDEN);
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
#define GET_PACKAGES
#define PACKAGE(FULLNAME) addPackage(FULLNAME);
#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
#undef CHECKER
#undef GET_CHECKERS
#undef PACKAGE
#undef GET_PACKAGES
// Register checkers from plugins.
for (const std::string &Plugin : Plugins) {
// Get access to the plugin.
std::string ErrorMsg;
DynamicLibrary Lib =
DynamicLibrary::getPermanentLibrary(Plugin.c_str(), &ErrorMsg);
if (!Lib.isValid()) {
Diags.Report(diag::err_fe_unable_to_load_plugin) << Plugin << ErrorMsg;
continue;
}
// See if its compatible with this build of clang.
const char *PluginAPIVersion = static_cast<const char *>(
Lib.getAddressOfSymbol("clang_analyzerAPIVersionString"));
if (!isCompatibleAPIVersion(PluginAPIVersion)) {
Diags.Report(diag::warn_incompatible_analyzer_plugin_api)
<< llvm::sys::path::filename(Plugin);
Diags.Report(diag::note_incompatible_analyzer_plugin_api)
<< CLANG_ANALYZER_API_VERSION_STRING << PluginAPIVersion;
continue;
}
// Register its checkers.
RegisterCheckersFn RegisterPluginCheckers =
reinterpret_cast<RegisterCheckersFn>(
Lib.getAddressOfSymbol("clang_registerCheckers"));
if (RegisterPluginCheckers)
RegisterPluginCheckers(*this);
}
// Register statically linked checkers, that aren't generated from the tblgen
// file, but rather passed their registry function as a parameter in
// checkerRegistrationFns.
for (const auto &Fn : CheckerRegistrationFns)
Fn(*this);
// Sort checkers for efficient collection.
// FIXME: Alphabetical sort puts 'experimental' in the middle.
// Would it be better to name it '~experimental' or something else
// that's ASCIIbetically last?
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
llvm::sort(Packages, PackageNameLT{});
llvm::sort(Checkers, CheckerNameLT{});
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
#define GET_CHECKER_DEPENDENCIES
#define CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY) \
addDependency(FULLNAME, DEPENDENCY);
[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
2020-05-27 18:29:47 +08:00
#define GET_CHECKER_WEAK_DEPENDENCIES
#define CHECKER_WEAK_DEPENDENCY(FULLNAME, DEPENDENCY) \
addWeakDependency(FULLNAME, DEPENDENCY);
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
#define GET_CHECKER_OPTIONS
#define CHECKER_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, \
DEVELOPMENT_STATUS, IS_HIDDEN) \
addCheckerOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC, \
DEVELOPMENT_STATUS, IS_HIDDEN);
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
#define GET_PACKAGE_OPTIONS
#define PACKAGE_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, \
DEVELOPMENT_STATUS, IS_HIDDEN) \
addPackageOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC, \
DEVELOPMENT_STATUS, IS_HIDDEN);
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
#undef CHECKER_DEPENDENCY
#undef GET_CHECKER_DEPENDENCIES
[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
2020-05-27 18:29:47 +08:00
#undef CHECKER_WEAK_DEPENDENCY
#undef GET_CHECKER_WEAK_DEPENDENCIES
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
#undef CHECKER_OPTION
#undef GET_CHECKER_OPTIONS
#undef PACKAGE_OPTION
#undef GET_PACKAGE_OPTIONS
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
[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
2020-05-27 18:29:47 +08:00
resolveDependencies<true>();
resolveDependencies<false>();
#ifndef NDEBUG
[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
2020-05-27 18:29:47 +08:00
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!");
}
}
#endif
[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
2020-05-27 18:29:47 +08:00
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
resolveCheckerAndPackageOptions();
// Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
// command line.
for (const std::pair<std::string, bool> &Opt : AnOpts.CheckersAndPackages) {
CheckerInfoListRange CheckerForCmdLineArg =
getMutableCheckersForCmdLineArg(Opt.first);
if (CheckerForCmdLineArg.begin() == CheckerForCmdLineArg.end()) {
Diags.Report(diag::err_unknown_analyzer_checker_or_package) << Opt.first;
Diags.Report(diag::note_suggest_disabling_all_checkers);
}
for (CheckerInfo &checker : CheckerForCmdLineArg) {
checker.State = Opt.second ? StateFromCmdLine::State_Enabled
: StateFromCmdLine::State_Disabled;
}
}
validateCheckerOptions();
}
[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
2020-05-27 18:29:47 +08:00
//===----------------------------------------------------------------------===//
// 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) {
[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
2020-05-27 18:29:47 +08:00
// 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;
[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
2020-05-27 18:29:47 +08:00
CheckerInfoSet Deps;
if (!collectStrongDependencies(Checker.Dependencies, Mgr, Deps,
IsEnabledFromCmdLine)) {
// If we failed to enable any of the dependencies, don't enable this
// checker.
continue;
}
[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
2020-05-27 18:29:47 +08:00
Tmp.insert(Deps.begin(), Deps.end());
// Enable the checker.
[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
2020-05-27 18:29:47 +08:00
Tmp.insert(&Checker);
}
[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
2020-05-27 18:29:47 +08:00
// 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;
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
[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
2020-05-27 18:29:47 +08:00
collectWeakDependencies(Checker.WeakDependencies, Mgr, Deps, IsEnabled);
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
[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
2020-05-27 18:29:47 +08:00
if (!collectStrongDependencies(Checker.Dependencies, Mgr, Deps,
IsEnabledFromCmdLine)) {
// If we failed to enable any of the dependencies, don't enable this
// checker.
continue;
}
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
[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
2020-05-27 18:29:47 +08:00
// Note that set_union also preserves the order of insertion.
EnabledCheckers.set_union(Deps);
EnabledCheckers.insert(&Checker);
}
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
}
[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
2020-05-27 18:29:47 +08:00
template <typename IsEnabledFn>
static bool
[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
2020-05-27 18:29:47 +08:00
collectStrongDependencies(const CheckerRegistry::ConstCheckerInfoList &Deps,
const CheckerManager &Mgr,
CheckerRegistry::CheckerInfoSet &Ret,
IsEnabledFn IsEnabled) {
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
for (const CheckerRegistry::CheckerInfo *Dependency : Deps) {
[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
2020-05-27 18:29:47 +08:00
if (!IsEnabled(Dependency))
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
return false;
// Collect dependencies recursively.
[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
2020-05-27 18:29:47 +08:00
if (!collectStrongDependencies(Dependency->Dependencies, Mgr, Ret,
IsEnabled))
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
return false;
Ret.insert(Dependency);
[analyzer] Reimplement dependencies between checkers Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
2019-01-27 04:06:54 +08:00
}
return true;
}
[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
2020-05-27 18:29:47 +08:00
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 "
"dependencies!");
auto DependencyIt = binaryFind(Checkers, Entry.second);
assert(DependencyIt != Checkers.end() &&
DependencyIt->FullName == Entry.second &&
"Failed to find the dependency of a checker!");
[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
2020-05-27 18:29:47 +08:00
if (IsWeak)
CheckerIt->WeakDependencies.emplace_back(&*DependencyIt);
else
CheckerIt->Dependencies.emplace_back(&*DependencyIt);
}
}
void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
Dependencies.emplace_back(FullName, Dependency);
}
[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
2020-05-27 18:29:47 +08:00
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,
const CheckerRegistry::CmdLineOption &Option,
AnalyzerOptions &AnOpts,
DiagnosticsEngine &Diags) {
std::string FullOption = (FullName + ":" + Option.OptionName).str();
auto It =
AnOpts.Config.insert({FullOption, std::string(Option.DefaultValStr)});
// Insertation was successful -- CmdLineOption's constructor will validate
// whether values received from plugins or TableGen files are correct.
if (It.second)
return;
// Insertion failed, the user supplied this package/checker option on the
// command line. If the supplied value is invalid, we'll restore the option
// to it's default value, and if we're in non-compatibility mode, we'll also
// emit an error.
StringRef SuppliedValue = It.first->getValue();
if (Option.OptionType == "bool") {
if (SuppliedValue != "true" && SuppliedValue != "false") {
if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
Diags.Report(diag::err_analyzer_checker_option_invalid_input)
<< FullOption << "a boolean value";
}
It.first->setValue(std::string(Option.DefaultValStr));
}
return;
}
if (Option.OptionType == "int") {
int Tmp;
bool HasFailed = SuppliedValue.getAsInteger(0, Tmp);
if (HasFailed) {
if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
Diags.Report(diag::err_analyzer_checker_option_invalid_input)
<< FullOption << "an integer value";
}
It.first->setValue(std::string(Option.DefaultValStr));
}
return;
}
}
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
template <class T>
static void
insertOptionToCollection(StringRef FullName, T &Collection,
const CheckerRegistry::CmdLineOption &Option,
AnalyzerOptions &AnOpts, DiagnosticsEngine &Diags) {
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
auto It = binaryFind(Collection, FullName);
assert(It != Collection.end() &&
"Failed to find the checker while attempting to add a command line "
"option to it!");
insertAndValidate(FullName, Option, AnOpts, Diags);
It->CmdLineOptions.emplace_back(Option);
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
}
void CheckerRegistry::resolveCheckerAndPackageOptions() {
for (const std::pair<StringRef, CmdLineOption> &CheckerOptEntry :
CheckerOptions) {
insertOptionToCollection(CheckerOptEntry.first, Checkers,
CheckerOptEntry.second, AnOpts, Diags);
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
}
for (const std::pair<StringRef, CmdLineOption> &PackageOptEntry :
PackageOptions) {
insertOptionToCollection(PackageOptEntry.first, Packages,
PackageOptEntry.second, AnOpts, Diags);
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
}
}
void CheckerRegistry::addPackage(StringRef FullName) {
Packages.emplace_back(PackageInfo(FullName));
}
void CheckerRegistry::addPackageOption(StringRef OptionType,
StringRef PackageFullName,
StringRef OptionName,
StringRef DefaultValStr,
StringRef Description,
StringRef DevelopmentStatus,
bool IsHidden) {
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
PackageOptions.emplace_back(
PackageFullName, CmdLineOption{OptionType, OptionName, DefaultValStr,
Description, DevelopmentStatus, IsHidden});
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
}
void CheckerRegistry::addChecker(InitializationFunction Rfn,
ShouldRegisterFunction Sfn, StringRef Name,
StringRef Desc, StringRef DocsUri,
bool IsHidden) {
Checkers.emplace_back(Rfn, Sfn, Name, Desc, DocsUri, IsHidden);
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
// Record the presence of the checker in its packages.
StringRef PackageName, LeafName;
std::tie(PackageName, LeafName) = Name.rsplit(PackageSeparator);
while (!LeafName.empty()) {
PackageSizes[PackageName] += 1;
std::tie(PackageName, LeafName) = PackageName.rsplit(PackageSeparator);
}
}
void CheckerRegistry::addCheckerOption(StringRef OptionType,
StringRef CheckerFullName,
StringRef OptionName,
StringRef DefaultValStr,
StringRef Description,
StringRef DevelopmentStatus,
bool IsHidden) {
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
CheckerOptions.emplace_back(
CheckerFullName, CmdLineOption{OptionType, OptionName, DefaultValStr,
Description, DevelopmentStatus, IsHidden});
[analyzer][NFC] Reimplement checker options TL;DR: * Add checker and package options to the TableGen files * Added a new class called CmdLineOption, and both Package and Checker recieved a list<CmdLineOption> field. * Added every existing checker and package option to Checkers.td. * The CheckerRegistry class * Received some comments to most of it's inline classes * Received the CmdLineOption and PackageInfo inline classes, a list of CmdLineOption was added to CheckerInfo and PackageInfo * Added addCheckerOption and addPackageOption * Added a new field called Packages, used in addPackageOptions, filled up in addPackage Detailed description: In the last couple months, a lot of effort was put into tightening the analyzer's command line interface. The main issue is that it's spectacularly easy to mess up a lenghty enough invocation of the analyzer, and the user was given no warnings or errors at all in that case. We can divide the effort of resolving this into several chapters: * Non-checker analyzer configurations: Gather every analyzer configuration into a dedicated file. Emit errors for non-existent configurations or incorrect values. Be able to list these configurations. Tighten AnalyzerOptions interface to disallow making such a mistake in the future. * Fix the "Checker Naming Bug" by reimplementing checker dependencies: When cplusplus.InnerPointer was enabled, it implicitly registered unix.Malloc, which implicitly registered some sort of a modeling checker from the CStringChecker family. This resulted in all of these checker objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions asking for the wrong checker options from the command line: cplusplus.InnerPointer:Optimisic istead of unix.Malloc:Optimistic. This was resolved by making CheckerRegistry responsible for checker dependency handling, instead of checkers themselves. * Checker options: (this patch included!) Same as the first item, but for checkers. (+ minor fixes here and there, and everything else that is yet to come) There were several issues regarding checker options, that non-checker configurations didn't suffer from: checker plugins are loaded runtime, and they could add new checkers and new options, meaning that unlike for non-checker configurations, we can't collect every checker option purely by generating code. Also, as seen from the "Checker Naming Bug" issue raised above, they are very rarely used in practice, and all sorts of skeletons fell out of the closet while working on this project. They were extremely problematic for users as well, purely because of how long they were. Consider the following monster of a checker option: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false While we were able to verify whether the checker itself (the part before the colon) existed, any errors past that point were unreported, easily resulting in 7+ hours of analyses going to waste. This patch, similarly to how dependencies were reimplemented, uses TableGen to register checker options into Checkers.td, so that Checkers.inc now contains entries for both checker and package options. Using the preprocessor, Checkers.inc is converted into code in CheckerRegistry, adding every builtin (checkers and packages that have an entry in the Checkers.td file) checker and package option to the registry. The new addPackageOption and addCheckerOption functions expose the same functionality to statically-linked non-builtin and plugin checkers and packages as well. Emitting errors for incorrect user input, being able to list these options, and some other functionalies will land in later patches. Differential Revision: https://reviews.llvm.org/D57855 llvm-svn: 358752
2019-04-19 20:32:10 +08:00
}
void CheckerRegistry::initializeManager(CheckerManager &CheckerMgr) const {
// Initialize the CheckerManager with all enabled checkers.
for (const auto *Checker : EnabledCheckers) {
CheckerMgr.setCurrentCheckerName(CheckerNameRef(Checker->FullName));
Checker->Initialize(CheckerMgr);
}
}
static void
isOptionContainedIn(const CheckerRegistry::CmdLineOptionList &OptionList,
StringRef SuppliedChecker, StringRef SuppliedOption,
const AnalyzerOptions &AnOpts, DiagnosticsEngine &Diags) {
if (!AnOpts.ShouldEmitErrorsOnInvalidConfigValue)
return;
using CmdLineOption = CheckerRegistry::CmdLineOption;
auto SameOptName = [SuppliedOption](const CmdLineOption &Opt) {
return Opt.OptionName == SuppliedOption;
};
[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
2020-05-27 18:29:47 +08:00
const auto *OptionIt = llvm::find_if(OptionList, SameOptName);
if (OptionIt == OptionList.end()) {
Diags.Report(diag::err_analyzer_checker_option_unknown)
<< SuppliedChecker << SuppliedOption;
return;
}
}
void CheckerRegistry::validateCheckerOptions() const {
for (const auto &Config : AnOpts.Config) {
StringRef SuppliedCheckerOrPackage;
StringRef SuppliedOption;
std::tie(SuppliedCheckerOrPackage, SuppliedOption) =
Config.getKey().split(':');
if (SuppliedOption.empty())
continue;
// AnalyzerOptions' config table contains the user input, so an entry could
// look like this:
//
// cor:NoFalsePositives=true
//
// Since lower_bound would look for the first element *not less* than "cor",
// it would return with an iterator to the first checker in the core, so we
// we really have to use find here, which uses operator==.
auto CheckerIt =
llvm::find(Checkers, CheckerInfo(SuppliedCheckerOrPackage));
if (CheckerIt != Checkers.end()) {
isOptionContainedIn(CheckerIt->CmdLineOptions, SuppliedCheckerOrPackage,
SuppliedOption, AnOpts, Diags);
continue;
}
[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
2020-05-27 18:29:47 +08:00
const auto *PackageIt =
llvm::find(Packages, PackageInfo(SuppliedCheckerOrPackage));
if (PackageIt != Packages.end()) {
isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedCheckerOrPackage,
SuppliedOption, AnOpts, Diags);
continue;
}
Diags.Report(diag::err_unknown_analyzer_checker_or_package)
<< SuppliedCheckerOrPackage;
}
}
//===----------------------------------------------------------------------===//
// Printing functions.
//===----------------------------------------------------------------------===//
void CheckerRegistry::printCheckerWithDescList(raw_ostream &Out,
size_t MaxNameChars) const {
// FIXME: Print available packages.
Out << "CHECKERS:\n";
// Find the maximum option length.
size_t OptionFieldWidth = 0;
for (const auto &Checker : Checkers) {
// Limit the amount of padding we are willing to give up for alignment.
// Package.Name Description [Hidden]
size_t NameLength = Checker.FullName.size();
if (NameLength <= MaxNameChars)
OptionFieldWidth = std::max(OptionFieldWidth, NameLength);
}
const size_t InitialPad = 2;
auto Print = [=](llvm::raw_ostream &Out, const CheckerInfo &Checker,
StringRef Description) {
AnalyzerOptions::printFormattedEntry(Out, {Checker.FullName, Description},
InitialPad, OptionFieldWidth);
Out << '\n';
};
for (const auto &Checker : Checkers) {
// The order of this if branches is significant, we wouldn't like to display
// developer checkers even in the alpha output. For example,
// alpha.cplusplus.IteratorModeling is a modeling checker, hence it's hidden
// by default, and users (even when the user is a developer of an alpha
// checker) shouldn't normally tinker with whether they should be enabled.
if (Checker.IsHidden) {
if (AnOpts.ShowCheckerHelpDeveloper)
Print(Out, Checker, Checker.Desc);
continue;
}
if (Checker.FullName.startswith("alpha")) {
if (AnOpts.ShowCheckerHelpAlpha)
Print(Out, Checker,
("(Enable only for development!) " + Checker.Desc).str());
continue;
}
if (AnOpts.ShowCheckerHelp)
Print(Out, Checker, Checker.Desc);
}
}
void CheckerRegistry::printEnabledCheckerList(raw_ostream &Out) const {
for (const auto *i : EnabledCheckers)
Out << i->FullName << '\n';
}
void CheckerRegistry::printCheckerOptionList(raw_ostream &Out) const {
Out << "OVERVIEW: Clang Static Analyzer Checker and Package Option List\n\n";
Out << "USAGE: -analyzer-config <OPTION1=VALUE,OPTION2=VALUE,...>\n\n";
Out << " -analyzer-config OPTION1=VALUE, -analyzer-config "
"OPTION2=VALUE, ...\n\n";
Out << "OPTIONS:\n\n";
std::multimap<StringRef, const CmdLineOption &> OptionMap;
for (const CheckerInfo &Checker : Checkers) {
for (const CmdLineOption &Option : Checker.CmdLineOptions) {
OptionMap.insert({Checker.FullName, Option});
}
}
for (const PackageInfo &Package : Packages) {
for (const CmdLineOption &Option : Package.CmdLineOptions) {
OptionMap.insert({Package.FullName, Option});
}
}
auto Print = [] (llvm::raw_ostream &Out, StringRef FullOption, StringRef Desc) {
AnalyzerOptions::printFormattedEntry(Out, {FullOption, Desc},
/*InitialPad*/ 2,
/*EntryWidth*/ 50,
/*MinLineWidth*/ 90);
Out << "\n\n";
};
for (const std::pair<const StringRef, const CmdLineOption &> &Entry :
OptionMap) {
const CmdLineOption &Option = Entry.second;
std::string FullOption = (Entry.first + ":" + Option.OptionName).str();
std::string Desc =
("(" + Option.OptionType + ") " + Option.Description + " (default: " +
(Option.DefaultValStr.empty() ? "\"\"" : Option.DefaultValStr) + ")")
.str();
// The list of these if branches is significant, we wouldn't like to
// display hidden alpha checker options for
// -analyzer-checker-option-help-alpha.
if (Option.IsHidden) {
if (AnOpts.ShowCheckerOptionDeveloperList)
Print(Out, FullOption, Desc);
continue;
}
if (Option.DevelopmentStatus == "alpha" ||
Entry.first.startswith("alpha")) {
if (AnOpts.ShowCheckerOptionAlphaList)
Print(Out, FullOption,
llvm::Twine("(Enable only for development!) " + Desc).str());
continue;
}
if (AnOpts.ShowCheckerOptionList)
Print(Out, FullOption, Desc);
}
}