<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi Kristóf,<div><br></div><div>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.</div><div>Landed a fix in r348125.</div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Nov 30, 2018 at 10:27 PM Kristof Umann via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: szelethus<br>
Date: Fri Nov 30 13:24:31 2018<br>
New Revision: 348038<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=348038&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=348038&view=rev</a><br>
Log:<br>
[analyzer] Emit an error for invalid -analyzer-config inputs<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D53280" rel="noreferrer" target="_blank">https://reviews.llvm.org/D53280</a><br>
<br>
Added:<br>
cfe/trunk/test/Analysis/invalid-analyzer-config-value.c<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td<br>
cfe/trunk/include/clang/Driver/CC1Options.td<br>
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h<br>
cfe/trunk/lib/Driver/ToolChains/Clang.cpp<br>
cfe/trunk/lib/Frontend/CompilerInvocation.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=348038&r1=348037&r2=348038&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=348038&r1=348037&r2=348038&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Nov 30 13:24:31 2018<br>
@@ -299,6 +299,9 @@ def err_analyzer_config_no_value : Error<br>
"analyzer-config option '%0' has a key but no value">;<br>
def err_analyzer_config_multiple_values : Error<<br>
"analyzer-config option '%0' should contain only one '='">;<br>
+def err_analyzer_config_invalid_input : Error<<br>
+ "invalid input for analyzer-config option '%0', that expects %1 value">;<br>
+def err_analyzer_config_unknown : Error<"unknown analyzer-config '%0'">;<br>
<br>
def err_drv_invalid_hvx_length : Error<<br>
"-mhvx-length is not supported without a -mhvx/-mhvx= flag">;<br>
<br>
Modified: cfe/trunk/include/clang/Driver/CC1Options.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=348038&r1=348037&r2=348038&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=348038&r1=348037&r2=348038&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)<br>
+++ cfe/trunk/include/clang/Driver/CC1Options.td Fri Nov 30 13:24:31 2018<br>
@@ -138,6 +138,12 @@ def analyzer_list_enabled_checkers : Fla<br>
def analyzer_config : Separate<["-"], "analyzer-config">,<br>
HelpText<"Choose analyzer options to enable">;<br>
<br>
+def analyzer_config_compatibility_mode : Separate<["-"], "analyzer-config-compatibility-mode">,<br>
+ HelpText<"Don't emit errors on invalid analyzer-config inputs">;<br>
+<br>
+def analyzer_config_compatibility_mode_EQ : Joined<["-"], "analyzer-config-compatibility-mode=">,<br>
+ Alias<analyzer_config_compatibility_mode>;<br>
+<br>
//===----------------------------------------------------------------------===//<br>
// Migrator Options<br>
//===----------------------------------------------------------------------===//<br>
<br>
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=348038&r1=348037&r2=348038&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=348038&r1=348037&r2=348038&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)<br>
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Fri Nov 30 13:24:31 2018<br>
@@ -200,6 +200,7 @@ public:<br>
unsigned ShowCheckerHelp : 1;<br>
unsigned ShowEnabledCheckerList : 1;<br>
unsigned ShowConfigOptionsList : 1;<br>
+ unsigned ShouldEmitErrorsOnInvalidConfigValue : 1;<br>
unsigned AnalyzeAll : 1;<br>
unsigned AnalyzerDisplayProgress : 1;<br>
unsigned AnalyzeNestedBlocks : 1;<br>
@@ -222,6 +223,7 @@ public:<br>
/// The mode of function selection used during inlining.<br>
AnalysisInliningMode InliningMode = NoRedundancy;<br>
<br>
+ // Create a field for each -analyzer-config option.<br>
#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \<br>
SHALLOW_VAL, DEEP_VAL) \<br>
ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)<br>
@@ -233,13 +235,39 @@ public:<br>
#undef ANALYZER_OPTION<br>
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE<br>
<br>
+ // Create an array of all -analyzer-config command line options. Sort it in<br>
+ // the constructor.<br>
+ std::vector<StringRef> AnalyzerConfigCmdFlags = {<br>
+#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \<br>
+ SHALLOW_VAL, DEEP_VAL) \<br>
+ ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)<br>
+<br>
+#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \<br>
+ CMDFLAG,<br>
+<br>
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"<br>
+#undef ANALYZER_OPTION<br>
+#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE<br>
+ };<br>
+<br>
+ bool isUnknownAnalyzerConfig(StringRef Name) const {<br>
+<br>
+ assert(std::is_sorted(AnalyzerConfigCmdFlags.begin(),<br>
+ AnalyzerConfigCmdFlags.end()));<br>
+<br>
+ return !std::binary_search(AnalyzerConfigCmdFlags.begin(),<br>
+ AnalyzerConfigCmdFlags.end(), Name);<br>
+ }<br>
+<br>
AnalyzerOptions()<br>
: DisableAllChecks(false), ShowCheckerHelp(false),<br>
ShowEnabledCheckerList(false), ShowConfigOptionsList(false),<br>
AnalyzeAll(false), AnalyzerDisplayProgress(false),<br>
AnalyzeNestedBlocks(false), eagerlyAssumeBinOpBifurcation(false),<br>
TrimGraph(false), visualizeExplodedGraphWithGraphViz(false),<br>
- UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false) {}<br>
+ UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false) {<br>
+ llvm::sort(AnalyzerConfigCmdFlags);<br>
+ }<br>
<br>
/// Interprets an option's string value as a boolean. The "true" string is<br>
/// interpreted as true and the "false" string is interpreted as false.<br>
<br>
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=348038&r1=348037&r2=348038&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=348038&r1=348037&r2=348038&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)<br>
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Nov 30 13:24:31 2018<br>
@@ -2368,6 +2368,9 @@ static void RenderAnalyzerOptions(const<br>
// Treat blocks as analysis entry points.<br>
CmdArgs.push_back("-analyzer-opt-analyze-nested-blocks");<br>
<br>
+ // Enable compatilibily mode to avoid analyzer-config related errors.<br>
+ CmdArgs.push_back("-analyzer-config-compatibility-mode=true");<br>
+<br>
// Add default argument set.<br>
if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) {<br>
CmdArgs.push_back("-analyzer-checker=core");<br>
<br>
Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=348038&r1=348037&r2=348038&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=348038&r1=348037&r2=348038&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Nov 30 13:24:31 2018<br>
@@ -181,8 +181,10 @@ static void addDiagnosticArgs(ArgList &A<br>
}<br>
}<br>
<br>
+// Parse the Static Analyzer configuration. If \p Diags is set to nullptr,<br>
+// it won't verify the input.<br>
static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,<br>
- DiagnosticsEngine &Diags);<br>
+ DiagnosticsEngine *Diags);<br>
<br>
static void getAllNoBuiltinFuncValues(ArgList &Args,<br>
std::vector<std::string> &Funcs) {<br>
@@ -284,6 +286,12 @@ static bool ParseAnalyzerArgs(AnalyzerOp<br>
Opts.ShowCheckerHelp = Args.hasArg(OPT_analyzer_checker_help);<br>
Opts.ShowConfigOptionsList = Args.hasArg(OPT_analyzer_config_help);<br>
Opts.ShowEnabledCheckerList = Args.hasArg(OPT_analyzer_list_enabled_checkers);<br>
+ Opts.ShouldEmitErrorsOnInvalidConfigValue =<br>
+ /* negated */!llvm::StringSwitch<bool>(<br>
+ Args.getLastArgValue(OPT_analyzer_config_compatibility_mode))<br>
+ .Case("true", true)<br>
+ .Case("false", false)<br>
+ .Default(false);<br>
Opts.DisableAllChecks = Args.hasArg(OPT_analyzer_disable_all_checks);<br>
<br>
Opts.visualizeExplodedGraphWithGraphViz =<br>
@@ -320,7 +328,7 @@ static bool ParseAnalyzerArgs(AnalyzerOp<br>
<br>
// Go through the analyzer configuration options.<br>
for (const auto *A : Args.filtered(OPT_analyzer_config)) {<br>
- A->claim();<br>
+<br>
// We can have a list of comma separated config names, e.g:<br>
// '-analyzer-config key1=val1,key2=val2'<br>
StringRef configList = A->getValue();<br>
@@ -342,11 +350,24 @@ static bool ParseAnalyzerArgs(AnalyzerOp<br>
Success = false;<br>
break;<br>
}<br>
+<br>
+ // TODO: Check checker options too, possibly in CheckerRegistry.<br>
+ // Leave unknown non-checker configs unclaimed.<br>
+ if (!key.contains(":") && Opts.isUnknownAnalyzerConfig(key)) {<br>
+ if (Opts.ShouldEmitErrorsOnInvalidConfigValue)<br>
+ Diags.Report(diag::err_analyzer_config_unknown) << key;<br>
+ continue;<br>
+ }<br>
+<br>
+ A->claim();<br>
Opts.Config[key] = val;<br>
}<br>
}<br>
<br>
- parseAnalyzerConfigs(Opts, Diags);<br>
+ if (Opts.ShouldEmitErrorsOnInvalidConfigValue)<br>
+ parseAnalyzerConfigs(Opts, &Diags);<br>
+ else<br>
+ parseAnalyzerConfigs(Opts, nullptr);<br>
<br>
llvm::raw_string_ostream os(Opts.FullCompilerInvocation);<br>
for (unsigned i = 0; i < Args.getNumInputArgStrings(); ++i) {<br>
@@ -365,56 +386,82 @@ static StringRef getStringOption(Analyze<br>
}<br>
<br>
static void initOption(AnalyzerOptions::ConfigTable &Config,<br>
+ DiagnosticsEngine *Diags,<br>
StringRef &OptionField, StringRef Name,<br>
StringRef DefaultVal) {<br>
+ // String options may be known to invalid (e.g. if the expected string is a<br>
+ // file name, but the file does not exist), those will have to be checked in<br>
+ // parseConfigs.<br>
OptionField = getStringOption(Config, Name, DefaultVal);<br>
}<br>
<br>
static void initOption(AnalyzerOptions::ConfigTable &Config,<br>
+ DiagnosticsEngine *Diags,<br>
bool &OptionField, StringRef Name, bool DefaultVal) {<br>
- // FIXME: We should emit a warning here if the value is something other than<br>
- // "true", "false", or the empty string (meaning the default value),<br>
- // but the AnalyzerOptions doesn't have access to a diagnostic engine.<br>
- OptionField = llvm::StringSwitch<bool>(getStringOption(Config, Name,<br>
- (DefaultVal ? "true" : "false")))<br>
+ auto PossiblyInvalidVal = llvm::StringSwitch<Optional<bool>>(<br>
+ getStringOption(Config, Name, (DefaultVal ? "true" : "false")))<br>
.Case("true", true)<br>
.Case("false", false)<br>
- .Default(DefaultVal);<br>
+ .Default(None);<br>
+<br>
+ if (!PossiblyInvalidVal) {<br>
+ if (Diags)<br>
+ Diags->Report(diag::err_analyzer_config_invalid_input)<br>
+ << Name << "a boolean";<br>
+ else<br>
+ OptionField = DefaultVal;<br>
+ } else<br>
+ OptionField = PossiblyInvalidVal.getValue();<br>
}<br>
<br>
static void initOption(AnalyzerOptions::ConfigTable &Config,<br>
+ DiagnosticsEngine *Diags,<br>
unsigned &OptionField, StringRef Name,<br>
unsigned DefaultVal) {<br>
+<br>
OptionField = DefaultVal;<br>
bool HasFailed = getStringOption(Config, Name, std::to_string(DefaultVal))<br>
.getAsInteger(10, OptionField);<br>
- assert(!HasFailed && "analyzer-config option should be numeric");<br>
- (void)HasFailed;<br>
+ if (Diags && HasFailed)<br>
+ Diags->Report(diag::err_analyzer_config_invalid_input)<br>
+ << Name << "an unsigned";<br>
}<br>
<br>
static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,<br>
- DiagnosticsEngine &Diags) {<br>
- // TODO: Emit warnings for incorrect options.<br>
+ DiagnosticsEngine *Diags) {<br>
// TODO: There's no need to store the entire configtable, it'd be plenty<br>
// enough tostore checker options.<br>
<br>
#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \<br>
- initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, DEFAULT_VAL); \<br>
+ initOption(AnOpts.Config, Diags, AnOpts.NAME, CMDFLAG, DEFAULT_VAL);<br>
<br>
#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \<br>
SHALLOW_VAL, DEEP_VAL) \<br>
switch (AnOpts.getUserMode()) { \<br>
case UMK_Shallow: \<br>
- initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, SHALLOW_VAL); \<br>
+ initOption(AnOpts.Config, Diags, AnOpts.NAME, CMDFLAG, SHALLOW_VAL); \<br>
break; \<br>
case UMK_Deep: \<br>
- initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, DEEP_VAL); \<br>
+ initOption(AnOpts.Config, Diags, AnOpts.NAME, CMDFLAG, DEEP_VAL); \<br>
break; \<br>
} \<br>
<br>
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"<br>
#undef ANALYZER_OPTION<br>
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE<br>
+<br>
+ // At this point, AnalyzerOptions is configured. Let's validate some options.<br>
+<br>
+ if (!Diags)<br>
+ return;<br>
+<br>
+ if (!AnOpts.CTUDir.empty() && !llvm::sys::fs::is_directory(AnOpts.CTUDir))<br>
+ Diags->Report(diag::err_analyzer_config_invalid_input)<br>
+ << "ctu-dir" << "a filename";<br>
+<br>
+ if (!AnOpts.CTUDir.empty() && !llvm::sys::fs::is_directory(AnOpts.CTUDir))<br>
+ Diags->Report(diag::err_analyzer_config_invalid_input)<br>
+ << "model-path" << "a filename";<br>
}<br>
<br>
static bool ParseMigratorArgs(MigratorOptions &Opts, ArgList &Args) {<br>
<br>
Added: cfe/trunk/test/Analysis/invalid-analyzer-config-value.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/invalid-analyzer-config-value.c?rev=348038&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/invalid-analyzer-config-value.c?rev=348038&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Analysis/invalid-analyzer-config-value.c (added)<br>
+++ cfe/trunk/test/Analysis/invalid-analyzer-config-value.c Fri Nov 30 13:24:31 2018<br>
@@ -0,0 +1,71 @@<br>
+// RUN: not %clang_analyze_cc1 -verify %s \<br>
+// RUN: -analyzer-checker=core \<br>
+// RUN: -analyzer-config notes-as-events=yesplease \<br>
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-BOOL-INPUT<br>
+<br>
+// CHECK-BOOL-INPUT: (frontend): invalid input for analyzer-config option<br>
+// CHECK-BOOL-INPUT-SAME: 'notes-as-events', that expects a boolean value<br>
+<br>
+// RUN: %clang_analyze_cc1 -verify %s \<br>
+// RUN: -analyzer-checker=core \<br>
+// RUN: -analyzer-config-compatibility-mode=true \<br>
+// RUN: -analyzer-config notes-as-events=yesplease<br>
+<br>
+<br>
+// RUN: not %clang_analyze_cc1 -verify %s \<br>
+// RUN: -analyzer-checker=core \<br>
+// RUN: -analyzer-config max-inlinable-size=400km/h \<br>
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-UINT-INPUT<br>
+<br>
+// CHECK-UINT-INPUT: (frontend): invalid input for analyzer-config option<br>
+// CHECK-UINT-INPUT-SAME: 'max-inlinable-size', that expects an unsigned<br>
+// CHECK-UINT-INPUT-SAME: value<br>
+<br>
+// RUN: %clang_analyze_cc1 -verify %s \<br>
+// RUN: -analyzer-checker=core \<br>
+// RUN: -analyzer-config-compatibility-mode=true \<br>
+// RUN: -analyzer-config max-inlinable-size=400km/h<br>
+<br>
+<br>
+// RUN: not %clang_analyze_cc1 -verify %s \<br>
+// RUN: -analyzer-checker=core \<br>
+// RUN: -analyzer-config ctu-dir=0123012301230123 \<br>
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-FILENAME-INPUT<br>
+<br>
+// CHECK-FILENAME-INPUT: (frontend): invalid input for analyzer-config option<br>
+// CHECK-FILENAME-INPUT-SAME: 'ctu-dir', that expects a filename<br>
+// CHECK-FILENAME-INPUT-SAME: value<br>
+<br>
+// RUN: %clang_analyze_cc1 -verify %s \<br>
+// RUN: -analyzer-checker=core \<br>
+// RUN: -analyzer-config-compatibility-mode=true \<br>
+// RUN: -analyzer-config ctu-dir=0123012301230123<br>
+<br>
+<br>
+// RUN: not %clang_analyze_cc1 -verify %s \<br>
+// RUN: -analyzer-checker=core \<br>
+// RUN: -analyzer-config no-false-positives=true \<br>
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-CFG<br>
+<br>
+// CHECK-UNKNOWN-CFG: (frontend): unknown analyzer-config 'no-false-positives'<br>
+<br>
+// RUN: %clang_analyze_cc1 -verify %s \<br>
+// RUN: -analyzer-checker=core \<br>
+// RUN: -analyzer-config-compatibility-mode=true \<br>
+// RUN: -analyzer-config no-false-positives=true<br>
+<br>
+<br>
+// Test the driver properly using "analyzer-config-compatibility-mode=true",<br>
+// no longer causing an error on input error.<br>
+// RUN: %clang --analyze %s<br>
+<br>
+// RUN: not %clang --analyze %s \<br>
+// RUN: -Xclang -analyzer-config -Xclang no-false-positives=true \<br>
+// RUN: -Xclang -analyzer-config-compatibility-mode=false \<br>
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-NO-COMPAT<br>
+<br>
+// CHECK-NO-COMPAT: error: unknown analyzer-config 'no-false-positives'<br>
+<br>
+// expected-no-diagnostics<br>
+<br>
+int main() {}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>