Szelethus updated this revision to Diff 189088.
Szelethus added a comment.
Herald added a subscriber: Charusso.

- Capitalized some variable names
- Added more comments
- Preferring binary searches to linear searches wherever possible


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57855/new/

https://reviews.llvm.org/D57855

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/StaticAnalyzer/Checkers/CheckerBase.td
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/disable-all-checks.c
  test/Analysis/invalid-checker-option.c
  utils/TableGen/ClangSACheckersEmitter.cpp

Index: utils/TableGen/ClangSACheckersEmitter.cpp
===================================================================
--- utils/TableGen/ClangSACheckersEmitter.cpp
+++ utils/TableGen/ClangSACheckersEmitter.cpp
@@ -90,6 +90,26 @@
       .str();
 }
 
+/// Retrieves the type from a CmdOptionTypeEnum typed Record object. Note that
+/// the class itself has to be modified for adding a new option type in
+/// CheckerBase.td.
+static std::string getCheckerOptionType(const Record &R) {
+  if (BitsInit *BI = R.getValueAsBitsInit("Type")) {
+    switch(getValueFromBitsInit(BI, R)) {
+    case 0:
+      return "int";
+    case 1:
+      return "string";
+    case 2:
+      return "bool";
+    }
+  }
+  PrintFatalError(R.getLoc(),
+                  "unable to parse command line option type for "
+                  + getCheckerFullName(&R));
+  return "";
+}
+
 static void printChecker(llvm::raw_ostream &OS, const Record &R) {
     OS << "CHECKER(" << "\"";
     OS.write_escaped(getCheckerFullName(&R)) << "\", ";
@@ -134,6 +154,45 @@
   OS << "#endif // GET_PACKAGES\n"
         "\n";
 
+  // Emit a package option.
+  //
+  // PACKAGE_OPTION(OPTIONTYPE, PACKAGENAME, OPTIONNAME, DESCRIPTION, DEFAULT)
+  //   - OPTIONTYPE: Type of the option, whether it's integer or boolean etc.
+  //                 This is important for validating user input. Note that
+  //                 it's a string, rather than an actual type: since we can
+  //                 load checkers runtime, we can't use template hackery for
+  //                 sorting this out compile-time.
+  //   - PACKAGENAME: Name of the package.
+  //   - OPTIONNAME: Name of the option.
+  //   - DESCRIPTION
+  //   - DEFAULT: The default value for this option.
+  //
+  // The full option can be specified in the command like like this:
+  //   -analyzer-config PACKAGENAME:OPTIONNAME=VALUE
+  OS << "\n"
+        "#ifdef GET_PACKAGE_OPTIONS\n";
+  for (const Record *Package : packages) {
+
+    if (Package->isValueUnset("PackageOptions"))
+      continue;
+
+    std::vector<Record *> PackageOptions = Package
+                                       ->getValueAsListOfDefs("PackageOptions");
+    for (Record *PackageOpt : PackageOptions) {
+      OS << "PACKAGE_OPTION(\"";
+      OS.write_escaped(getCheckerOptionType(*PackageOpt)) << "\", \"";
+      OS.write_escaped(getPackageFullName(Package)) << "\", ";
+      OS << '\"' << getStringValue(*PackageOpt, "CmdFlag") << "\", ";
+      OS << '\"';
+      OS.write_escaped(getStringValue(*PackageOpt, "Desc")) << "\", ";
+      OS << '\"';
+      OS.write_escaped(getStringValue(*PackageOpt, "DefaultVal")) << "\"";
+      OS << ")\n";
+    }
+  }
+  OS << "#endif // GET_PACKAGE_OPTIONS\n"
+        "\n";
+
   // Emit checkers.
   //
   // CHECKER(FULLNAME, CLASS, HELPTEXT)
@@ -160,15 +219,15 @@
   //   - DEPENDENCY: The full name of the checker FULLNAME depends on.
   OS << "\n"
         "#ifdef GET_CHECKER_DEPENDENCIES\n";
-  for (const Record *checker : checkers) {
-    if (checker->isValueUnset("Dependencies"))
+  for (const Record *Checker : checkers) {
+    if (Checker->isValueUnset("Dependencies"))
       continue;
 
     for (const Record *Dependency :
-                            checker->getValueAsListOfDefs("Dependencies")) {
+                            Checker->getValueAsListOfDefs("Dependencies")) {
       OS << "CHECKER_DEPENDENCY(";
       OS << '\"';
-      OS.write_escaped(getCheckerFullName(checker)) << "\", ";
+      OS.write_escaped(getCheckerFullName(Checker)) << "\", ";
       OS << '\"';
       OS.write_escaped(getCheckerFullName(Dependency)) << '\"';
       OS << ")\n";
@@ -176,5 +235,45 @@
   }
   OS << "\n"
         "#endif // GET_CHECKER_DEPENDENCIES\n";
+
+  // Emit a package option.
+  //
+  // CHECKER_OPTION(OPTIONTYPE, CHECKERNAME, OPTIONNAME, DESCRIPTION, DEFAULT)
+  //   - OPTIONTYPE: Type of the option, whether it's integer or boolean etc.
+  //                 This is important for validating user input. Note that
+  //                 it's a string, rather than an actual type: since we can
+  //                 load checkers runtime, we can't use template hackery for
+  //                 sorting this out compile-time.
+  //   - CHECKERNAME: Name of the package.
+  //   - OPTIONNAME: Name of the option.
+  //   - DESCRIPTION
+  //   - DEFAULT: The default value for this option.
+  //
+  // The full option can be specified in the command like like this:
+  //   -analyzer-config CHECKERNAME:OPTIONNAME=VALUE
+  OS << "\n"
+        "#ifdef GET_CHECKER_OPTIONS\n";
+  for (const Record *checker : checkers) {
+
+    if (checker->isValueUnset("CheckerOptions"))
+      continue;
+
+    std::vector<Record *> CheckerOptions = checker
+                                       ->getValueAsListOfDefs("CheckerOptions");
+    for (Record *CheckerOpt : CheckerOptions) {
+      OS << "CHECKER_OPTION(\"";
+      OS << getCheckerOptionType(*CheckerOpt) << "\", \"";
+      OS.write_escaped(getCheckerFullName(checker)) << "\", ";
+      OS << '\"' << getStringValue(*CheckerOpt, "CmdFlag") << "\", ";
+      OS << '\"';
+      OS.write_escaped(getStringValue(*CheckerOpt, "Desc")) << "\", ";
+      OS << '\"';
+      OS.write_escaped(getStringValue(*CheckerOpt, "DefaultVal")) << "\"";
+      OS << ")";
+      OS << '\n';
+    }
+  }
+  OS << "#endif // GET_CHECKER_OPTIONS\n"
+        "\n";
 }
 } // end namespace clang
Index: test/Analysis/invalid-checker-option.c
===================================================================
--- /dev/null
+++ test/Analysis/invalid-checker-option.c
@@ -0,0 +1,19 @@
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config RetainOneTwoThree:CheckOSObject=false \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER
+
+// Note that non-existent packages and checkers were always reported.
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config RetainOneTwoThree:CheckOSObject=false \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER
+
+// CHECK-NON-EXISTENT-CHECKER: (frontend): no analyzer checkers or packages
+// CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
+
+// expected-no-diagnostics
+
+int main() {}
Index: test/Analysis/disable-all-checks.c
===================================================================
--- test/Analysis/disable-all-checks.c
+++ test/Analysis/disable-all-checks.c
@@ -12,7 +12,7 @@
 //
 // expected-no-diagnostics
 
-// CHECK: no analyzer checkers are associated with 'non.existant.Checker'
+// CHECK: no analyzer checkers or packages are associated with 'non.existant.Checker'
 // CHECK: use -analyzer-disable-all-checks to disable all static analyzer checkers
 int buggy() {
   int x = 0;
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ 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/CheckerManager.h"
@@ -18,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
+#include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
@@ -38,9 +40,37 @@
   return strcmp(versionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
 }
 
-static bool checkerNameLT(const CheckerRegistry::CheckerInfo &a,
-                          const CheckerRegistry::CheckerInfo &b) {
-  return a.FullName < b.FullName;
+namespace {
+template <class CheckerOrPackageInfo>
+struct FullNameLT {
+  bool operator()(const CheckerOrPackageInfo &Lhs,
+                  const CheckerOrPackageInfo &Rhs) {
+    return Lhs.FullName < Rhs.FullName;
+  }
+};
+
+using CheckerNameLT = FullNameLT<CheckerRegistry::CheckerInfo>;
+using PackageNameLT = FullNameLT<CheckerRegistry::PackageInfo>;
+} // end of anonymous namespace
+
+template <class CheckerOrPackageInfoList>
+static typename
+std::conditional<std::is_const<CheckerOrPackageInfoList>::value,
+                 typename CheckerOrPackageInfoList::const_iterator,
+                 typename CheckerOrPackageInfoList::iterator>::type
+    binaryFind(CheckerOrPackageInfoList &Collection, StringRef FullName) {
+
+  using CheckerOrPackage = typename CheckerOrPackageInfoList::value_type;
+  using CheckerOrPackageFullNameLT = FullNameLT<CheckerOrPackage>;
+
+  assert(std::is_sorted(Collection.begin(), Collection.end(),
+                        CheckerOrPackageFullNameLT{}) &&
+         "Can't do binary search on a non-sorted collection!");
+
+  typename CheckerOrPackageInfoList::value_type Info(FullName);
+
+  return llvm::lower_bound(Collection, Info,
+      FullNameLT<typename CheckerOrPackageInfoList::value_type>{});
 }
 
 static constexpr char PackageSeparator = '.';
@@ -65,15 +95,7 @@
 CheckerRegistry::CheckerInfoListRange
 CheckerRegistry::getMutableCheckersForCmdLineArg(StringRef CmdLineArg) {
 
-  assert(std::is_sorted(Checkers.begin(), Checkers.end(), checkerNameLT) &&
-         "In order to efficiently gather checkers, this function expects them "
-         "to be already sorted!");
-
-  // Use a binary search to find the possible start of the package.
-  CheckerRegistry::CheckerInfo
-      packageInfo(nullptr, nullptr, CmdLineArg, "", "");
-  auto it = std::lower_bound(Checkers.begin(), Checkers.end(),
-                             packageInfo, checkerNameLT);
+  auto it = binaryFind(Checkers, CmdLineArg);
 
   if (!isInPackage(*it, CmdLineArg))
     return { Checkers.end(), Checkers.end() };
@@ -83,38 +105,44 @@
   // checker.
   size_t size = 1;
   llvm::StringMap<size_t>::const_iterator packageSize =
-      Packages.find(CmdLineArg);
+      PackageSizes.find(CmdLineArg);
 
-  if (packageSize != Packages.end())
+  if (packageSize != PackageSizes.end())
     size = packageSize->getValue();
 
   return { it, it + size };
 }
 
 CheckerRegistry::CheckerRegistry(
-     ArrayRef<std::string> plugins, DiagnosticsEngine &diags,
+     ArrayRef<std::string> Plugins, DiagnosticsEngine &Diags,
      AnalyzerOptions &AnOpts, const LangOptions &LangOpts,
      ArrayRef<std::function<void(CheckerRegistry &)>>
-         checkerRegistrationFns)
-  : Diags(diags), AnOpts(AnOpts), LangOpts(LangOpts) {
+         CheckerRegistrationFns)
+  : Diags(Diags), AnOpts(AnOpts), LangOpts(LangOpts) {
 
   // Register builtin checkers.
 #define GET_CHECKERS
 #define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI)                            \
   addChecker(register##CLASS, shouldRegister##CLASS, FULLNAME, HELPTEXT,       \
              DOC_URI);
+
+#define GET_PACKAGES
+#define PACKAGE(FULLNAME)                                                      \
+  addPackage(FULLNAME);
+
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER
 #undef GET_CHECKERS
+#undef PACKAGE
+#undef GET_PACKAGES
 
   // Register checkers from plugins.
-  for (ArrayRef<std::string>::iterator i = plugins.begin(), e = plugins.end();
-       i != e; ++i) {
+  for (const std::string &Plugin : Plugins) {
     // Get access to the plugin.
     std::string err;
-    DynamicLibrary lib = DynamicLibrary::getPermanentLibrary(i->c_str(), &err);
+    DynamicLibrary lib = DynamicLibrary::getPermanentLibrary(Plugin.c_str(), &err);
     if (!lib.isValid()) {
-      diags.Report(diag::err_fe_unable_to_load_plugin) << *i << err;
+      Diags.Report(diag::err_fe_unable_to_load_plugin) << Plugin << err;
       continue;
     }
 
@@ -123,7 +151,7 @@
       (const char *) lib.getAddressOfSymbol("clang_analyzerAPIVersionString");
     if (!isCompatibleAPIVersion(pluginAPIVersion)) {
       Diags.Report(diag::warn_incompatible_analyzer_plugin_api)
-          << llvm::sys::path::filename(*i);
+          << llvm::sys::path::filename(Plugin);
       Diags.Report(diag::note_incompatible_analyzer_plugin_api)
           << CLANG_ANALYZER_API_VERSION_STRING
           << pluginAPIVersion;
@@ -142,37 +170,50 @@
   // file, but rather passed their registry function as a parameter in 
   // checkerRegistrationFns. 
 
-  for (const auto &Fn : checkerRegistrationFns)
+  for (const auto &Fn : CheckerRegistrationFns)
     Fn(*this);
 
   // Sort checkers for efficient collection.
   // FIXME: Alphabetical sort puts 'experimental' in the middle.
   // Would it be better to name it '~experimental' or something else
   // that's ASCIIbetically last?
-  llvm::sort(Checkers, checkerNameLT);
+  llvm::sort(Checkers, CheckerNameLT{});
+  llvm::sort(Packages, PackageNameLT{});
 
 #define GET_CHECKER_DEPENDENCIES
 
 #define CHECKER_DEPENDENCY(FULLNAME, DEPENDENCY)                               \
   addDependency(FULLNAME, DEPENDENCY);
 
+#define GET_CHECKER_OPTIONS
+#define CHECKER_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL)             \
+  addCheckerOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC);
+
+#define GET_PACKAGE_OPTIONS
+#define PACKAGE_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL)             \
+  addPackageOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC);
+
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER_DEPENDENCY
 #undef GET_CHECKER_DEPENDENCIES
+#undef CHECKER_OPTION
+#undef GET_CHECKER_OPTIONS
+#undef PACKAGE_OPTION
+#undef GET_PACKAGE_OPTIONS
 
   // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
   // command line.
-  for (const std::pair<std::string, bool> &opt : AnOpts.CheckersControlList) {
-    CheckerInfoListRange checkersForCmdLineArg =
-                                     getMutableCheckersForCmdLineArg(opt.first);
+  for (const std::pair<std::string, bool> &Opt : AnOpts.CheckersControlList) {
+    CheckerInfoListRange CheckerForCmdLineArg =
+                                     getMutableCheckersForCmdLineArg(Opt.first);
 
-    if (checkersForCmdLineArg.begin() == checkersForCmdLineArg.end()) {
-      Diags.Report(diag::err_unknown_analyzer_checker) << opt.first;
+    if (CheckerForCmdLineArg.begin() == CheckerForCmdLineArg.end()) {
+      Diags.Report(diag::err_unknown_analyzer_checker) << Opt.first;
       Diags.Report(diag::note_suggest_disabling_all_checkers);
     }
 
-    for (CheckerInfo &checker : checkersForCmdLineArg) {
-      checker.State = opt.second ? StateFromCmdLine::State_Enabled :
+    for (CheckerInfo &checker : CheckerForCmdLineArg) {
+      checker.State = Opt.second ? StateFromCmdLine::State_Enabled :
                                    StateFromCmdLine::State_Disabled;
     }
   }
@@ -255,11 +296,58 @@
   StringRef packageName, leafName;
   std::tie(packageName, leafName) = Name.rsplit(PackageSeparator);
   while (!leafName.empty()) {
-    Packages[packageName] += 1;
+    PackageSizes[packageName] += 1;
     std::tie(packageName, leafName) = packageName.rsplit(PackageSeparator);
   }
 }
 
+void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
+  auto CheckerIt = binaryFind(Checkers, FullName);
+  assert(CheckerIt != Checkers.end() &&
+         "Failed to find the checker while attempting to set up it's "
+         "dependencies!");
+
+  auto DependencyIt = binaryFind(Checkers, Dependency);
+  assert(DependencyIt != Checkers.end() &&
+         "Failed to find the dependency of a checker!");
+
+  CheckerIt->Dependencies.emplace_back(&*DependencyIt);
+}
+
+void CheckerRegistry::addCheckerOption(StringRef OptionType,
+                                       StringRef CheckerFullName,
+                                       StringRef OptionName,
+                                       StringRef DefaultValStr,
+                                       StringRef Description) {
+
+  auto CheckerIt = binaryFind(Checkers, CheckerFullName);
+  assert(CheckerIt != Checkers.end() &&
+         "Failed to find the checker while attempting to add a command line "
+         "option to it!");
+
+  CheckerIt->CmdLineOptions.emplace_back(
+      OptionType, OptionName, DefaultValStr, Description);
+}
+
+void CheckerRegistry::addPackage(StringRef FullName) {
+  Packages.emplace_back(PackageInfo(FullName));
+}
+
+void CheckerRegistry::addPackageOption(StringRef OptionType,
+                                       StringRef FullName,
+                                       StringRef OptionName,
+                                       StringRef DefaultValStr,
+                                       StringRef Description) {
+
+  auto PackageIt = binaryFind(Packages, FullName);
+  assert(PackageIt != Packages.end() &&
+         "Failed to find the package while attempting to add a command line "
+         "option to it!");
+
+  PackageIt->CmdLineOptions.emplace_back(
+      OptionType, OptionName, DefaultValStr, Description);
+}
+
 void CheckerRegistry::initializeManager(CheckerManager &checkerMgr) const {
   // Collect checkers enabled by the options.
   CheckerInfoSet enabledCheckers = getEnabledCheckers();
@@ -277,25 +365,26 @@
     if (pos == StringRef::npos)
       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;
-      }
-    }
-    if (!hasChecker)
-      Diags.Report(diag::err_unknown_analyzer_checker) << checkerName;
+    StringRef SuppliedChecker(config.first().substr(0, pos));
+
+    // We can't get away with binary search here, as it uses lower_bound.
+    if (llvm::binary_search(
+            Checkers, CheckerInfo(SuppliedChecker), CheckerNameLT{}))
+      continue;
+
+    if (llvm::binary_search(
+            Packages, PackageInfo(SuppliedChecker), PackageNameLT{}))
+      continue;
+
+    Diags.Report(diag::err_unknown_analyzer_checker) << SuppliedChecker;
   }
 }
 
-void CheckerRegistry::printHelp(raw_ostream &out,
-                                size_t maxNameChars) const {
+void CheckerRegistry::printCheckerWithDescList(raw_ostream &Out,
+                                               size_t MaxNameChars) const {
   // FIXME: Print available packages.
 
-  out << "CHECKERS:\n";
+  Out << "CHECKERS:\n";
 
   // Find the maximum option length.
   size_t optionFieldWidth = 0;
@@ -303,31 +392,31 @@
     // Limit the amount of padding we are willing to give up for alignment.
     //   Package.Name     Description  [Hidden]
     size_t nameLength = i.FullName.size();
-    if (nameLength <= maxNameChars)
+    if (nameLength <= MaxNameChars)
       optionFieldWidth = std::max(optionFieldWidth, nameLength);
   }
 
   const size_t initialPad = 2;
   for (const auto &i : Checkers) {
-    out.indent(initialPad) << i.FullName;
+    Out.indent(initialPad) << i.FullName;
 
     int pad = optionFieldWidth - i.FullName.size();
 
     // Break on long option names.
     if (pad < 0) {
-      out << '\n';
+      Out << '\n';
       pad = optionFieldWidth + initialPad;
     }
-    out.indent(pad + 2) << i.Desc;
+    Out.indent(pad + 2) << i.Desc;
 
-    out << '\n';
+    Out << '\n';
   }
 }
 
-void CheckerRegistry::printList(raw_ostream &out) const {
+void CheckerRegistry::printEnabledCheckerList(raw_ostream &Out) const {
   // Collect checkers enabled by the options.
   CheckerInfoSet enabledCheckers = getEnabledCheckers();
 
   for (const auto *i : enabledCheckers)
-    out << i->FullName << '\n';
+    Out << i->FullName << '\n';
 }
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -50,7 +50,8 @@
   out << "OVERVIEW: Clang Static Analyzer Checkers List\n\n";
   out << "USAGE: -analyzer-checker <CHECKER or PACKAGE,...>\n\n";
 
-  CheckerRegistry(plugins, diags, anopts, langOpts).printHelp(out);
+  CheckerRegistry(plugins, diags, anopts, langOpts)
+      .printCheckerWithDescList(out);
 }
 
 void ento::printEnabledCheckerList(raw_ostream &out,
@@ -60,7 +61,8 @@
                                    const LangOptions &langOpts) {
   out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
 
-  CheckerRegistry(plugins, diags, anopts, langOpts).printList(out);
+  CheckerRegistry(plugins, diags, anopts, langOpts)
+      .printEnabledCheckerList(out);
 }
 
 void ento::printAnalyzerConfigList(raw_ostream &out) {
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -422,7 +422,7 @@
 
   OptionField = DefaultVal;
   bool HasFailed = getStringOption(Config, Name, std::to_string(DefaultVal))
-                     .getAsInteger(10, OptionField);
+                     .getAsInteger(0, OptionField);
   if (Diags && HasFailed)
     Diags->Report(diag::err_analyzer_config_invalid_input)
       << Name << "an unsigned";
Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -92,6 +92,26 @@
   using InitializationFunction = void (*)(CheckerManager &);
   using ShouldRegisterFunction = bool (*)(const LangOptions &);
 
+  /// Specifies a command line option. It may either belong to a checker or a
+  /// package.
+  struct CmdLineOption {
+    StringRef OptionType;
+    StringRef OptionName;
+    StringRef DefaultValStr;
+    StringRef Description;
+
+    CmdLineOption(StringRef OptionType, StringRef OptionName,
+                  StringRef DefaultValStr, StringRef Description)
+      : OptionType(OptionType), OptionName(OptionName),
+        DefaultValStr(DefaultValStr), Description(Description) {
+
+      assert((OptionType == "bool" || OptionType == "string" ||
+              OptionType == "int") && "Unknown command line option type!");
+    }
+  };
+
+  using CmdLineOptionList = llvm::SmallVector<CmdLineOption, 0>;
+
   struct CheckerInfo;
 
   using CheckerInfoList = std::vector<CheckerInfo>;
@@ -99,6 +119,8 @@
   using ConstCheckerInfoList = llvm::SmallVector<const CheckerInfo *, 0>;
   using CheckerInfoSet = llvm::SetVector<const CheckerInfo *>;
 
+  /// Specifies a checker. Note that this isn't what we call a checker object,
+  /// it merely contains everything required to create one.
   struct CheckerInfo {
     enum class StateFromCmdLine {
       // This checker wasn't explicitly enabled or disabled.
@@ -109,11 +131,12 @@
       State_Enabled
     };
 
-    InitializationFunction Initialize;
-    ShouldRegisterFunction ShouldRegister;
+    InitializationFunction Initialize = nullptr;
+    ShouldRegisterFunction ShouldRegister = nullptr;
     StringRef FullName;
     StringRef Desc;
     StringRef DocumentationUri;
+    CmdLineOptionList CmdLineOptions;
     StateFromCmdLine State = StateFromCmdLine::State_Unspecified;
 
     ConstCheckerInfoList Dependencies;
@@ -130,10 +153,24 @@
                 StringRef Name, StringRef Desc, StringRef DocsUri)
         : Initialize(Fn), ShouldRegister(sfn), FullName(Name), Desc(Desc),
           DocumentationUri(DocsUri) {}
+
+    // Used for lower_bound.
+    CheckerInfo(StringRef FullName) : FullName(FullName) {}
   };
 
   using StateFromCmdLine = CheckerInfo::StateFromCmdLine;
 
+  /// Specifies a package. Each package option is implicitly an option for all
+  /// checkers within the package.
+  struct PackageInfo {
+    StringRef FullName;
+    CmdLineOptionList CmdLineOptions;
+
+    explicit PackageInfo(StringRef FullName) : FullName(FullName) {}
+  };
+
+  using PackageInfoList = llvm::SmallVector<PackageInfo, 0>;
+
 private:
   template <typename T>
   static void initializeManager(CheckerManager &mgr) {
@@ -164,25 +201,40 @@
 
   /// Makes the checker with the full name \p fullName depends on the checker
   /// called \p dependency.
-  void addDependency(StringRef fullName, StringRef dependency) {
-    auto CheckerThatNeedsDeps =
-       [&fullName](const CheckerInfo &Chk) { return Chk.FullName == fullName; };
-    auto Dependency =
-      [&dependency](const CheckerInfo &Chk) {
-        return Chk.FullName == dependency;
-      };
-
-    auto CheckerIt = llvm::find_if(Checkers, CheckerThatNeedsDeps);
-    assert(CheckerIt != Checkers.end() &&
-           "Failed to find the checker while attempting to set up it's "
-           "dependencies!");
-
-    auto DependencyIt = llvm::find_if(Checkers, Dependency);
-    assert(DependencyIt != Checkers.end() &&
-           "Failed to find the dependency of a checker!");
-
-    CheckerIt->Dependencies.push_back(&*DependencyIt);
-  }
+  void addDependency(StringRef fullName, StringRef dependency);
+
+  /// Registers an option to a given checker. A checker option will always have
+  /// the following format:
+  ///   CheckerFullName:OptionName=Value
+  /// And can be specified from the command line like this:
+  ///   -analyzer-config CheckerFullName:OptionName=Value
+  ///
+  /// Options for unknown checkers, or unknown options for a given checker, or
+  /// invalid value types for that given option are reported as an error in
+  /// non-compatibility mode.
+  void addCheckerOption(StringRef OptionType,
+                        StringRef CheckerFullName,
+                        StringRef OptionName,
+                        StringRef DefaultValStr,
+                        StringRef Description);
+
+  /// Adds a package to the registry.
+  void addPackage(StringRef FullName);
+
+  /// Registers an option to a given package. A package option will always have
+  /// the following format:
+  ///   PackageFullName:OptionName=Value
+  /// And can be specified from the command line like this:
+  ///   -analyzer-config PackageFullName:OptionName=Value
+  ///
+  /// Options for unknown packages, or unknown options for a given package, or
+  /// invalid value types for that given option are reported as an error in
+  /// non-compatibility mode.
+  void addPackageOption(StringRef OptionType,
+                        StringRef PackageFullName,
+                        StringRef OptionName,
+                        StringRef DefaultValStr,
+                        StringRef Description);
 
   // FIXME: This *really* should be added to the frontend flag descriptions.
   /// Initializes a CheckerManager by calling the initialization functions for
@@ -196,8 +248,9 @@
 
   /// Prints the name and description of all checkers in this registry.
   /// This output is not intended to be machine-parseable.
-  void printHelp(raw_ostream &out, size_t maxNameChars = 30) const;
-  void printList(raw_ostream &out) const;
+  void printCheckerWithDescList(raw_ostream &Out,
+                                size_t MaxNameChars = 30) const;
+  void printEnabledCheckerList(raw_ostream &Out) const;
 
 private:
   /// Collect all enabled checkers. The returned container preserves the order
@@ -211,7 +264,10 @@
   CheckerInfoListRange getMutableCheckersForCmdLineArg(StringRef CmdLineArg);
 
   CheckerInfoList Checkers;
-  llvm::StringMap<size_t> Packages;
+  PackageInfoList Packages;
+  /// Used for couting how many checkers belong to a certain package in the
+  /// \c Checkers field. For convenience purposes.
+  llvm::StringMap<size_t> PackageSizes;
 
   DiagnosticsEngine &Diags;
   AnalyzerOptions &AnOpts;
@@ -219,7 +275,6 @@
 };
 
 } // namespace ento
-
 } // namespace clang
 
 #endif // LLVM_CLANG_STATICANALYZER_CORE_CHECKERREGISTRY_H
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -42,7 +42,13 @@
 // development, but unwanted for developers who target only a single platform.
 def PortabilityOptIn : Package<"portability">, ParentPackage<OptIn>;
 
-def Nullability : Package<"nullability">;
+def Nullability : Package<"nullability">,
+  PackageOptions<[
+    CmdLineOption<Boolean,
+                  "NoDiagnoseCallsToSystemHeaders",
+                  "",
+                  "false">
+  ]>;
 
 def Cplusplus : Package<"cplusplus">;
 def CplusplusAlpha : Package<"cplusplus">, ParentPackage<Alpha>;
@@ -369,6 +375,14 @@
   HelpText<"The base of several malloc() related checkers. On it's own it "
            "emits no reports, but adds valuable information to the analysis "
            "when enabled.">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "Optimistic",
+                  "If set to true, the checker assumes that all the "
+                  "allocating and deallocating functions are annotated with "
+                  "ownership_holds, ownership_takes and ownership_returns.",
+                  "false">
+  ]>,
   Dependencies<[CStringModeling]>,
   Documentation<NotDocumented>;
 
@@ -445,7 +459,28 @@
   Documentation<NotDocumented>;
 
 def MoveChecker: Checker<"Move">,
-   HelpText<"Find use-after-move bugs in C++">,
+  HelpText<"Find use-after-move bugs in C++">,
+  CheckerOptions<[
+    CmdLineOption<String,
+                  "WarnOn",
+                  "In non-aggressive mode, only warn on use-after-move of "
+                  "local variables (or local rvalue references) and of STL "
+                  "objects. The former is possible because local variables (or "
+                  "local rvalue references) are not tempting their user to "
+                  "re-use the storage. The latter is possible because STL "
+                  "objects are known to end up in a valid but unspecified "
+                  "state after the move and their state-reset methods are also "
+                  "known, which allows us to predict precisely when "
+                  "use-after-move is invalid. Some STL objects are known to "
+                  "conform to additional contracts after move, so they are not "
+                  "tracked. However, smart pointers specifically are tracked "
+                  "because we can perform extra checking over them. In "
+                  "aggressive mode, warn on any use-after-move because the "
+                  "user has intentionally asked us to completely eliminate "
+                  "use-after-move in his code. Values: \"KnownsOnly\", "
+                  "\"KnownsAndLocals\", \"All\".",
+                  "KnownsAndLocals">
+  ]>,
   Documentation<HasDocumentation>;
 
 } // end: "cplusplus"
@@ -454,6 +489,12 @@
 
 def VirtualCallChecker : Checker<"VirtualCall">,
   HelpText<"Check virtual function calls during construction or destruction">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "PureOnly",
+                  "Whether to only report calls to pure virtual methods.",
+                  "false">
+  ]>,
   Documentation<HasDocumentation>;
 
 } // end: "optin.cplusplus"
@@ -490,7 +531,40 @@
   Documentation<HasAlphaDocumentation>;
 
 def UninitializedObjectChecker: Checker<"UninitializedObject">,
-     HelpText<"Reports uninitialized fields after object construction">,
+  HelpText<"Reports uninitialized fields after object construction">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "Pedantic",
+                  "If set to false, the checker won't emit warnings "
+                  "for objects that don't have at least one initialized "
+                  "field.",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "NotesAsWarnings",
+                  "If set to true, the checker will emit a warning "
+                  "for each uninitalized field, as opposed to emitting one "
+                  "warning per constructor call, and listing the uninitialized "
+                  "fields that belongs to it in notes.",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "CheckPointeeInitialization",
+                  "If set to false, the checker will not analyze "
+                  "the pointee of pointer/reference fields, and will only "
+                  "check whether the object itself is initialized.",
+                  "false">,
+    CmdLineOption<String,
+                  "IgnoreRecordsWithField",
+                  "If supplied, the checker will not analyze "
+                  "structures that have a field with a name or type name that "
+                  "matches the given pattern.",
+                  "\"\"">,
+    CmdLineOption<Boolean,
+                  "IgnoreGuardedFields",
+                  "If set to true, the checker will analyze _syntactically_ "
+                  "whether the found uninitialized object is used without a "
+                  "preceding assert call. Defaults to false.",
+                  "false">
+  ]>,
   Documentation<HasAlphaDocumentation>;
 
 } // end: "alpha.cplusplus"
@@ -551,6 +625,13 @@
 
 def PaddingChecker : Checker<"Padding">,
   HelpText<"Check for excessively padded structs.">,
+  CheckerOptions<[
+    CmdLineOption<Integer,
+                  "AllowedPad",
+                  "Reports are only generated if the excessive padding exceeds "
+                  "'AllowedPad' in bytes.",
+                  "24">
+  ]>,
   Documentation<NotDocumented>;
 
 } // end: "padding"
@@ -659,11 +740,18 @@
   HelpText<"Check for overflows in the arguments to malloc()">,
   Documentation<HasAlphaDocumentation>;
 
-// Operating systems specific PROT_READ/PROT_WRITE values is not implemented,
-// the defaults are correct for several common operating systems though,
-// but may need to be overridden via the related analyzer-config flags.
 def MmapWriteExecChecker : Checker<"MmapWriteExec">,
   HelpText<"Warn on mmap() calls that are both writable and executable">,
+  CheckerOptions<[
+    CmdLineOption<Integer,
+                  "MmapProtExec",
+                  "Specifies the value of PROT_EXEC",
+                  "0x04">,
+    CmdLineOption<Integer,
+                  "MmapProtRead",
+                  "Specifies the value of PROT_READ",
+                  "0x01">
+  ]>,
   Documentation<HasAlphaDocumentation>;
 
 } // end "alpha.security"
@@ -701,6 +789,14 @@
 def NumberObjectConversionChecker : Checker<"NumberObjectConversion">,
   HelpText<"Check for erroneous conversions of objects representing numbers "
            "into numbers">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "Pedantic",
+                  "Enables detection of more conversion patterns (which are "
+                  "most likely more harmless, and therefore are more likely to "
+                  "produce false positives).",
+                  "false">
+  ]>,
   Documentation<NotDocumented>;
 
 def MacOSXAPIChecker : Checker<"API">,
@@ -787,6 +883,22 @@
 
 def RetainCountChecker : Checker<"RetainCount">,
   HelpText<"Check for leaks and improper reference count management">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "leak-diagnostics-reference-allocation",
+                  "Reports are only generated if the excessive padding exceeds "
+                  "'AllowedPad' in bytes.",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "CheckOSObject",
+                  "Reports are only generated if the excessive padding exceeds "
+                  "'AllowedPad' in bytes.",
+                  "true">,
+    CmdLineOption<Boolean,
+                  "TrackNSCFStartParam",
+                  "",
+                  "false">
+  ]>,
   Dependencies<[RetainCountBase]>,
   Documentation<HasDocumentation>;
 
@@ -895,6 +1007,17 @@
 def NonLocalizedStringChecker : Checker<"NonLocalizedStringChecker">,
   HelpText<"Warns about uses of non-localized NSStrings passed to UI methods "
            "expecting localized NSStrings">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "AggressiveReport",
+                  "Marks a string being returned by any call as localized if "
+                  "it is in LocStringFunctions (LSF) or the function is "
+                  "annotated. Otherwise, we mark it as NonLocalized "
+                  "(Aggressive) or NonLocalized only if it is not backed by a "
+                  "SymRegion (Non-Aggressive), basically leaving only string "
+                  "literals as NonLocalized.",
+                  "false">
+  ]>,
   Documentation<HasDocumentation>;
 
 def EmptyLocalizationContextChecker :
@@ -953,6 +1076,72 @@
 
 def AnalysisOrderChecker : Checker<"AnalysisOrder">,
   HelpText<"Print callbacks that are called during analysis in order">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "PreStmtCastExpr",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PostStmtCastExpr",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PreStmtArraySubscriptExpr",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PostStmtArraySubscriptExpr",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PreStmtCXXNewExpr",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PostStmtCXXNewExpr",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PreStmtOffsetOfExpr",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PostStmtOffsetOfExpr",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PreCall",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "PostCall",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "EndFunction",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "NewAllocator",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "Bind",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "LiveSymbols",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "RegionChanges",
+                  "",
+                  "false">,
+    CmdLineOption<Boolean,
+                  "*",
+                  "Enables all callbacks.",
+                  "false">
+  ]>,
   Documentation<NotDocumented>;
 
 def DominatorsTreeDumper : Checker<"DumpDominators">,
@@ -1022,6 +1211,25 @@
 
 def CloneChecker : Checker<"CloneChecker">,
   HelpText<"Reports similar pieces of code.">,
+  CheckerOptions<[
+    CmdLineOption<Integer,
+                  "MinimumCloneComplexity",
+                  "Ensures that every clone has at least the given complexity. "
+                  "Complexity is here defined as the total amount of children "
+                  "of a statement. This constraint assumes the first statement "
+                  "in the group is representative for all other statements in "
+                  "the group in terms of complexity.",
+                  "50">,
+    CmdLineOption<Boolean,
+                  "ReportNormalClones",
+                  "Report all clones, even less suspicious ones.",
+                  "true">,
+    CmdLineOption<String,
+                  "IgnoredFilesPattern",
+                  "If supplied, the checker wont analyze files with a filename "
+                  "that matches the given pattern.",
+                  "\"\"">
+  ]>,
   Documentation<HasAlphaDocumentation>;
 
 } // end "clone"
Index: include/clang/StaticAnalyzer/Checkers/CheckerBase.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/CheckerBase.td
+++ include/clang/StaticAnalyzer/Checkers/CheckerBase.td
@@ -10,14 +10,44 @@
 //
 //===----------------------------------------------------------------------===//
 
+/// Describes a checker or package option type. This is important for validating
+/// user supplied inputs.
+/// New option types can be added by modifying this enum. Note that this
+/// requires changes in the TableGen emitter file ClangSACheckersEmitter.cpp.
+class CmdLineOptionTypeEnum<bits<2> val> {
+  bits<2> Type = val;
+}
+def Integer : CmdLineOptionTypeEnum<0>;
+def String : CmdLineOptionTypeEnum<1>;
+def Boolean : CmdLineOptionTypeEnum<2>;
+
+class Type<CmdLineOptionTypeEnum val> {
+  bits<2> Type = val.Type;
+}
+
+/// Describes an option for a checker or a package.
+class CmdLineOption<CmdLineOptionTypeEnum type, string cmdFlag, string desc,
+                    string defaultVal> {
+  bits<2> Type = type.Type;
+  string CmdFlag = cmdFlag;
+  string Desc = desc;
+  string DefaultVal = defaultVal;
+}
+
+/// Describes a list of package options.
+class PackageOptions<list<CmdLineOption> opts> {
+  list<CmdLineOption> PackageOptions = opts;
+}
+
 /// Describes a package. Every checker is a part of a package, for example,
 /// 'NullDereference' is part of the 'core' package, hence it's full name is
 /// 'core.NullDereference'.
 /// Example:
 ///   def Core : Package<"core">;
 class Package<string name> {
-  string       PackageName = name;
-  Package ParentPackage;
+  string              PackageName = name;
+  list<CmdLineOption> PackageOptions;
+  Package             ParentPackage;
 }
 
 /// Describes a 'super' package that holds another package inside it. This is
@@ -52,11 +82,17 @@
 ///   def DereferenceChecker : Checker<"NullDereference">,
 ///     HelpText<"Check for dereferences of null pointers">;
 class Checker<string name = ""> {
-  string        CheckerName = name;
-  string        HelpText;
-  list<Checker> Dependencies;
-  bits<2>       Documentation;
-  Package       ParentPackage;
+  string              CheckerName = name;
+  string              HelpText;
+  list<CmdLineOption> CheckerOptions;
+  list<Checker>       Dependencies;
+  bits<2>             Documentation;
+  Package             ParentPackage;
+}
+
+/// Describes a list of checker options.
+class CheckerOptions<list<CmdLineOption> opts> {
+  list<CmdLineOption> CheckerOptions = opts;
 }
 
 /// Describes dependencies in between checkers. For example, InnerPointerChecker
Index: include/clang/Basic/DiagnosticCommonKinds.td
===================================================================
--- include/clang/Basic/DiagnosticCommonKinds.td
+++ include/clang/Basic/DiagnosticCommonKinds.td
@@ -292,7 +292,7 @@
 
 // Static Analyzer Core
 def err_unknown_analyzer_checker : Error<
-    "no analyzer checkers are associated with '%0'">;
+    "no analyzer checkers or packages are associated with '%0'">;
 def note_suggest_disabling_all_checkers : Note<
     "use -analyzer-disable-all-checks to disable all static analyzer checkers">;
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to