r361011 - [analyzer] Validate checker option names and values
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Fri May 17 09:28:34 PDT 2019
Looks like your r361042 fixed the ppc64be bot nonetheless. Maybe it was
related after all :)
*From: *Kristof Umann <kristof.umann at ericsson.com>
*Date: *Fri, May 17, 2019 at 11:10 AM
*To: *Nico Weber
*Cc: *cfe-commits
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>
> *Date: *Fri, May 17, 2019 at 5:49 AM
> *To: *<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
> 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/533645f6/attachment-0001.html>
More information about the cfe-commits
mailing list