Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td =================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td @@ -307,6 +307,8 @@ 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">; Index: cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h @@ -107,6 +107,17 @@ 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 @@ 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) Index: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ cfe/trunk/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 @@ 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 @@ 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 @@ } } +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; } } Index: cfe/trunk/test/Analysis/checker-plugins.c =================================================================== --- cfe/trunk/test/Analysis/checker-plugins.c +++ cfe/trunk/test/Analysis/checker-plugins.c @@ -62,3 +62,35 @@ // 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 Index: cfe/trunk/test/Analysis/invalid-checker-option.c =================================================================== --- cfe/trunk/test/Analysis/invalid-checker-option.c +++ cfe/trunk/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() {}