r348038 - [analyzer] Emit an error for invalid -analyzer-config inputs
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 3 03:38:47 PST 2018
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 at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181203/fbd6fc23/attachment-0001.html>
More information about the cfe-commits
mailing list