Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, 
baloghadamsoftware.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.
Herald added a project: clang.

Validate whether the option exists, and also whether the supplied value is of 
the correct type.

This is the final part of the checker option refactoring. I guess we could 
arrange the `AnalyzerOptions` related changes to 5 "chapters":

1. Reimplement `-analyzer-config` options
2. Fix the checker naming bug by reimplementing checker dependencies
3. Reimplement checker options
4. (already in the works!) Document the frontend of the analyzer, sneak peak 
here 
<https://github.com/Szelethus/clang/blob/frontend_doc/docs/analyzer/developer-docs/analyzer_frontend.rst>.
5. (soon™) Make `AnalyzerOptions` const everywhere after the analyzer is 
initialized, make `AnalyzerOptions::ConfigTable` private, reimplement 
`debug.ConfigDumper`.


Repository:
  rC Clang

https://reviews.llvm.org/D57860

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/invalid-checker-option.c

Index: test/Analysis/invalid-checker-option.c
===================================================================
--- test/Analysis/invalid-checker-option.c
+++ test/Analysis/invalid-checker-option.c
@@ -14,6 +14,53 @@
 // 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
+
 // expected-no-diagnostics
 
 int main() {}
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -365,6 +365,55 @@
   }
 }
 
+static void validateOptionNameAndValue(
+    const CheckerRegistry::CmdLineOptionList &OptionList,
+    StringRef SuppliedChecker,
+    StringRef SuppliedOption,
+    StringRef SuppliedValue,
+    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);
+
+  // In this case we know that Supplied* point to different parts of the same
+  // string.
+  StringRef FullOption(SuppliedChecker.data(),
+                       SuppliedChecker.size() + SuppliedOption.size() + 1);
+
+  if (OptionIt == OptionList.end()) {
+    Diags.Report(diag::err_analyzer_checker_option_unknown)
+        << SuppliedChecker << SuppliedOption;
+    return;
+  }
+
+  if (OptionIt->OptionType == "bool") {
+    if (SuppliedValue != "true" && SuppliedValue != "false") {
+      Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+          << FullOption << "a boolean";
+      return;
+    }
+  }
+
+  if (OptionIt->OptionType == "int") {
+    int Tmp;
+    bool HasFailed = SuppliedValue.getAsInteger(0, Tmp);
+    if (HasFailed) {
+      Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+          << FullOption << "an integer";
+      return;
+    }
+  }
+}
+
 void CheckerRegistry::validateCheckerOptions() const {
   for (const auto &config : AnOpts.Config) {
     size_t pos = config.getKey().find(':');
@@ -372,16 +421,28 @@
       continue;
 
     StringRef SuppliedChecker(config.first().substr(0, pos));
+    StringRef SuppliedOption(config.first().drop_front(pos + 1));
+    StringRef SuppliedValue(config.second);
 
     auto CheckerIt =
         llvm::find_if(Checkers, SameCheckerName(SuppliedChecker));
-    if (CheckerIt != Checkers.end())
+    if (CheckerIt != Checkers.end()) {
+      validateOptionNameAndValue(CheckerIt->CmdLineOptions,
+                                 SuppliedChecker,
+                                 SuppliedOption,
+                                 SuppliedValue, AnOpts, Diags);
       continue;
+    }
 
     auto PackageIt =
         llvm::find_if(Packages, SamePackageName(SuppliedChecker));
-    if (PackageIt != Packages.end())
+    if (PackageIt != Packages.end()) {
+      validateOptionNameAndValue(PackageIt->CmdLineOptions,
+                                 SuppliedChecker,
+                                 SuppliedOption,
+                                 SuppliedValue, AnOpts, Diags);
       continue;
+    }
 
     Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker;
   }
Index: include/clang/Basic/DiagnosticDriverKinds.td
===================================================================
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -301,6 +301,8 @@
 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 value">;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D57860: [analyzer]... Kristóf Umann via Phabricator via cfe-commits

Reply via email to