r361011 - [analyzer] Validate checker option names and values
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Fri May 17 08:10:25 PDT 2019
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick seems to be doing fine now, http://lab.llvm.org:8011/builders/clang-ppc64be-linux seems to crash on files totally unrelated to mine.
________________________________
From: Kristof Umann
Sent: 17 May 2019 15:59:28
To: Nico Weber
Cc: cfe-commits
Subject: Re: r361011 - [analyzer] Validate checker option names and values
I'll take a look.
________________________________
From: Nico Weber <thakis at chromium.org>
Sent: 17 May 2019 15:09:18
To: Kristof Umann
Cc: cfe-commits
Subject: Re: r361011 - [analyzer] Validate checker option names and values
It looks like this change is making gcc-7 crash on these (and other http://lab.llvm.org:8011/console) bots:
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/18639
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/33837/
[100/212] Building CXX object tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/ASTImporterTest.cpp.o
FAILED: tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/ASTImporterTest.cpp.o
...
g++-7: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.
[101/212] Building CXX object tools/clang/unittests/ASTMatchers/CMakeFiles/ASTMatchersTests.dir/ASTMatchersTraversalTest.cpp.o
FAILED: tools/clang/unittests/ASTMatchers/CMakeFiles/ASTMatchersTests.dir/ASTMatchersTraversalTest.cpp.o
...
g++-7: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.
[102/212] Building CXX object tools/clang/unittests/ASTMatchers/CMakeFiles/ASTMatchersTests.dir/ASTMatchersNarrowingTest.cpp.o
FAILED: tools/clang/unittests/ASTMatchers/CMakeFiles/ASTMatchersTests.dir/ASTMatchersNarrowingTest.cpp.o
...
g++-7: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.
[103/212] Building CXX object tools/clang/unittests/ASTMatchers/CMakeFiles/ASTMatchersTests.dir/ASTMatchersNodeTest.cpp.o
FAILED: tools/clang/unittests/ASTMatchers/CMakeFiles/ASTMatchersTests.dir/ASTMatchersNodeTest.cpp.o
...
g++-7: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.
[104/212] Building CXX object tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/CommentHandlerTest.cpp.o
FAILED: tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/CommentHandlerTest.cpp.o
...
g++-7: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.
[105/212] Building CXX object tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/ExecutionTest.cpp.o
FAILED: tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/ExecutionTest.cpp.o
...
g++-7: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.
[106/212] Building CXX object tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/ASTImporterVisibilityTest.cpp.o
FAILED: tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/ASTImporterVisibilityTest.cpp.o
...
g++-7: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.
From: Kristof Umann via cfe-commits <cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>>
Date: Fri, May 17, 2019 at 5:49 AM
To: <cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>>
Author: szelethus
Date: Fri May 17 02:51:59 2019
New Revision: 361011
URL: http://llvm.org/viewvc/llvm-project?rev=361011&view=rev
Log:
[analyzer] Validate checker option names and values
Validate whether the option exists, and also whether the supplied value is of
the correct type. With this patch, invoking the analyzer should be, at least
in the frontend mode, a lot safer.
Differential Revision: https://reviews.llvm.org/D57860
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
cfe/trunk/test/Analysis/checker-plugins.c
cfe/trunk/test/Analysis/invalid-checker-option.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=361011&r1=361010&r2=361011&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri May 17 02:51:59 2019
@@ -307,6 +307,8 @@ def err_analyzer_config_multiple_values
def err_analyzer_config_invalid_input : Error<
"invalid input for analyzer-config option '%0', that expects %1 value">;
def err_analyzer_config_unknown : Error<"unknown analyzer-config '%0'">;
+def err_analyzer_checker_option_unknown : Error<
+ "checker '%0' has no option called '%1'">;
def err_analyzer_checker_option_invalid_input : Error<
"invalid input for checker option '%0', that expects %1">;
Modified: cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h?rev=361011&r1=361010&r2=361011&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h Fri May 17 02:51:59 2019
@@ -107,6 +107,17 @@ public:
assert((OptionType == "bool" || OptionType == "string" ||
OptionType == "int") &&
"Unknown command line option type!");
+
+ assert((OptionType != "bool" ||
+ (DefaultValStr == "true" || DefaultValStr == "false")) &&
+ "Invalid value for boolean command line option! Maybe incorrect "
+ "parameters to the addCheckerOption or addPackageOption method?");
+
+ int Tmp;
+ assert((OptionType != "int" || !DefaultValStr.getAsInteger(0, Tmp)) &&
+ "Invalid value for integer command line option! Maybe incorrect "
+ "parameters to the addCheckerOption or addPackageOption method?");
+ (void)Tmp;
}
};
@@ -150,6 +161,12 @@ public:
return State == StateFromCmdLine::State_Disabled && ShouldRegister(LO);
}
+ // Since each checker must have a different full name, we can identify
+ // CheckerInfo objects by them.
+ bool operator==(const CheckerInfo &Rhs) const {
+ return FullName == Rhs.FullName;
+ }
+
CheckerInfo(InitializationFunction Fn, ShouldRegisterFunction sfn,
StringRef Name, StringRef Desc, StringRef DocsUri,
bool IsHidden)
Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp?rev=361011&r1=361010&r2=361011&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp Fri May 17 02:51:59 2019
@@ -9,6 +9,7 @@
#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"
@@ -306,18 +307,61 @@ void CheckerRegistry::addDependency(Stri
Dependencies.emplace_back(FullName, Dependency);
}
+/// 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, 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 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";
+ }
+ }
+ 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";
+ }
+ }
+ return;
+ }
+}
+
template <class T>
static void
insertOptionToCollection(StringRef FullName, T &Collection,
const CheckerRegistry::CmdLineOption &Option,
- AnalyzerOptions &AnOpts) {
+ AnalyzerOptions &AnOpts, DiagnosticsEngine &Diags) {
auto It = binaryFind(Collection, FullName);
assert(It != Collection.end() &&
"Failed to find the checker while attempting to add a command line "
"option to it!");
- AnOpts.Config.insert(
- {(FullName + ":" + Option.OptionName).str(), Option.DefaultValStr});
+ insertAndValidate(FullName, Option, AnOpts, Diags);
It->CmdLineOptions.emplace_back(Option);
}
@@ -326,14 +370,14 @@ void CheckerRegistry::resolveCheckerAndP
for (const std::pair<StringRef, CmdLineOption> &CheckerOptEntry :
CheckerOptions) {
insertOptionToCollection(CheckerOptEntry.first, Checkers,
- CheckerOptEntry.second, AnOpts);
+ CheckerOptEntry.second, AnOpts, Diags);
}
CheckerOptions.clear();
for (const std::pair<StringRef, CmdLineOption> &PackageOptEntry :
PackageOptions) {
- insertOptionToCollection(PackageOptEntry.first, Checkers,
- PackageOptEntry.second, AnOpts);
+ insertOptionToCollection(PackageOptEntry.first, Packages,
+ PackageOptEntry.second, AnOpts, Diags);
}
PackageOptions.clear();
}
@@ -388,23 +432,62 @@ void CheckerRegistry::initializeManager(
}
}
+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;
+ };
+
+ 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) {
- size_t Pos = Config.getKey().find(':');
- if (Pos == StringRef::npos)
+
+ StringRef SuppliedChecker;
+ StringRef SuppliedOption;
+ std::tie(SuppliedChecker, SuppliedOption) = Config.getKey().split(':');
+
+ if (SuppliedOption.empty())
continue;
- bool HasChecker = false;
- StringRef CheckerName = Config.getKey().substr(0, Pos);
- for (const auto &Checker : Checkers) {
- if (Checker.FullName.startswith(CheckerName) &&
- (Checker.FullName.size() == Pos || Checker.FullName[Pos] == '.')) {
- HasChecker = true;
- break;
- }
+ // 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(SuppliedChecker));
+ if (CheckerIt != Checkers.end()) {
+ isOptionContainedIn(CheckerIt->CmdLineOptions, SuppliedChecker,
+ SuppliedOption, AnOpts, Diags);
+ continue;
+ }
+
+ auto PackageIt = llvm::find(Packages, PackageInfo(SuppliedChecker));
+ if (PackageIt != Packages.end()) {
+ isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedChecker,
+ SuppliedOption, AnOpts, Diags);
+ continue;
}
- if (!HasChecker)
- Diags.Report(diag::err_unknown_analyzer_checker) << CheckerName;
+
+ Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker;
}
}
Modified: cfe/trunk/test/Analysis/checker-plugins.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/checker-plugins.c?rev=361011&r1=361010&r2=361011&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/checker-plugins.c (original)
+++ cfe/trunk/test/Analysis/checker-plugins.c Fri May 17 02:51:59 2019
@@ -62,3 +62,35 @@ void caller() {
// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-TRUE
// CHECK-CHECKER-OPTION-TRUE: example.MyChecker:ExampleOption = true
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN: -analyzer-checker=example.MyChecker \
+// RUN: -analyzer-config example.MyChecker:Example=true \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER-OPTION
+
+// CHECK-NON-EXISTENT-CHECKER-OPTION: (frontend): checker 'example.MyChecker'
+// CHECK-NON-EXISTENT-CHECKER-OPTION-SAME: has no option called 'Example'
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN: -analyzer-checker=example.MyChecker \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config example.MyChecker:Example=true
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN: -analyzer-checker=example.MyChecker \
+// RUN: -analyzer-config example.MyChecker:ExampleOption=example \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-BOOL-VALUE
+
+// CHECK-INVALID-BOOL-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-BOOL-VALUE-SAME: 'example.MyChecker:ExampleOption', that
+// CHECK-INVALID-BOOL-VALUE-SAME: expects a boolean value
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN: -analyzer-checker=example.MyChecker \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config example.MyChecker:ExampleOption=example
Modified: cfe/trunk/test/Analysis/invalid-checker-option.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/invalid-checker-option.c?rev=361011&r1=361010&r2=361011&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/invalid-checker-option.c (original)
+++ cfe/trunk/test/Analysis/invalid-checker-option.c Fri May 17 02:51:59 2019
@@ -14,6 +14,63 @@
// CHECK-NON-EXISTENT-CHECKER: (frontend): no analyzer checkers or packages
// CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
+
+// Every other error should be avoidable in compatiblity mode.
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config debug.AnalysisOrder:Everything=false \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER-OPTION
+
+// CHECK-NON-EXISTENT-CHECKER-OPTION: (frontend): checker 'debug.AnalysisOrder'
+// CHECK-NON-EXISTENT-CHECKER-OPTION-SAME: has no option called 'Everything'
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config debug.AnalysisOrder:Everything=false
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config debug.AnalysisOrder:*=nothankyou \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-BOOL-VALUE
+
+// CHECK-INVALID-BOOL-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-BOOL-VALUE-SAME: 'debug.AnalysisOrder:*', that expects a
+// CHECK-INVALID-BOOL-VALUE-SAME: boolean value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config debug.AnalysisOrder:*=nothankyou
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config optin.performance.Padding:AllowedPad=surpriseme \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-INT-VALUE
+
+// CHECK-INVALID-INT-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-INT-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-INVALID-INT-VALUE-SAME: expects an integer value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config-compatibility-mode=true \
+// RUN: -analyzer-config optin.performance.Padding:AllowedPad=surpriseme
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=sure \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-PACKAGE-VALUE
+
+// CHECK-PACKAGE-VALUE: (frontend): invalid input for checker option
+// CHECK-PACKAGE-VALUE-SAME: 'nullability:NoDiagnoseCallsToSystemHeaders', that
+// CHECK-PACKAGE-VALUE-SAME: expects a boolean value
+
// expected-no-diagnostics
int main() {}
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190517/9a31b4c7/attachment-0001.html>
More information about the cfe-commits
mailing list