[analyzer] Emit an error rather than assert on invalid checker option input

[Szelethus]

Summary

Asserting on invalid input isn't very nice, hence the patch to emit an error instead.

This is the first of many patches to overhaul the way we handle checker options, and I admit that it's a kind of "things that didn't fit anywhere else" patch.

[teemperor]

LGTM for the CloneChecker changes.

[NoQ]

Looks cool, thanks!!

include/clang/Basic/DiagnosticDriverKinds.td

306–307

I suggest hardcoding the word "value" in every message rather than putting it here, because it may be desirable to substitute it with something more specific or more accurate in every checker's specific circumstances.

Eg., "a non-negative integer" would be more precise than "a non-negative value".

lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

352–355

I passively wish for a certain amount of de-duplication that wouldn't require every checker to obtain a diagnostics engine every time it tries to read an option. Eg.,

auto *Checker = Mgr.registerChecker<PaddingChecker>();
Checker->AllowedPad = Mgr.getAnalyzerOptions()
        .getCheckerIntegerOption(Checker, "AllowedPad", 24);
if (Checker->AllowedPad < 0)
  Mgr.reportInvalidOptionValue(Checker, "AllowedPad", "a non-negative");

Or maybe even something like that:

auto *Checker = Mgr.registerChecker<PaddingChecker>();
Checker->AllowedPad = Mgr.getAnalyzerOptions()
        .getCheckerIntegerOption(Checker, "AllowedPad", 24,
                [](int x) -> Option<std::string> {
                    if (x < 0) {
                      // Makes getCheckerIntegerOption() emit a diagnostic
                      // and return the default value.
                      return "a non-negative";
                    }
                    // Makes getCheckerIntegerOption() successfully return
                    // the user-specified value.
                    return None;
                });

I.e., a validator lambda.

[Szelethus]

include/clang/Basic/DiagnosticDriverKinds.td

306–307

Yup, makes total sense!

lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

352–355

First one, sure. I'm a little unsure about the second: No other "options"-like classes have access to a DiagnosticsEngine in clang, as far as I'm aware, and I guess keeping AnalyzerOptions as simple is possible is preferred. Not only that, but a validator lambda seems an to be an overkill (though really-really cool) solution. Your first bit of code is far more readable IMO.

[Szelethus]

• Added a convenience method to CheckerManager for reporting invalid user input
• Edited the error message to be more flexible

[NoQ]

lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

352–355

Hmm, in the first example we'll also have to manually reset the option to the default value if it is invalid, which is also annoying - even if easy to understand, it's also easy to forget.

And with that and a bit of polish, the lambda approach isn't really much more verbose, and definitely involves less duplication:

auto *Checker = Mgr.registerChecker<PaddingChecker>();
Checker->AllowedPad = Mgr.getAnalyzerOptions()
        .getCheckerIntegerOption(Checker, "AllowedPad", 24);
if (Checker->AllowedPad < 0) {
  Mgr.reportInvalidOptionValue(Checker, "AllowedPad", "a non-negative value");
  Checker->AllowedPad = 24;
}

vs.

auto *Checker = Mgr.registerChecker<PaddingChecker>();
Checker->AllowedPad = Mgr.getAnalyzerOptions()
        .getCheckerIntegerOption(Checker, "AllowedPad", /*Default=*/ 24,
                                 /*Validate=*/ [](int x) { return x >= 0; },
                                 /*ValidMsg=*/ "a non-negative value");

[Szelethus]

lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

352–355

Alright, so I've given this a lot of thought, here's where I'm standing on the issue:

• I would prefer not to add DiagnosticsEngine to AnalyzerOptions. In fact, I'd prefer not to add it even as a parameter to one of it's methods -- designwise, it should be a simple mapping of the command line parameters, not doing any complicated hackery.
• You got me convinced the validator lambda thing ;). However, a nice implementation of this (emphasis on nice) is most definitely a bigger undertaking.
• Once we're at the topic of "easy to forget", we could also verify compile-time whether checker options are actually used -- what I'm thinking here, is something like this:

auto *Checker = Mgr.registerChecker<PaddingChecker>();
Mgr.initFieldWithOption(Checker, "AllowedPad",
                        // Note that we should be able
                        // to know the default value.
                        Checker->AllowedPad,
                        // We could make this optional by defining a
                        // default validator...
                        /*Validate=*/ [](int x) { return x >= 0; },
                        // ...aaaand a default error message.
                        /*ValidMsg=*/ "a non-negative value");

CheckerManager later (once all checker registry functions finished) could validate, with the help of CheckerRegistry, whether

• All options for a given checker were queried for,
• The supplied checker options is valid, if not, restore them in compatibility mode, emit an error otherwise,
• No list is complete without a third item.

For now, I admit, I have little interest in this. Would you mind me committing as is?

[NoQ]

Would you mind me committing as is?

Np, please do, the patch is great regardless!

lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

352–355

Once we're at the topic of "easy to forget", we could also verify compile-time whether checker options are actually used -- what I'm thinking here, is something like this:

auto *Checker = Mgr.registerChecker<PaddingChecker>();
Mgr.initFieldWithOption(Checker, "AllowedPad",
                        // Note that we should be able
                        // to know the default value.
                        Checker->AllowedPad,
                        // We could make this optional by defining a
                        // default validator...
                        /*Validate=*/ [](int x) { return x >= 0; },
                        // ...aaaand a default error message.
                        /*ValidMsg=*/ "a non-negative value");

For ULTIMATE reliability and reusability, i'd also use a pointer-to-member argument instead of a pass-by-reference out-parameter, so that make sure that the value is indeed stored in the checker:

Mgr.initFieldWithOption(Checker, "AllowedPad",
                        &PaddingChecker::AllowedPad,
                        ...)

(sorry, couldn't help myself)

include/clang/Basic/DiagnosticDriverKinds.td

 
 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_analyzer_checker_option_invalid_input : Error<
+  "invalid input for checker option '%0', that expects %1">;
 
 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/StaticAnalyzer/Core/CheckerManager.h

public:
   bool hasPathSensitiveCheckers() const;
 
   void finishedCheckerRegistration();
 
   const LangOptions &getLangOpts() const { return LangOpts; }
   AnalyzerOptions &getAnalyzerOptions() { return AOptions; }
   ASTContext &getASTContext() { return Context; }
 
+  /// Emits an error through a DiagnosticsEngine about an invalid user supplied
+  /// checker option value.
+  void reportInvalidCheckerOptionValue(const CheckerBase *C,
+                                       StringRef OptionName,
+                                       StringRef ExpectedValueDesc);
+
   using CheckerRef = CheckerBase *;
   using CheckerTag = const void *;
   using CheckerDtor = CheckerFn<void ()>;
 
 //===----------------------------------------------------------------------===//
 // Checker registration.
 //===----------------------------------------------------------------------===//
 

lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp

bool ObjCDeallocChecker::isNibLoadedIvarWithoutRetain(
 
   if (PropImpl->getPropertyDecl()->getSetterMethodDecl())
     return false;
 
   return true;
 }
 
 void ento::registerObjCDeallocChecker(CheckerManager &Mgr) {
-  const LangOptions &LangOpts = Mgr.getLangOpts();
-  // These checker only makes sense under MRR.
-  if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount)
-    return;
-
   Mgr.registerChecker<ObjCDeallocChecker>();
 }
 
 bool ento::shouldRegisterObjCDeallocChecker(const LangOptions &LO) {
-  return true;
+  // These checker only makes sense under MRR.
+  return LO.getGC() != LangOptions::GCOnly && !LO.ObjCAutoRefCount;
 }

lib/StaticAnalyzer/Checkers/CloneChecker.cpp

 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class CloneChecker
     : public Checker<check::ASTCodeBody, check::EndOfTranslationUnit> {
+public:
+  // Checker options.
+  int MinComplexity;
+  bool ReportNormalClones;
+  StringRef IgnoredFilesPattern;
+
+private:
   mutable CloneDetector Detector;
   mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious;
 
 public:
   void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
                         BugReporter &BR) const;
 
   void checkEndOfTranslationUnit(const TranslationUnitDecl *TU,

 }
 
 void CloneChecker::checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
                                              AnalysisManager &Mgr,
                                              BugReporter &BR) const {
   // At this point, every statement in the translation unit has been analyzed by
   // the CloneDetector. The only thing left to do is to report the found clones.
 
-  int MinComplexity = Mgr.getAnalyzerOptions().getCheckerIntegerOption(
-      this, "MinimumCloneComplexity", 50);
-  assert(MinComplexity >= 0);
-
-  bool ReportSuspiciousClones = Mgr.getAnalyzerOptions()
-    .getCheckerBooleanOption(this, "ReportSuspiciousClones", true);
-
-  bool ReportNormalClones = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
-      this, "ReportNormalClones", true);
-
-  StringRef IgnoredFilesPattern = Mgr.getAnalyzerOptions()
-    .getCheckerStringOption(this, "IgnoredFilesPattern", "");
-
   // Let the CloneDetector create a list of clones from all the analyzed
   // statements. We don't filter for matching variable patterns at this point
   // because reportSuspiciousClones() wants to search them for errors.
   std::vector<CloneDetector::CloneGroup> AllCloneGroups;
 
   Detector.findClones(
       AllCloneGroups, FilenamePatternConstraint(IgnoredFilesPattern),
       RecursiveCloneTypeIIHashConstraint(), MinGroupSizeConstraint(2),
       MinComplexityConstraint(MinComplexity),
       RecursiveCloneTypeIIVerifyConstraint(), OnlyLargestCloneConstraint());
 
-  if (ReportSuspiciousClones)
-    reportSuspiciousClones(BR, Mgr, AllCloneGroups);
 
   // We are done for this translation unit unless we also need to report normal
   // clones.
   if (!ReportNormalClones)
     return;
 
   // Now that the suspicious clone detector has checked for pattern errors,
   // we also filter all clones who don't have matching patterns
void CloneChecker::reportSuspiciousClones(
   }
 }
 
 //===----------------------------------------------------------------------===//
 // Register CloneChecker
 //===----------------------------------------------------------------------===//
 
 void ento::registerCloneChecker(CheckerManager &Mgr) {
-  Mgr.registerChecker<CloneChecker>();
+  auto *Checker = Mgr.registerChecker<CloneChecker>();
+
+  Checker->MinComplexity = Mgr.getAnalyzerOptions().getCheckerIntegerOption(
+      Checker, "MinimumCloneComplexity", 50);
+
+  if (Checker->MinComplexity < 0)
+    Mgr.reportInvalidCheckerOptionValue(
+        Checker, "MinimumCloneComplexity", "a non-negative value");
+
+  Checker->ReportNormalClones = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+      Checker, "ReportNormalClones", true);
+
+  Checker->IgnoredFilesPattern = Mgr.getAnalyzerOptions()
+    .getCheckerStringOption(Checker, "IgnoredFilesPattern", "");
 }
 
 bool ento::shouldRegisterCloneChecker(const LangOptions &LO) {
   return true;
 }

lib/StaticAnalyzer/Checkers/MoveChecker.cpp

 // MoveChecker.cpp - Check use of moved-from objects. - C++ ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This defines checker which checks for potential misuses of a moved-from
 // object. That means method calls on the object or copying it in moved-from
 // state.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ExprCXX.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/StringSet.h"
 
private:
     // How exactly the object was misused.
     const MisuseKind MK;
     bool Found;
   };
 
   AggressivenessKind Aggressiveness;
 
 public:
-  void setAggressiveness(StringRef Str) {
+  void setAggressiveness(StringRef Str, CheckerManager &Mgr) {
     Aggressiveness =
         llvm::StringSwitch<AggressivenessKind>(Str)
             .Case("KnownsOnly", AK_KnownsOnly)
             .Case("KnownsAndLocals", AK_KnownsAndLocals)
             .Case("All", AK_All)
-            .Default(AK_KnownsAndLocals); // A sane default.
+            .Default(AK_Invalid);
+
+    if (Aggressiveness == AK_Invalid)
+      Mgr.reportInvalidCheckerOptionValue(this, "WarnOn",
+          "either \"KnownsOnly\", \"KnownsAndLocals\" or \"All\" string value");
   };
 
 private:
   mutable std::unique_ptr<BugType> BT;
 
   // Check if the given form of potential misuse of a given object
   // should be reported. If so, get it reported. The callback from which
   // this function was called should immediately return after the call
for (auto I: RS) {
         Out << ": moved and reported";
       Out << NL;
     }
   }
 }
 void ento::registerMoveChecker(CheckerManager &mgr) {
   MoveChecker *chk = mgr.registerChecker<MoveChecker>();
   chk->setAggressiveness(
-      mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn", ""));
+      mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn",
+                                                      "KnownsAndLocals"), mgr);
 }
 
 bool ento::shouldRegisterMoveChecker(const LangOptions &LO) {
   return true;
 }

lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include <numeric>
public:
   }
 };
 } // namespace
 
 void ento::registerPaddingChecker(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker<PaddingChecker>();
   Checker->AllowedPad = Mgr.getAnalyzerOptions()
           .getCheckerIntegerOption(Checker, "AllowedPad", 24);
-  assert(Checker->AllowedPad >= 0 &&
-         "AllowedPad option should be non-negative");
+  if (Checker->AllowedPad < 0)
+    Mgr.reportInvalidCheckerOptionValue(
+        Checker, "AllowedPad", "a non-negative value");
 }
 
 bool ento::shouldRegisterPaddingChecker(const LangOptions &LO) {
   return true;
 }

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

 // Some of the logic is implemented in UninitializedPointee.cpp, to reduce the
 // complexity of this file.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "UninitializedObject.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
 
 using namespace clang;
 using namespace clang::ento;
 using namespace clang::ast_matchers;
void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) {
   ChOpts.IsPedantic =
       AnOpts.getCheckerBooleanOption(Chk, "Pedantic", /*DefaultVal*/ false);
   ChOpts.ShouldConvertNotesToWarnings = AnOpts.getCheckerBooleanOption(
       Chk, "NotesAsWarnings", /*DefaultVal*/ false);
   ChOpts.CheckPointeeInitialization = AnOpts.getCheckerBooleanOption(
       Chk, "CheckPointeeInitialization", /*DefaultVal*/ false);
   ChOpts.IgnoredRecordsWithFieldPattern =
       AnOpts.getCheckerStringOption(Chk, "IgnoreRecordsWithField",
-                                    /*DefaultVal*/ "");
+                                    /*DefaultVal*/ "\"\"");
   ChOpts.IgnoreGuardedFields =
       AnOpts.getCheckerBooleanOption(Chk, "IgnoreGuardedFields",
                                      /*DefaultVal*/ false);
+
+  std::string ErrorMsg;
+  if (!llvm::Regex(ChOpts.IgnoredRecordsWithFieldPattern).isValid(ErrorMsg))
+    Mgr.reportInvalidCheckerOptionValue(Chk, "IgnoreRecordsWithField",
+        "a valid regex, building failed with error message "
+        "\"" + ErrorMsg + "\"");
 }
 
 bool ento::shouldRegisterUninitializedObjectChecker(const LangOptions &LO) {
   return true;
 }

lib/StaticAnalyzer/Core/CheckerManager.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Stmt.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
#ifndef NDEBUG
   // Make sure that for every event that has listeners, there is at least
   // one dispatcher registered for it.
   for (const auto &Event : Events)
     assert(Event.second.HasDispatcher &&
            "No dispatcher registered for an event");
 #endif
 }
 
+void CheckerManager::reportInvalidCheckerOptionValue(
+    const CheckerBase *C, StringRef OptionName, StringRef ExpectedValueDesc) {
+
+  Context.getDiagnostics()
+      .Report(diag::err_analyzer_checker_option_invalid_input)
+          << (llvm::Twine() + C->getTagDescription() + ":" + OptionName).str()
+          << ExpectedValueDesc;
+}
+
 //===----------------------------------------------------------------------===//
 // Functions for running checkers for AST traversing..
 //===----------------------------------------------------------------------===//
 
 void CheckerManager::runCheckersOnASTDecl(const Decl *D, AnalysisManager& mgr,
                                           BugReporter &BR) {
   assert(D);
 

test/Analysis/copypaste/suspicious-clones.cpp

-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.clone.CloneChecker -analyzer-config alpha.clone.CloneChecker:ReportSuspiciousClones=true  -analyzer-config alpha.clone.CloneChecker:ReportNormalClones=false -analyzer-config alpha.clone.CloneChecker:MinimumCloneComplexity=10 -verify %s
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.clone.CloneChecker \
+// RUN:   -analyzer-config alpha.clone.CloneChecker:ReportNormalClones=false \
+// RUN:   -analyzer-config alpha.clone.CloneChecker:MinimumCloneComplexity=10
 
 // Tests finding a suspicious clone that references local variables.
 
 void log();
 
 int max(int a, int b) {
   log();
   if (a > b)

test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \
 // RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
 // RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind" \
 // RUN:   -std=c++11 -verify  %s
 
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.cplusplus.UninitializedObject \
+// RUN:   -analyzer-config \
+// RUN:     alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="([)]" \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-UNINIT-INVALID-REGEX
+
+// CHECK-UNINIT-INVALID-REGEX: (frontend): invalid input for checker option
+// CHECK-UNINIT-INVALID-REGEX-SAME: 'alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField',
+// CHECK-UNINIT-INVALID-REGEX-SAME: that expects a valid regex, building failed
+// CHECK-UNINIT-INVALID-REGEX-SAME: with error message "parentheses not
+// CHECK-UNINIT-INVALID-REGEX-SAME: balanced"
+
+
 // expected-no-diagnostics
 
 // Both type and name contains "kind".
 struct UnionLikeStruct1 {
   enum Kind {
     volume,
     area
   } kind;

test/Analysis/outofbound.c

 // RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-store=region -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix \
 // RUN:   -analyzer-checker=alpha.security.ArrayBound \
-// RUN:   -analyzer-config unix:Optimistic=true
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void *calloc(size_t, size_t);
 
 char f1() {
   char* s = "abcd";
   char c = s[4]; // no-warning

test/Analysis/padding_c.c

-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=optin.performance \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=2
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=optin.performance.Padding \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=-10 \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PAD-NEGATIVE-VALUE
+
+// CHECK-PAD-NEGATIVE-VALUE: (frontend): invalid input for checker option
+// CHECK-PAD-NEGATIVE-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-PAD-NEGATIVE-VALUE-SAME: expects a non-negative value
 
 #if __has_include(<stdalign.h>)
 #include <stdalign.h>
 #endif
 
 #if __has_include(<stdalign.h>) || defined(__cplusplus)
 // expected-warning@+1{{Excessive padding in 'struct FieldAttrAlign' (6 padding}}
 struct FieldAttrAlign {

test/Analysis/undef-buffers.c

 // RUN: %clang_analyze_cc1 -analyzer-store=region -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix \
 // RUN:   -analyzer-checker=core.uninitialized \
-// RUN:   -analyzer-config unix:Optimistic=true
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void free(void *);
 
 char stackBased1 () {
   char buf[2];
   buf[0] = 'a';

test/Analysis/use-after-move.cpp

 // RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
 // RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
 // RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
 // RUN:  -analyzer-checker debug.ExprInspection
 
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.Move \
+// RUN:   -analyzer-config cplusplus.Move:WarnOn="a bunch of things" \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-MOVE-INVALID-VALUE
+
+// CHECK-MOVE-INVALID-VALUE: (frontend): invalid input for checker option
+// CHECK-MOVE-INVALID-VALUE-SAME: 'cplusplus.Move:WarnOn', that expects either
+// CHECK-MOVE-INVALID-VALUE-SAME: "KnownsOnly", "KnownsAndLocals" or "All"
+// CHECK-MOVE-INVALID-VALUE-SAME: string value
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 void clang_analyzer_warnIfReached();
 
 class B {
 public:
   B() = default;
   B(const B &) = default;