Index: include/clang/Basic/DiagnosticDriverKinds.td =================================================================== --- include/clang/Basic/DiagnosticDriverKinds.td +++ include/clang/Basic/DiagnosticDriverKinds.td @@ -296,6 +296,9 @@ "analyzer-config option '%0' has a key but no value">; def err_analyzer_config_multiple_values : Error< "analyzer-config option '%0' should contain only one '='">; +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_drv_invalid_hvx_length : Error< "-mhvx-length is not supported without a -mhvx/-mhvx= flag">; Index: include/clang/Driver/CC1Options.td =================================================================== --- include/clang/Driver/CC1Options.td +++ include/clang/Driver/CC1Options.td @@ -138,6 +138,12 @@ def analyzer_config : Separate<["-"], "analyzer-config">, HelpText<"Choose analyzer options to enable">; +def analyzer_config_compatibility_mode : Separate<["-"], "analyzer-config-compatibility-mode">, + HelpText<"Don't emit errors on invalid analyzer-config inputs">; + +def analyzer_config_compatibility_mode_EQ : Joined<["-"], "analyzer-config-compatibility-mode=">, + Alias; + //===----------------------------------------------------------------------===// // Migrator Options //===----------------------------------------------------------------------===// Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h =================================================================== --- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -200,6 +200,7 @@ unsigned ShowCheckerHelp : 1; unsigned ShowEnabledCheckerList : 1; unsigned ShowConfigOptionsList : 1; + unsigned ShouldEmitErrorsOnInvalidConfigValue : 1; unsigned AnalyzeAll : 1; unsigned AnalyzerDisplayProgress : 1; unsigned AnalyzeNestedBlocks : 1; @@ -227,12 +228,37 @@ ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL) #define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \ + /* Create a field for each -analyzer-config option. */ \ TYPE NAME; #include "clang/StaticAnalyzer/Core/AnalyzerOptions.def" #undef ANALYZER_OPTION #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE + // Create an array of all -analyzer-config command line options. Sort it in + // the constructor. + std::vector AnalyzerConfigCmdFlags = { +#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \ + SHALLOW_VAL, DEEP_VAL) \ + ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL) + +#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \ + CMDFLAG, + +#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def" +#undef ANALYZER_OPTION +#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE + }; + + bool isUnknownAnalyzerConfig(StringRef Name) const { + + assert(std::is_sorted(AnalyzerConfigCmdFlags.begin(), + AnalyzerConfigCmdFlags.end())); + + return !std::binary_search(AnalyzerConfigCmdFlags.begin(), + AnalyzerConfigCmdFlags.end(), Name); + } + AnalyzerOptions() : DisableAllChecks(false), ShowCheckerHelp(false), ShowEnabledCheckerList(false), AnalyzeAll(false), @@ -240,7 +266,9 @@ eagerlyAssumeBinOpBifurcation(false), TrimGraph(false), visualizeExplodedGraphWithGraphViz(false), UnoptimizedCFG(false), - PrintStats(false), NoRetryExhausted(false) {} + PrintStats(false), NoRetryExhausted(false) { + llvm::sort(AnalyzerConfigCmdFlags); + } /// Interprets an option's string value as a boolean. The "true" string is /// interpreted as true and the "false" string is interpreted as false. Index: lib/Driver/ToolChains/Clang.cpp =================================================================== --- lib/Driver/ToolChains/Clang.cpp +++ lib/Driver/ToolChains/Clang.cpp @@ -2344,6 +2344,9 @@ // Treat blocks as analysis entry points. CmdArgs.push_back("-analyzer-opt-analyze-nested-blocks"); + // Enable compatilibily mode to avoid analyzer-config related errors. + CmdArgs.push_back("-analyzer-config-compatibility-mode=true"); + // Add default argument set. if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) { CmdArgs.push_back("-analyzer-checker=core"); Index: lib/Frontend/CompilerInvocation.cpp =================================================================== --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -181,8 +181,10 @@ } } +// Parse the Static Analyzer configuration. If \p Diags is set to nullptr, +// it won't verify the input. static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts, - DiagnosticsEngine &Diags); + DiagnosticsEngine *Diags); static void getAllNoBuiltinFuncValues(ArgList &Args, std::vector &Funcs) { @@ -284,6 +286,12 @@ Opts.ShowCheckerHelp = Args.hasArg(OPT_analyzer_checker_help); Opts.ShowConfigOptionsList = Args.hasArg(OPT_analyzer_config_help); Opts.ShowEnabledCheckerList = Args.hasArg(OPT_analyzer_list_enabled_checkers); + Opts.ShouldEmitErrorsOnInvalidConfigValue = + /* negated */!llvm::StringSwitch( + Args.getLastArgValue(OPT_analyzer_config_compatibility_mode)) + .Case("true", true) + .Case("false", false) + .Default(false); Opts.DisableAllChecks = Args.hasArg(OPT_analyzer_disable_all_checks); Opts.visualizeExplodedGraphWithGraphViz = @@ -320,7 +328,7 @@ // Go through the analyzer configuration options. for (const auto *A : Args.filtered(OPT_analyzer_config)) { - A->claim(); + // We can have a list of comma separated config names, e.g: // '-analyzer-config key1=val1,key2=val2' StringRef configList = A->getValue(); @@ -342,11 +350,24 @@ Success = false; break; } + + // TODO: Check checker options too, possibly in CheckerRegistry. + // Leave unknown non-checker configs unclaimed. + if (!key.contains(":") && Opts.isUnknownAnalyzerConfig(key)) { + if (Opts.ShouldEmitErrorsOnInvalidConfigValue) + Diags.Report(diag::err_analyzer_config_unknown) << key; + continue; + } + + A->claim(); Opts.Config[key] = val; } } - parseAnalyzerConfigs(Opts, Diags); + if (Opts.ShouldEmitErrorsOnInvalidConfigValue) + parseAnalyzerConfigs(Opts, &Diags); + else + parseAnalyzerConfigs(Opts, nullptr); llvm::raw_string_ostream os(Opts.FullCompilerInvocation); for (unsigned i = 0; i < Args.getNumInputArgStrings(); ++i) { @@ -365,56 +386,82 @@ } static void initOption(AnalyzerOptions::ConfigTable &Config, + DiagnosticsEngine *Diags, StringRef &OptionField, StringRef Name, StringRef DefaultVal) { + // String options may be known to invalid (e.g. if the expected string is a + // file name, but the file does not exist), those will have to be checked in + // parseConfigs. OptionField = getStringOption(Config, Name, DefaultVal); } static void initOption(AnalyzerOptions::ConfigTable &Config, + DiagnosticsEngine *Diags, bool &OptionField, StringRef Name, bool DefaultVal) { - // FIXME: We should emit a warning here if the value is something other than - // "true", "false", or the empty string (meaning the default value), - // but the AnalyzerOptions doesn't have access to a diagnostic engine. - OptionField = llvm::StringSwitch(getStringOption(Config, Name, - (DefaultVal ? "true" : "false"))) + auto PossiblyInvalidVal = llvm::StringSwitch>( + getStringOption(Config, Name, (DefaultVal ? "true" : "false"))) .Case("true", true) .Case("false", false) - .Default(DefaultVal); + .Default(None); + + if (!PossiblyInvalidVal) { + if (Diags) + Diags->Report(diag::err_analyzer_config_invalid_input) + << Name << "a boolean"; + else + OptionField = DefaultVal; + } else + OptionField = PossiblyInvalidVal.getValue(); } static void initOption(AnalyzerOptions::ConfigTable &Config, + DiagnosticsEngine *Diags, unsigned &OptionField, StringRef Name, unsigned DefaultVal) { + OptionField = DefaultVal; bool HasFailed = getStringOption(Config, Name, std::to_string(DefaultVal)) .getAsInteger(10, OptionField); - assert(!HasFailed && "analyzer-config option should be numeric"); - (void)HasFailed; + if (Diags && HasFailed) + Diags->Report(diag::err_analyzer_config_invalid_input) + << Name << "an unsigned"; } static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts, - DiagnosticsEngine &Diags) { - // TODO: Emit warnings for incorrect options. + DiagnosticsEngine *Diags) { // TODO: There's no need to store the entire configtable, it'd be plenty // enough tostore checker options. #define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \ - initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, DEFAULT_VAL); \ + initOption(AnOpts.Config, Diags, AnOpts.NAME, CMDFLAG, DEFAULT_VAL); #define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \ SHALLOW_VAL, DEEP_VAL) \ switch (AnOpts.getUserMode()) { \ case UMK_Shallow: \ - initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, SHALLOW_VAL); \ + initOption(AnOpts.Config, Diags, AnOpts.NAME, CMDFLAG, SHALLOW_VAL); \ break; \ case UMK_Deep: \ - initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, DEEP_VAL); \ + initOption(AnOpts.Config, Diags, AnOpts.NAME, CMDFLAG, DEEP_VAL); \ break; \ } \ #include "clang/StaticAnalyzer/Core/AnalyzerOptions.def" #undef ANALYZER_OPTION #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE + + // At this point, AnalyzerOptions is configured. Let's validate some options. + + if (!Diags) + return; + + if (!AnOpts.CTUDir.empty() && !llvm::sys::fs::is_directory(AnOpts.CTUDir)) + Diags->Report(diag::err_analyzer_config_invalid_input) + << "ctu-dir" << "a filename"; + + if (!AnOpts.CTUDir.empty() && !llvm::sys::fs::is_directory(AnOpts.CTUDir)) + Diags->Report(diag::err_analyzer_config_invalid_input) + << "model-path" << "a filename"; } static bool ParseMigratorArgs(MigratorOptions &Opts, ArgList &Args) { Index: test/Analysis/invalid-analyzer-config-value.c =================================================================== --- /dev/null +++ test/Analysis/invalid-analyzer-config-value.c @@ -0,0 +1,72 @@ +// RUN: not %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config notes-as-events=yesplease \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-BOOL-INPUT + +// CHECK-BOOL-INPUT: (frontend): invalid input for analyzer-config option +// CHECK-BOOL-INPUT-SAME: 'notes-as-events', that expects a boolean value + +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config-compatibility-mode=true \ +// RUN: -analyzer-config notes-as-events=yesplease + + +// RUN: not %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config max-inlinable-size=400km/h \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-UINT-INPUT + +// CHECK-UINT-INPUT: (frontend): invalid input for analyzer-config option +// CHECK-UINT-INPUT-SAME: 'max-inlinable-size', that expects an unsigned +// CHECK-UINT-INPUT-SAME: value + +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config-compatibility-mode=true \ +// RUN: -analyzer-config max-inlinable-size=400km/h + + +// RUN: not %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config ctu-dir=0123012301230123 \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-FILENAME-INPUT + +// CHECK-FILENAME-INPUT: (frontend): invalid input for analyzer-config option +// CHECK-FILENAME-INPUT-SAME: 'ctu-dir', that expects a filename +// CHECK-FILENAME-INPUT-SAME: value + +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config-compatibility-mode=true \ +// RUN: -analyzer-config ctu-dir=0123012301230123 + + +// RUN: not %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config no-false-positives=true \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-CFG + +// CHECK-UNKNOWN-CFG: (frontend): unknown analyzer-config 'no-false-positives' + +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config-compatibility-mode=true \ +// RUN: -analyzer-config no-false-positives=true + + +// Test the driver properly using "analyzer-config-compatibility-mode=true", +// no longer causing an error on input error. +// RUN: %clang --analyze %s \ +// RUN: -Xclang -analyzer-config -Xclang no-false-positives=true + +// RUN: not %clang --analyze %s \ +// RUN: -Xclang -analyzer-config -Xclang no-false-positives=true \ +// RUN: -Xclang -analyzer-config-compatibility-mode=false \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-NO-COMPAT + +// CHECK-NO-COMPAT: error: unknown analyzer-config 'no-false-positives' + +// expected-no-diagnostics + +int main() {}