From: Kristof Umann Date: Fri, 17 May 2019 09:51:59 +0000 (+0000) Subject: [analyzer] Validate checker option names and values X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=37bdceb8805f914e4237882fe6dc603a9b97a4f1;p=clang [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@361011 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticDriverKinds.td b/include/clang/Basic/DiagnosticDriverKinds.td index 9885802002..3c2cedbaf9 100644 --- a/include/clang/Basic/DiagnosticDriverKinds.td +++ b/include/clang/Basic/DiagnosticDriverKinds.td @@ -307,6 +307,8 @@ def err_analyzer_config_multiple_values : Error< 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">; diff --git a/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h b/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h index c6cb8ac631..875cb8edb8 100644 --- a/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ b/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h @@ -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) diff --git a/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp index 7eef09b883..437e4aaf1a 100644 --- a/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ b/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -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(StringRef FullName, StringRef Dependency) { 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 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::resolveCheckerAndPackageOptions() { for (const std::pair &CheckerOptEntry : CheckerOptions) { insertOptionToCollection(CheckerOptEntry.first, Checkers, - CheckerOptEntry.second, AnOpts); + CheckerOptEntry.second, AnOpts, Diags); } CheckerOptions.clear(); for (const std::pair &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(CheckerManager &CheckerMgr) const { } } +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; } - if (!HasChecker) - Diags.Report(diag::err_unknown_analyzer_checker) << CheckerName; + + auto PackageIt = llvm::find(Packages, PackageInfo(SuppliedChecker)); + if (PackageIt != Packages.end()) { + isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedChecker, + SuppliedOption, AnOpts, Diags); + continue; + } + + Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker; } } diff --git a/test/Analysis/checker-plugins.c b/test/Analysis/checker-plugins.c index 1e0d6c6341..0745357e52 100644 --- a/test/Analysis/checker-plugins.c +++ b/test/Analysis/checker-plugins.c @@ -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 diff --git a/test/Analysis/invalid-checker-option.c b/test/Analysis/invalid-checker-option.c index 4ce783539f..26dfcb2d56 100644 --- a/test/Analysis/invalid-checker-option.c +++ b/test/Analysis/invalid-checker-option.c @@ -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() {}