Hi Kristóf, There is clearly a typo in checking for -ctu-dir and -model-path being a directory. Instead of checking -model-path, the code was checking for -ctu-dir twice. Landed a fix in r348125.
On Fri, Nov 30, 2018 at 10:27 PM Kristof Umann via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: szelethus > Date: Fri Nov 30 13:24:31 2018 > New Revision: 348038 > > URL: http://llvm.org/viewvc/llvm-project?rev=348038&view=rev > Log: > [analyzer] Emit an error for invalid -analyzer-config inputs > > Differential Revision: https://reviews.llvm.org/D53280 > > Added: > cfe/trunk/test/Analysis/invalid-analyzer-config-value.c > Modified: > cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td > cfe/trunk/include/clang/Driver/CC1Options.td > cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h > cfe/trunk/lib/Driver/ToolChains/Clang.cpp > cfe/trunk/lib/Frontend/CompilerInvocation.cpp > > Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=348038&r1=348037&r2=348038&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Nov 30 > 13:24:31 2018 > @@ -299,6 +299,9 @@ def err_analyzer_config_no_value : Error > "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">; > > Modified: cfe/trunk/include/clang/Driver/CC1Options.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=348038&r1=348037&r2=348038&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Driver/CC1Options.td (original) > +++ cfe/trunk/include/clang/Driver/CC1Options.td Fri Nov 30 13:24:31 2018 > @@ -138,6 +138,12 @@ def analyzer_list_enabled_checkers : Fla > 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<analyzer_config_compatibility_mode>; > + > > > //===----------------------------------------------------------------------===// > // Migrator Options > > > //===----------------------------------------------------------------------===// > > Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=348038&r1=348037&r2=348038&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h > (original) > +++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Fri Nov > 30 13:24:31 2018 > @@ -200,6 +200,7 @@ public: > unsigned ShowCheckerHelp : 1; > unsigned ShowEnabledCheckerList : 1; > unsigned ShowConfigOptionsList : 1; > + unsigned ShouldEmitErrorsOnInvalidConfigValue : 1; > unsigned AnalyzeAll : 1; > unsigned AnalyzerDisplayProgress : 1; > unsigned AnalyzeNestedBlocks : 1; > @@ -222,6 +223,7 @@ public: > /// The mode of function selection used during inlining. > AnalysisInliningMode InliningMode = NoRedundancy; > > + // Create a field for each -analyzer-config option. > #define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, > \ > SHALLOW_VAL, DEEP_VAL) > \ > ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL) > @@ -233,13 +235,39 @@ public: > #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<StringRef> 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), ShowConfigOptionsList(false), > AnalyzeAll(false), AnalyzerDisplayProgress(false), > AnalyzeNestedBlocks(false), eagerlyAssumeBinOpBifurcation(false), > TrimGraph(false), visualizeExplodedGraphWithGraphViz(false), > - UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false) > {} > + UnoptimizedCFG(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. > > Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=348038&r1=348037&r2=348038&view=diff > > ============================================================================== > --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original) > +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Nov 30 13:24:31 2018 > @@ -2368,6 +2368,9 @@ static void RenderAnalyzerOptions(const > // 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"); > > Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=348038&r1=348037&r2=348038&view=diff > > ============================================================================== > --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original) > +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Nov 30 13:24:31 2018 > @@ -181,8 +181,10 @@ static void addDiagnosticArgs(ArgList &A > } > } > > +// 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<std::string> &Funcs) { > @@ -284,6 +286,12 @@ static bool ParseAnalyzerArgs(AnalyzerOp > 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<bool>( > + > 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 @@ static bool ParseAnalyzerArgs(AnalyzerOp > > // 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 @@ static bool ParseAnalyzerArgs(AnalyzerOp > 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 StringRef getStringOption(Analyze > } > > 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<bool>(getStringOption(Config, Name, > - (DefaultVal ? "true" : > "false"))) > + auto PossiblyInvalidVal = llvm::StringSwitch<Optional<bool>>( > + 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) { > > Added: cfe/trunk/test/Analysis/invalid-analyzer-config-value.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/invalid-analyzer-config-value.c?rev=348038&view=auto > > ============================================================================== > --- cfe/trunk/test/Analysis/invalid-analyzer-config-value.c (added) > +++ cfe/trunk/test/Analysis/invalid-analyzer-config-value.c Fri Nov 30 > 13:24:31 2018 > @@ -0,0 +1,71 @@ > +// 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: 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() {} > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > -- Regards, Ilya Biryukov
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
