[analyzer] Validate checker option names and values

Validate whether the option exists, and also whether the supplied value is of
the correct type. With this patch, invoking the analyzer should be, at least
in the frontend mode, a lot safer.

Differential Revision: https://reviews.llvm.org/D57860

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@361011 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticDriverKinds.td b/include/clang/Basic/DiagnosticDriverKinds.td
index 988580200274883af33a1fe23db301711f0fd819..3c2cedbaf95ddd42fa35ce8cbdedc925657e5e37 100644 (file)
--- a/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/include/clang/Basic/DiagnosticDriverKinds.td
@@ -307,6 +307,8 @@ def err_analyzer_config_multiple_values : Error<
 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_unknown : Error<
+  "checker '%0' has no option called '%1'">;
 def err_analyzer_checker_option_invalid_input : Error<
   "invalid input for checker option '%0', that expects %1">;
 

diff --git a/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h b/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index c6cb8ac6319526159598fbc8df5d871d2497a231..875cb8edb8ed0dd5f5d679f70e767149567d3940 100644 (file)
--- a/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -107,6 +107,17 @@ public:
       assert((OptionType == "bool" || OptionType == "string" ||
               OptionType == "int") &&
              "Unknown command line option type!");
+
+      assert((OptionType != "bool" ||
+              (DefaultValStr == "true" || DefaultValStr == "false")) &&
+             "Invalid value for boolean command line option! Maybe incorrect "
+             "parameters to the addCheckerOption or addPackageOption method?");
+
+      int Tmp;
+      assert((OptionType != "int" || !DefaultValStr.getAsInteger(0, Tmp)) &&
+             "Invalid value for integer command line option! Maybe incorrect "
+             "parameters to the addCheckerOption or addPackageOption method?");
+      (void)Tmp;
     }
   };
 
@@ -150,6 +161,12 @@ public:
       return State == StateFromCmdLine::State_Disabled && ShouldRegister(LO);
     }
 
+    // Since each checker must have a different full name, we can identify
+    // CheckerInfo objects by them.
+    bool operator==(const CheckerInfo &Rhs) const {
+      return FullName == Rhs.FullName;
+    }
+
     CheckerInfo(InitializationFunction Fn, ShouldRegisterFunction sfn,
                 StringRef Name, StringRef Desc, StringRef DocsUri,
                 bool IsHidden)

diff --git a/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
index 7eef09b883357ce7fba136521a5c5c14545ada98..437e4aaf1a2ed29387b84c42b677cbd8001a83b4 100644 (file)
--- a/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ b/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -9,6 +9,7 @@
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
@@ -306,18 +307,61 @@ void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
   Dependencies.emplace_back(FullName, Dependency);
 }
 
+/// Insert the checker/package option to AnalyzerOptions' config table, and
+/// validate it, if the user supplied it on the command line.
+static void insertAndValidate(StringRef FullName,
+                              const CheckerRegistry::CmdLineOption &Option,
+                              AnalyzerOptions &AnOpts,
+                              DiagnosticsEngine &Diags) {
+
+  std::string FullOption = (FullName + ":" + Option.OptionName).str();
+
+  auto It = AnOpts.Config.insert({FullOption, Option.DefaultValStr});
+
+  // Insertation was successful -- CmdLineOption's constructor will validate
+  // whether values received from plugins or TableGen files are correct.
+  if (It.second)
+    return;
+
+  // Insertion failed, the user supplied this package/checker option on the
+  // command line. If the supplied value is invalid, we'll emit an error.
+
+  StringRef SuppliedValue = It.first->getValue();
+
+  if (Option.OptionType == "bool") {
+    if (SuppliedValue != "true" && SuppliedValue != "false") {
+      if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
+        Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+            << FullOption << "a boolean value";
+      }
+    }
+    return;
+  }
+
+  if (Option.OptionType == "int") {
+    int Tmp;
+    bool HasFailed = SuppliedValue.getAsInteger(0, Tmp);
+    if (HasFailed) {
+      if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
+        Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+            << FullOption << "an integer value";
+      }
+    }
+    return;
+  }
+}
+
 template <class T>
 static void
 insertOptionToCollection(StringRef FullName, T &Collection,
                          const CheckerRegistry::CmdLineOption &Option,
-                         AnalyzerOptions &AnOpts) {
+                         AnalyzerOptions &AnOpts, DiagnosticsEngine &Diags) {
   auto It = binaryFind(Collection, FullName);
   assert(It != Collection.end() &&
          "Failed to find the checker while attempting to add a command line "
          "option to it!");
 
-  AnOpts.Config.insert(
-      {(FullName + ":" + Option.OptionName).str(), Option.DefaultValStr});
+  insertAndValidate(FullName, Option, AnOpts, Diags);
 
   It->CmdLineOptions.emplace_back(Option);
 }
@@ -326,14 +370,14 @@ void CheckerRegistry::resolveCheckerAndPackageOptions() {
   for (const std::pair<StringRef, CmdLineOption> &CheckerOptEntry :
        CheckerOptions) {
     insertOptionToCollection(CheckerOptEntry.first, Checkers,
-                             CheckerOptEntry.second, AnOpts);
+                             CheckerOptEntry.second, AnOpts, Diags);
   }
   CheckerOptions.clear();
 
   for (const std::pair<StringRef, CmdLineOption> &PackageOptEntry :
        PackageOptions) {
-    insertOptionToCollection(PackageOptEntry.first, Checkers,
-                             PackageOptEntry.second, AnOpts);
+    insertOptionToCollection(PackageOptEntry.first, Packages,
+                             PackageOptEntry.second, AnOpts, Diags);
   }
   PackageOptions.clear();
 }
@@ -388,23 +432,62 @@ void CheckerRegistry::initializeManager(CheckerManager &CheckerMgr) const {
   }
 }
 
+static void
+isOptionContainedIn(const CheckerRegistry::CmdLineOptionList &OptionList,
+                    StringRef SuppliedChecker, StringRef SuppliedOption,
+                    const AnalyzerOptions &AnOpts, DiagnosticsEngine &Diags) {
+
+  if (!AnOpts.ShouldEmitErrorsOnInvalidConfigValue)
+    return;
+
+  using CmdLineOption = CheckerRegistry::CmdLineOption;
+
+  auto SameOptName = [SuppliedOption](const CmdLineOption &Opt) {
+    return Opt.OptionName == SuppliedOption;
+  };
+
+  auto OptionIt = llvm::find_if(OptionList, SameOptName);
+
+  if (OptionIt == OptionList.end()) {
+    Diags.Report(diag::err_analyzer_checker_option_unknown)
+        << SuppliedChecker << SuppliedOption;
+    return;
+  }
+}
+
 void CheckerRegistry::validateCheckerOptions() const {
   for (const auto &Config : AnOpts.Config) {
-    size_t Pos = Config.getKey().find(':');
-    if (Pos == StringRef::npos)
+
+    StringRef SuppliedChecker;
+    StringRef SuppliedOption;
+    std::tie(SuppliedChecker, SuppliedOption) = Config.getKey().split(':');
+
+    if (SuppliedOption.empty())
       continue;
 
-    bool HasChecker = false;
-    StringRef CheckerName = Config.getKey().substr(0, Pos);
-    for (const auto &Checker : Checkers) {
-      if (Checker.FullName.startswith(CheckerName) &&
-          (Checker.FullName.size() == Pos || Checker.FullName[Pos] == '.')) {
-        HasChecker = true;
-        break;
-      }
+    // AnalyzerOptions' config table contains the user input, so an entry could
+    // look like this:
+    //
+    //   cor:NoFalsePositives=true
+    //
+    // Since lower_bound would look for the first element *not less* than "cor",
+    // it would return with an iterator to the first checker in the core, so we
+    // we really have to use find here, which uses operator==.
+    auto CheckerIt = llvm::find(Checkers, CheckerInfo(SuppliedChecker));
+    if (CheckerIt != Checkers.end()) {
+      isOptionContainedIn(CheckerIt->CmdLineOptions, SuppliedChecker,
+                          SuppliedOption, AnOpts, Diags);
+      continue;
     }
-    if (!HasChecker)
-      Diags.Report(diag::err_unknown_analyzer_checker) << CheckerName;
+
+    auto PackageIt = llvm::find(Packages, PackageInfo(SuppliedChecker));
+    if (PackageIt != Packages.end()) {
+      isOptionContainedIn(PackageIt->CmdLineOptions, SuppliedChecker,
+                          SuppliedOption, AnOpts, Diags);
+      continue;
+    }
+
+    Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker;
   }
 }
 

diff --git a/test/Analysis/checker-plugins.c b/test/Analysis/checker-plugins.c
index 1e0d6c6341cca22c14213b359312a2150093675f..0745357e521addd25fb724e2745727eb6f9c14aa 100644 (file)
--- a/test/Analysis/checker-plugins.c
+++ b/test/Analysis/checker-plugins.c
@@ -62,3 +62,35 @@ void caller() {
 // RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-TRUE
 
 // CHECK-CHECKER-OPTION-TRUE: example.MyChecker:ExampleOption = true
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config example.MyChecker:Example=true \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER-OPTION
+
+// CHECK-NON-EXISTENT-CHECKER-OPTION: (frontend): checker 'example.MyChecker'
+// CHECK-NON-EXISTENT-CHECKER-OPTION-SAME: has no option called 'Example'
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config example.MyChecker:Example=true
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config example.MyChecker:ExampleOption=example \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-BOOL-VALUE
+
+// CHECK-INVALID-BOOL-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-BOOL-VALUE-SAME: 'example.MyChecker:ExampleOption', that
+// CHECK-INVALID-BOOL-VALUE-SAME: expects a boolean value
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config example.MyChecker:ExampleOption=example

diff --git a/test/Analysis/invalid-checker-option.c b/test/Analysis/invalid-checker-option.c
index 4ce783539fdf63cc66af7a9d3fafc3188d89ba6c..26dfcb2d56c186821955a68a1305e50857369c92 100644 (file)
--- a/test/Analysis/invalid-checker-option.c
+++ b/test/Analysis/invalid-checker-option.c
@@ -14,6 +14,63 @@
 // CHECK-NON-EXISTENT-CHECKER: (frontend): no analyzer checkers or packages
 // CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
 
+
+// Every other error should be avoidable in compatiblity mode.
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config debug.AnalysisOrder:Everything=false \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER-OPTION
+
+// CHECK-NON-EXISTENT-CHECKER-OPTION: (frontend): checker 'debug.AnalysisOrder'
+// CHECK-NON-EXISTENT-CHECKER-OPTION-SAME: has no option called 'Everything'
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:Everything=false
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config debug.AnalysisOrder:*=nothankyou \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-BOOL-VALUE
+
+// CHECK-INVALID-BOOL-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-BOOL-VALUE-SAME: 'debug.AnalysisOrder:*', that expects a
+// CHECK-INVALID-BOOL-VALUE-SAME: boolean value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:*=nothankyou
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=surpriseme \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-INT-VALUE
+
+// CHECK-INVALID-INT-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-INT-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-INVALID-INT-VALUE-SAME: expects an integer value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=surpriseme
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=sure \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PACKAGE-VALUE
+
+// CHECK-PACKAGE-VALUE: (frontend): invalid input for checker option
+// CHECK-PACKAGE-VALUE-SAME: 'nullability:NoDiagnoseCallsToSystemHeaders', that
+// CHECK-PACKAGE-VALUE-SAME: expects a boolean value
+
 // expected-no-diagnostics
 
 int main() {}