Index: include/clang/Basic/DiagnosticDriverKinds.td =================================================================== --- include/clang/Basic/DiagnosticDriverKinds.td +++ include/clang/Basic/DiagnosticDriverKinds.td @@ -303,6 +303,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: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h =================================================================== --- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ 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; } }; Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp =================================================================== --- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -315,6 +315,50 @@ CheckerIt->Dependencies.emplace_back(&*DependencyIt); } +/// 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 OptionType, StringRef FullName, + StringRef OptionName, StringRef DefaultValue, + AnalyzerOptions &AnOpts, + DiagnosticsEngine &Diags) { + + std::string FullOption = (FullName + ":" + OptionName).str(); + + auto It = AnOpts.Config.insert({FullOption, DefaultValue}); + + // 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 (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 (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; + } +} + void CheckerRegistry::addCheckerOption(StringRef OptionType, StringRef FullName, StringRef OptionName, @@ -329,7 +373,8 @@ CheckerIt->CmdLineOptions.emplace_back( OptionType, OptionName, DefaultValStr, Description); - AnOpts.Config.insert({(FullName + ":" + OptionName).str(), DefaultValStr}); + insertAndValidate( + OptionType, FullName, OptionName, DefaultValStr, AnOpts, Diags); } void CheckerRegistry::addPackage(StringRef FullName) { @@ -350,7 +395,8 @@ PackageIt->CmdLineOptions.emplace_back( OptionType, OptionName, DefaultValStr, Description); - AnOpts.Config.insert({(FullName + ":" + OptionName).str(), DefaultValStr}); + insertAndValidate( + OptionType, FullName, OptionName, DefaultValStr, AnOpts, Diags); } void CheckerRegistry::initializeManager(CheckerManager &checkerMgr) const { @@ -364,13 +410,38 @@ } } +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) - continue; + for (const auto &Config : AnOpts.Config) { + + StringRef SuppliedChecker; + StringRef SuppliedOption; + std::tie(SuppliedChecker, SuppliedOption) = Config.getKey().split(':'); - StringRef SuppliedChecker(config.first().substr(0, pos)); + if (SuppliedOption.empty()) + continue; // AnalyzerOptions' config table contains the user input, so it entry could // look like this: @@ -381,12 +452,18 @@ // 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()) + if (CheckerIt != Checkers.end()) { + isOptionContainedIn(CheckerIt->CmdLineOptions, SuppliedChecker, + SuppliedOption, AnOpts, Diags); continue; + } auto PackageIt = llvm::find(Packages, PackageInfo(SuppliedChecker)); - if (PackageIt != Packages.end()) + if (PackageIt != Packages.end()) { + isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedChecker, + SuppliedOption, AnOpts, Diags); continue; + } Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker; } Index: test/Analysis/invalid-checker-option.c =================================================================== --- test/Analysis/invalid-checker-option.c +++ 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() {}