[analyzer] Emit an error for invalid -analyzer-config inputs

[Szelethus]

Summary

I'm in the process of refactoring AnalyzerOptions. The main motivation behind here is to emit warnings if an invalid -analyzer-config option is given from the command line, and be able to list them all.

In this patch, I'll implement the warning.

[Szelethus]

@whisperity @xazax.hun A worry of mine is shared libraries, for example, we've got an array of Ericsson-specific checkers that we load run-time. Do we (or should we) support acquiring non-checker -analyzer-config options?

[NoQ]

I guess we should consider the idea in http://lists.llvm.org/pipermail/cfe-dev/2018-October/059864.html

lib/Frontend/CompilerInvocation.cpp

353

typo: "config"

[xazax.hun]

I agree with NoQ. Forward and backward compatibility might be important for CodeChecker as well.
But I wonder if it make sense to have analyzer-config compatibility mode on a per config basis?
E.g., if we have two configs:

• One did not exist in earlier clang versions, but a tool (like CodeChecker) would like to pass this to the analyzer without doing a version check first. Passing this in a compatibility mode makes sense. This could be the regural -analyzer-config
• The second option also did not exist in earlier clang versions, but we do not want to support those versions. In the case passing this config in a more strict mode makes sense. This could be something like -analyzer-config-strict.

Or we could choose the deafults the other way around.

What do you think? Does it make sense to have the compatibility mode on a per config bases or too much code for too little gain?

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

322

I wonder if it is worth to store this in a static object, have it sorted and use binary search for the warning. If we do not expect to have a lot more options in the future I am fine with the current solution.

[Szelethus]

Since I've dug myself quite deep into the refactoring effort, I realized that there are more elegant ways of achieving this, should I hammer AnalyzerOptions for long enough. I'll probably change everything but the revision title.

[Szelethus]

• Added the discussed compatibility flag that's off by default, but is enabled by default by the driver.
• Now emitting errors instead of warnings.
• For the first time, introduce errors rather then asserts for invalid input.

[Szelethus]

Polite ping. :) I'd be most comfortable landing D53692 together with this patch.

[xazax.hun]

Overall looks good to me, some minor comments inline.

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

232

I wonder if it is a good idea to put the comment inside the macro.

lib/Driver/ToolChains/Clang.cpp

2372

I wonder if it makes more sense to not add this here but rather make the default option to be true.

lib/Frontend/CompilerInvocation.cpp

367–370

Do we actually need the branching here? It would be perfectly fine to always pass a pointer to Diags but sometimes just ignore it.

[Szelethus]

Thanks! I'll commit around friday (with the requested fixes) to leave enough time for everyone to object.

[Szelethus]

lib/Driver/ToolChains/Clang.cpp

2372

We resolved this IRL, but just for the record, this line will run when the analyzer is invoked with --analyze (essentially, in driver mode), in which case we wouldn't like to receive errors on invalid input, but with -cc1 (frontend mode), it will be disabled by default .

[NoQ]

Thanks, this looks great!

lib/Frontend/CompilerInvocation.cpp

363

Maybe we can eventually turn this into an array and address by index that's auto-generated from the options table.

But that's pretty unimportant; even though we can query options pretty often, i don't think they are a performance bottleneck.

[Szelethus]

Thanks!

lib/Frontend/CompilerInvocation.cpp

363

Actually, ConfigTable will probably end up on the chopping board by the end of the refactoring.

[Szelethus]

lib/Frontend/CompilerInvocation.cpp

367–370

I'm not sure what would be the point of that, if we don't branch here, we'll have to somewhere else, and I'm not sure whether polluting parseAnalyzerConfigs with that would help on readability.

Or, if you mean that I should just check Opts.ShouldEmitErrorsOnInvalidConfigValue inside there, that could work, but I think it makes more sense that if we don't to validate configs, don't even supply the tool for it. Less room for error.

[NoQ]

Ugh. We broke backwards compatibility when an invalid -analyzer-config is specified but -analyze isn't specified. Yes, people are accidentally relying on that :/ :/ :/

In this case the driver doesn't know that it needs to add analyzer-config-compatibility-mode=true (or, even better, outright ignore all analyzer options) but the frontend doesn't know that it doesn't need to parse them.

[Szelethus]

In D53280#1319446, @NoQ wrote:

Ugh. We broke backwards compatibility when an invalid -analyzer-config is specified but -analyze isn't specified. Yes, people are accidentally relying on that :/ :/ :/

In this case the driver doesn't know that it needs to add analyzer-config-compatibility-mode=true (or, even better, outright ignore all analyzer options) but the frontend doesn't know that it doesn't need to parse them.

Oh my god. Can you go a little more in depth? Was the problem something along the lines of treu typo, or a non-existent flag? Is that at all salvageable? I mean, losing the entire patch would be super bad imo.

Also, I can't attend to this issue immediately -- replying on the phone is one thing, but almost any other action is out of my reach for the next couple days/weeks.

[Szelethus]

lib/Driver/ToolChains/Clang.cpp

3307–3309

Hmmm. If we just moved the CmdArgs.push_back("-analyzer-config-compatibility-mode=true"); line to this function (which actually invokes RenderAnalyzerOptions), that would solve the issue, wouldn't it? Or somewhere in an even more general driver function.

[NoQ]

It's a flag that was removed but old soft doesn't know that it was removed, and it won't be fixed because it's old. I guess a typo would have caused the same problem.

Not urgent, no pressure. Yes, i believe this patch is good and should stay there. At the same time, by the looks of it, coming up with a fix for this "problem" may be annoying.

lib/Driver/ToolChains/Clang.cpp

3307–3309

Yeah, but it's not going to be an AnalyzeJobAction in the problematic case. So you'd have to add this flag to every single clang invocation.

[Szelethus]

In D53280#1319503, @NoQ wrote:

It's a flag that was removed but old soft doesn't know that it was removed, and it won't be fixed because it's old.

We could just add the flag back, and do nothing with it. We have already mentioned converting some frontend flags to analyzer configs, and removing some long enabled-by-default options while still leaving them in the code in some form or another for backward compatibility, so some carefully documented litter in AnalyzerOptions here and there wouldn't be the end of the world.

Edit: I just realized that this could easily be an issue for other users of the static analyzer (or clang in general) as well.

lib/Driver/ToolChains/Clang.cpp

3307–3309

Aaaaand I bet that's something we can't do just like that.

[Szelethus]

I guess the solution would be to check whether there are any user supplied flags with "analyze" substring, and add the compatibility flag then. It is possible if not probable that a non-static-analyzer related flag with a name like that will eventually be added, but I guess we can live with it.

The deadline is around the creation of the 8.0.0. release branch, right?

[NoQ]

In D53280#1325375, @Szelethus wrote:

I guess the solution would be to check whether there are any user supplied flags with "analyze" substring, and add the compatibility flag then. It is possible if not probable that a non-static-analyzer related flag with a name like that will eventually be added, but I guess we can live with it.

Yeah, that's pretty much the easiest solution that i see (i guess we need to look for -analyzer-config specifically), but it's still pretty gross.

The deadline is around the creation of the 8.0.0. release branch, right?

I'd feel slightly more comfortable if some sort of fix gets in before the Christmas break.

In D53280#1319524, @Szelethus wrote:

We could just add the flag back, and do nothing with it. (...) Edit: I just realized that this could easily be an issue for other users of the static analyzer (or clang in general) as well.

It doesn't sound appealing to bring back all the flags that we've removed recently, but i guess it's not that hard to track back all the options that we removed since, say, 6.0.0 and add them back, marked as "deprecated" somehow, and remove them later when either we have a solution for the general case or we don't care anymore :)

[Szelethus]

The deadline is around the creation of the 8.0.0. release branch, right?

I'd feel slightly more comfortable if some sort of fix gets in before the Christmas break.

Sure! :)

include/clang/Basic/DiagnosticDriverKinds.td

 def note_drv_use_standard : Note<"use '%0'"
   "%select{| or '%3'|, '%3', or '%4'|, '%3', '%4', or '%5'}2 "
   "for '%1' standard">;
 
 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">;
 def warn_drv_vectorize_needs_hvx : Warning<
   "auto-vectorization requires HVX, use -mhvx to enable it">,
   InGroup<OptionIgnored>;
 
 def err_drv_module_header_wrong_kind : Error<

include/clang/Driver/CC1Options.td

def analyzer_config_help : Flag<["-"], "analyzer-config-help">,
   HelpText<"Display the list of -analyzer-config options">;
 
 def analyzer_list_enabled_checkers : Flag<["-"], "analyzer-list-enabled-checkers">,
   HelpText<"Display the list of enabled analyzer checkers">;
 
 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
 //===----------------------------------------------------------------------===//
 def migrator_no_nsalloc_error : Flag<["-"], "no-ns-alloc-error">,
   HelpText<"Do not error on use of NSAllocateCollectable/NSReallocateCollectable">;
 
 def migrator_no_finalize_removal : Flag<["-"], "no-finalize-removal">,
   HelpText<"Do not remove finalize method in gc mode">;

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

public:
   /// This flag allows one to disable analyzer checks on the code processed by
   /// the given analysis consumer. Note, the code will get parsed and the
   /// command-line options will get checked.
   unsigned DisableAllChecks : 1;
 
   unsigned ShowCheckerHelp : 1;
   unsigned ShowEnabledCheckerList : 1;
   unsigned ShowConfigOptionsList : 1;
+  unsigned ShouldEmitErrorsOnInvalidConfigValue : 1;
   unsigned AnalyzeAll : 1;
   unsigned AnalyzerDisplayProgress : 1;
   unsigned AnalyzeNestedBlocks : 1;
 
   unsigned eagerlyAssumeBinOpBifurcation : 1;
 
   unsigned TrimGraph : 1;
   unsigned visualizeExplodedGraphWithGraphViz : 1;
   unsigned UnoptimizedCFG : 1;
   unsigned PrintStats : 1;
 
   /// Do not re-analyze paths leading to exhausted nodes with a different
   /// strategy. We get better code coverage when retry is enabled.
   unsigned NoRetryExhausted : 1;
 
   /// The inlining stack depth limit.
   // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls).
   unsigned InlineMaxStackDepth = 5;
 
   /// 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)
 
 #define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL)                \
   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<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.
   ///
   /// If an option value is not provided, returns the given \p DefaultVal.
   /// @param [in] Name Name for option to retrieve.
   /// @param [in] DefaultVal Default value returned if no such option was
   /// specified.
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
   /// @param [in] C The checker object the option belongs to. Checker options
   /// are retrieved in the following format:
   /// `-analyzer-config <package and checker name>:OptionName=Value.
   /// @param [in] SearchInParents If set to true and the searched option was not
   /// specified for the given checker the options for the parent packages will
   /// be searched as well. The inner packages take precedence over the outer
   /// ones.
   StringRef getCheckerStringOption(StringRef Name, StringRef DefaultVal,
                               const ento::CheckerBase *C,
                               bool SearchInParents = false) const;
 
   /// Retrieves and sets the UserMode. This is a high-level option,
   /// which is used to set other low-level options. It is not accessible
   /// outside of AnalyzerOptions.
   UserModeKind getUserMode() const;
 
   ExplorationStrategyKind getExplorationStrategy() const;

lib/Driver/ToolChains/Clang.cpp

static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
                                   const llvm::Triple &Triple,
                                   const InputInfo &Input) {
   // Enable region store model by default.
   CmdArgs.push_back("-analyzer-store=region");
 
   // 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");
     CmdArgs.push_back("-analyzer-checker=apiModeling");
 
     if (!Triple.isWindowsMSVCEnvironment()) {
       CmdArgs.push_back("-analyzer-checker=unix");
     } else {
static void RenderDebugOptions(const ToolChain &TC, const Driver &D,
   // Do we need to explicitly import anonymous namespaces into the parent
   // scope?
   if (DebuggerTuning == llvm::DebuggerKind::SCE)
     CmdArgs.push_back("-dwarf-explicit-import");
 
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
 }
 
 void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                          const InputInfo &Output, const InputInfoList &Inputs,
                          const ArgList &Args, const char *LinkingOutput) const {
   const auto &TC = getToolChain();
   const llvm::Triple &RawTriple = TC.getTriple();
   const llvm::Triple &Triple = TC.getEffectiveTriple();
   const std::string &TripleStr = Triple.getTriple();
 
   bool KernelOrKext =
       Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
   const Driver &D = TC.getDriver();

lib/Frontend/CompilerInvocation.cpp

for (auto *A : Args.filtered(Group)) {
     } else {
       // Otherwise, add its value (for OPT_W_Joined and similar).
       for (const auto *Arg : A->getValues())
         Diagnostics.emplace_back(Arg);
     }
   }
 }
 
+// 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) {
   SmallVector<const char *, 8> Values;
   for (const auto &Arg : Args) {
     const Option &O = Arg->getOption();
     if (O.matches(options::OPT_fno_builtin_)) {
       const char *FuncName = Arg->getValue();
#include "clang/StaticAnalyzer/Core/Analyses.def"
     } else {
       Opts.InliningMode = Value;
     }
   }
 
   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 =
     Args.hasArg(OPT_analyzer_viz_egraph_graphviz);
   Opts.DumpExplodedGraphTo = Args.getLastArgValue(OPT_analyzer_dump_egraph);
   Opts.NoRetryExhausted = Args.hasArg(OPT_analyzer_disable_retry_exhausted);
   Opts.AnalyzeAll = Args.hasArg(OPT_analyzer_opt_analyze_headers);
   Opts.AnalyzerDisplayProgress = Args.hasArg(OPT_analyzer_display_progress);
for (const Arg *A :
     SmallVector<StringRef, 4> checkers;
     checkerList.split(checkers, ",");
     for (auto checker : checkers)
       Opts.CheckersControlList.emplace_back(checker, enable);
   }
 
   // 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();
     SmallVector<StringRef, 4> configVals;
     configList.split(configVals, ",");
     for (const auto &configVal : configVals) {
       StringRef key, val;
       std::tie(key, val) = configVal.split("=");
       if (val.empty()) {
         Diags.Report(SourceLocation(),
                      diag::err_analyzer_config_no_value) << configVal;
         Success = false;
         break;
       }
       if (val.find('=') != StringRef::npos) {
         Diags.Report(SourceLocation(),
                      diag::err_analyzer_config_multiple_values)
           << configVal;
         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) {
     if (i != 0)
       os << " ";
     os << Args.getArgString(i);
   }
   os.flush();
 
   return Success;
 }
 
 static StringRef getStringOption(AnalyzerOptions::ConfigTable &Config,
                                  StringRef OptionName, StringRef DefaultVal) {
   return Config.insert({OptionName, DefaultVal}).first->second;
 }
 
 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) {
   Opts.NoNSAllocReallocError = Args.hasArg(OPT_migrator_no_nsalloc_error);
   Opts.NoFinalizeRemoval = Args.hasArg(OPT_migrator_no_finalize_removal);
   return true;
 }
 

test/Analysis/invalid-analyzer-config-value.c

+// 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() {}