r358752 - [analyzer][NFC] Reimplement checker options

Kristof Umann via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 19 05:32:10 PDT 2019


Author: szelethus
Date: Fri Apr 19 05:32:10 2019
New Revision: 358752

URL: http://llvm.org/viewvc/llvm-project?rev=358752&view=rev
Log:
[analyzer][NFC] Reimplement checker options

TL;DR:

* Add checker and package options to the TableGen files
* Added a new class called CmdLineOption, and both Package and Checker recieved
   a list<CmdLineOption> field.
* Added every existing checker and package option to Checkers.td.
* The CheckerRegistry class
  * Received some comments to most of it's inline classes
  * Received the CmdLineOption and PackageInfo inline classes, a list of
     CmdLineOption was added to CheckerInfo and PackageInfo
  * Added addCheckerOption and addPackageOption
  * Added a new field called Packages, used in addPackageOptions, filled up in
     addPackage

Detailed description:

In the last couple months, a lot of effort was put into tightening the
analyzer's command line interface. The main issue is that it's spectacularly
easy to mess up a lenghty enough invocation of the analyzer, and the user was
given no warnings or errors at all in that case.

We can divide the effort of resolving this into several chapters:

* Non-checker analyzer configurations:
    Gather every analyzer configuration into a dedicated file. Emit errors for
    non-existent configurations or incorrect values. Be able to list these
    configurations. Tighten AnalyzerOptions interface to disallow making such
    a mistake in the future.

* Fix the "Checker Naming Bug" by reimplementing checker dependencies:
    When cplusplus.InnerPointer was enabled, it implicitly registered
    unix.Malloc, which implicitly registered some sort of a modeling checker
    from the CStringChecker family. This resulted in all of these checker
    objects recieving the name "cplusplus.InnerPointer", making AnalyzerOptions
    asking for the wrong checker options from the command line:
      cplusplus.InnerPointer:Optimisic
    istead of
      unix.Malloc:Optimistic.
    This was resolved by making CheckerRegistry responsible for checker
    dependency handling, instead of checkers themselves.

* Checker options: (this patch included!)
    Same as the first item, but for checkers.

(+ minor fixes here and there, and everything else that is yet to come)

There were several issues regarding checker options, that non-checker
configurations didn't suffer from: checker plugins are loaded runtime, and they
could add new checkers and new options, meaning that unlike for non-checker
configurations, we can't collect every checker option purely by generating code.
Also, as seen from the "Checker Naming Bug" issue raised above, they are very
rarely used in practice, and all sorts of skeletons fell out of the closet while
working on this project.

They were extremely problematic for users as well, purely because of how long
they were. Consider the following monster of a checker option:

  alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=false

While we were able to verify whether the checker itself (the part before the
colon) existed, any errors past that point were unreported, easily resulting
in 7+ hours of analyses going to waste.

This patch, similarly to how dependencies were reimplemented, uses TableGen to
register checker options into Checkers.td, so that Checkers.inc now contains
entries for both checker and package options. Using the preprocessor,
Checkers.inc is converted into code in CheckerRegistry, adding every builtin
(checkers and packages that have an entry in the Checkers.td file) checker and
package option to the registry. The new addPackageOption and addCheckerOption
functions expose the same functionality to statically-linked non-builtin and
plugin checkers and packages as well.

Emitting errors for incorrect user input, being able to list these options, and
some other functionalies will land in later patches.

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


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

Modified: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td?rev=358752&r1=358751&r2=358752&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td Fri Apr 19 05:32:10 2019
@@ -292,7 +292,7 @@ def err_omp_more_one_clause : Error<
 
 // 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">;
 }

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/CheckerBase.td?rev=358752&r1=358751&r2=358752&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/CheckerBase.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/CheckerBase.td Fri Apr 19 05:32:10 2019
@@ -10,14 +10,45 @@
 //
 //===----------------------------------------------------------------------===//
 
+/// 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;
+  // This field is optional.
+  list<CmdLineOption> PackageOptions;
+  Package             ParentPackage;
 }
 
 /// Describes a 'super' package that holds another package inside it. This is
@@ -52,11 +83,19 @@ class Documentation<DocumentationEnum va
 ///   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;
+  // This field is optional.
+  list<CmdLineOption> CheckerOptions;
+  // This field is optional.
+  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

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=358752&r1=358751&r2=358752&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Fri Apr 19 05:32:10 2019
@@ -42,7 +42,17 @@ def OptIn : Package<"optin">;
 // 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",
+                  "Suppresses warnings for violating nullability annotations "
+                  "of system header functions. This is useful if you are "
+                  "concerned with your custom nullability annotations more "
+                  "than with following nullability specifications of system "
+                  "header functions.",
+                  "false">
+  ]>;
 
 def Cplusplus : Package<"cplusplus">;
 def CplusplusAlpha : Package<"cplusplus">, ParentPackage<Alpha>;
@@ -371,6 +381,14 @@ def DynamicMemoryModeling: Checker<"Dyna
   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>;
 
@@ -447,7 +465,28 @@ def CXXSelfAssignmentChecker : Checker<"
   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"
@@ -456,6 +495,12 @@ let ParentPackage = CplusplusOptIn in {
 
 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"
@@ -492,7 +537,40 @@ def MismatchedIteratorChecker : Checker<
   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"
@@ -554,6 +632,13 @@ let ParentPackage = Performance in {
 
 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"
@@ -662,11 +747,18 @@ def MallocOverflowSecurityChecker : Chec
   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"
@@ -704,6 +796,14 @@ def NSOrCFErrorDerefChecker : Checker<"N
 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">,
@@ -795,6 +895,23 @@ def NSErrorChecker : Checker<"NSError">,
 
 def RetainCountChecker : Checker<"RetainCount">,
   HelpText<"Check for leaks and improper reference count management">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "CheckOSObject",
+                  "Find violations of retain-release rules applied to XNU "
+                  "OSObject instances. By default, the checker only checks "
+                  "retain-release rules for Objective-C NSObject instances "
+                  "and CoreFoundation objects.",
+                  "true">,
+    CmdLineOption<Boolean,
+                  "TrackNSCFStartParam",
+                  "Check not only that the code follows retain-release rules "
+                  "with respect to objects it allocates or borrows from "
+                  "elsewhere, but also that it fulfills its own retain count "
+                  "specification with respect to objects that it receives as "
+                  "arguments.",
+                  "false">
+  ]>,
   Dependencies<[RetainCountBase]>,
   Documentation<HasDocumentation>;
 
@@ -903,6 +1020,17 @@ let ParentPackage = LocalizabilityOptIn
 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 :
@@ -961,6 +1089,72 @@ let ParentPackage = Debug in {
 
 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">,
@@ -1034,6 +1228,25 @@ let ParentPackage = CloneDetectionAlpha
 
 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"

Modified: cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h?rev=358752&r1=358751&r2=358752&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h Fri Apr 19 05:32:10 2019
@@ -91,6 +91,27 @@ public:
   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>;
@@ -98,6 +119,8 @@ public:
   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.
@@ -113,6 +136,7 @@ public:
     StringRef FullName;
     StringRef Desc;
     StringRef DocumentationUri;
+    CmdLineOptionList CmdLineOptions;
     StateFromCmdLine State = StateFromCmdLine::State_Unspecified;
 
     ConstCheckerInfoList Dependencies;
@@ -136,6 +160,23 @@ public:
 
   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;
+
+    // Since each package must have a different full name, we can identify
+    // CheckerInfo objects by them.
+    bool operator==(const PackageInfo &Rhs) const {
+      return FullName == Rhs.FullName;
+    }
+
+    explicit PackageInfo(StringRef FullName) : FullName(FullName) {}
+  };
+
+  using PackageInfoList = llvm::SmallVector<PackageInfo, 0>;
+
 private:
   template <typename T> static void initializeManager(CheckerManager &mgr) {
     mgr.registerChecker<T>();
@@ -165,6 +206,35 @@ public:
   /// called \p dependency.
   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
   /// all checkers specified by the given CheckerOptInfo list. The order of this
@@ -193,21 +263,30 @@ private:
   CheckerInfoListRange getMutableCheckersForCmdLineArg(StringRef CmdLineArg);
 
   CheckerInfoList Checkers;
+  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;
 
   /// Contains all (Dependendent checker, Dependency) pairs. We need this, as
   /// we'll resolve dependencies after all checkers were added first.
   llvm::SmallVector<std::pair<StringRef, StringRef>, 0> Dependencies;
-
   void resolveDependencies();
 
+  /// Contains all (FullName, CmdLineOption) pairs. Similarly to dependencies,
+  /// we only modify the actual CheckerInfo and PackageInfo objects once all
+  /// of them have been added.
+  llvm::SmallVector<std::pair<StringRef, CmdLineOption>, 0> PackageOptions;
+  llvm::SmallVector<std::pair<StringRef, CmdLineOption>, 0> CheckerOptions;
+
+  void resolveCheckerAndPackageOptions();
+
   DiagnosticsEngine &Diags;
   AnalyzerOptions &AnOpts;
   const LangOptions &LangOpts;
 };
 
 } // namespace ento
-
 } // namespace clang
 
 #endif // LLVM_CLANG_STATICANALYZER_CORE_CHECKERREGISTRY_H

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=358752&r1=358751&r2=358752&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Apr 19 05:32:10 2019
@@ -422,7 +422,7 @@ static void initOption(AnalyzerOptions::
 
   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";

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp?rev=358752&r1=358751&r2=358752&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp Fri Apr 19 05:32:10 2019
@@ -45,6 +45,7 @@ template <class T> struct FullNameLT {
   }
 };
 
+using PackageNameLT = FullNameLT<CheckerRegistry::PackageInfo>;
 using CheckerNameLT = FullNameLT<CheckerRegistry::CheckerInfo>;
 } // end of anonymous namespace
 
@@ -118,6 +119,9 @@ CheckerRegistry::CheckerRegistry(
   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
@@ -166,6 +170,7 @@ CheckerRegistry::CheckerRegistry(
   // 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(Packages, PackageNameLT{});
   llvm::sort(Checkers, CheckerNameLT{});
 
 #define GET_CHECKER_DEPENDENCIES
@@ -173,11 +178,24 @@ CheckerRegistry::CheckerRegistry(
 #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
 
   resolveDependencies();
+  resolveCheckerAndPackageOptions();
 
   // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
   // command line.
@@ -266,20 +284,6 @@ CheckerRegistry::CheckerInfoSet CheckerR
   return EnabledCheckers;
 }
 
-void CheckerRegistry::addChecker(InitializationFunction Rfn,
-                                 ShouldRegisterFunction Sfn, StringRef Name,
-                                 StringRef Desc, StringRef DocsUri) {
-  Checkers.emplace_back(Rfn, Sfn, Name, Desc, DocsUri);
-
-  // Record the presence of the checker in its packages.
-  StringRef PackageName, LeafName;
-  std::tie(PackageName, LeafName) = Name.rsplit(PackageSeparator);
-  while (!LeafName.empty()) {
-    PackageSizes[PackageName] += 1;
-    std::tie(PackageName, LeafName) = PackageName.rsplit(PackageSeparator);
-  }
-}
-
 void CheckerRegistry::resolveDependencies() {
   for (const std::pair<StringRef, StringRef> &Entry : Dependencies) {
     auto CheckerIt = binaryFind(Checkers, Entry.first);
@@ -302,6 +306,72 @@ void CheckerRegistry::addDependency(Stri
   Dependencies.emplace_back(FullName, Dependency);
 }
 
+template <class T>
+static void
+insertOptionToCollection(StringRef FullName, T &Collection,
+                         const CheckerRegistry::CmdLineOption &&Option) {
+  auto It = binaryFind(Collection, FullName);
+  assert(It != Collection.end() &&
+         "Failed to find the checker while attempting to add a command line "
+         "option to it!");
+
+  It->CmdLineOptions.emplace_back(std::move(Option));
+}
+
+void CheckerRegistry::resolveCheckerAndPackageOptions() {
+  for (const std::pair<StringRef, CmdLineOption> &CheckerOptEntry :
+       CheckerOptions) {
+    insertOptionToCollection(CheckerOptEntry.first, Checkers,
+                             std::move(CheckerOptEntry.second));
+  }
+  CheckerOptions.clear();
+
+  for (const std::pair<StringRef, CmdLineOption> &PackageOptEntry :
+       PackageOptions) {
+    insertOptionToCollection(PackageOptEntry.first, Checkers,
+                             std::move(PackageOptEntry.second));
+  }
+  PackageOptions.clear();
+}
+
+void CheckerRegistry::addPackage(StringRef FullName) {
+  Packages.emplace_back(PackageInfo(FullName));
+}
+
+void CheckerRegistry::addPackageOption(StringRef OptionType,
+                                       StringRef PackageFullName,
+                                       StringRef OptionName,
+                                       StringRef DefaultValStr,
+                                       StringRef Description) {
+  PackageOptions.emplace_back(
+      PackageFullName,
+      CmdLineOption{OptionType, OptionName, DefaultValStr, Description});
+}
+
+void CheckerRegistry::addChecker(InitializationFunction Rfn,
+                                 ShouldRegisterFunction Sfn, StringRef Name,
+                                 StringRef Desc, StringRef DocsUri) {
+  Checkers.emplace_back(Rfn, Sfn, Name, Desc, DocsUri);
+
+  // Record the presence of the checker in its packages.
+  StringRef PackageName, LeafName;
+  std::tie(PackageName, LeafName) = Name.rsplit(PackageSeparator);
+  while (!LeafName.empty()) {
+    PackageSizes[PackageName] += 1;
+    std::tie(PackageName, LeafName) = PackageName.rsplit(PackageSeparator);
+  }
+}
+
+void CheckerRegistry::addCheckerOption(StringRef OptionType,
+                                       StringRef CheckerFullName,
+                                       StringRef OptionName,
+                                       StringRef DefaultValStr,
+                                       StringRef Description) {
+  CheckerOptions.emplace_back(
+      CheckerFullName,
+      CmdLineOption{OptionType, OptionName, DefaultValStr, Description});
+}
+
 void CheckerRegistry::initializeManager(CheckerManager &CheckerMgr) const {
   // Collect checkers enabled by the options.
   CheckerInfoSet enabledCheckers = getEnabledCheckers();

Modified: cfe/trunk/test/Analysis/disable-all-checks.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/disable-all-checks.c?rev=358752&r1=358751&r2=358752&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/disable-all-checks.c (original)
+++ cfe/trunk/test/Analysis/disable-all-checks.c Fri Apr 19 05:32:10 2019
@@ -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;

Added: cfe/trunk/test/Analysis/invalid-checker-option.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/invalid-checker-option.c?rev=358752&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/invalid-checker-option.c (added)
+++ cfe/trunk/test/Analysis/invalid-checker-option.c Fri Apr 19 05:32:10 2019
@@ -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() {}

Modified: cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp?rev=358752&r1=358751&r2=358752&view=diff
==============================================================================
--- cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp Fri Apr 19 05:32:10 2019
@@ -90,6 +90,26 @@ static std::string getCheckerDocs(const
       .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 @@ void EmitClangSACheckers(RecordKeeper &R
   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 @@ void EmitClangSACheckers(RecordKeeper &R
   //   - 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 @@ void EmitClangSACheckers(RecordKeeper &R
   }
   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




More information about the cfe-commits mailing list