[analyzer] Enable subcheckers to possess checker options

Under the term "subchecker", I mean checkers that do not have a checker class on
their own, like unix.MallocChecker to unix.DynamicMemoryModeling.

Since a checker object was required in order to retrieve checker options,
subcheckers couldn't possess options on their own.

This patch is also an excuse to change the argument order of getChecker*Option,
it always bothered me, now it resembles the actual command line argument
(checkername:option=value).

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


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

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

@@ -272,54 +272,74 @@ public:
   /// interpreted as true and the "false" string is interpreted as false.
   ///
   /// If an option value is not provided, returns the given \p DefaultVal.
-  /// @param [in] Name Name for option to retrieve.
+  /// @param [in] CheckerName The *full name* of the checker. One may retrieve
+  /// this from the checker object's field \c Name, or through \c
+  /// CheckerManager::getCurrentCheckName within the checker's registry
+  /// function.
+  /// Checker options are retrieved in the following format:
+  /// `-analyzer-config CheckerName:OptionName=Value.
+  /// @param [in] OptionName Name for option to retrieve.
   /// @param [in] DefaultVal Default value returned if no such option was
   /// specified.
-  /// @param [in] C The checker object the option belongs to. Checker options
-  /// are retrieved in the following format:
-  /// `-analyzer-config <package and checker name>:OptionName=Value.
   /// @param [in] SearchInParents If set to true and the searched option was not
   /// specified for the given checker the options for the parent packages will
   /// be searched as well. The inner packages take precedence over the outer
   /// ones.
-  bool getCheckerBooleanOption(StringRef Name, bool DefaultVal,
-                        const ento::CheckerBase *C,
-                        bool SearchInParents = false) const;
+  bool getCheckerBooleanOption(StringRef CheckerName, StringRef OptionName,
+                               bool DefaultVal,
+                               bool SearchInParents = false) const;
 
+  bool getCheckerBooleanOption(const ento::CheckerBase *C, StringRef OptionName,
+                               bool DefaultVal,
+                               bool SearchInParents = false) const;
 
   /// Interprets an option's string value as an integer value.
   ///
   /// If an option value is not provided, returns the given \p DefaultVal.
-  /// @param [in] Name Name for option to retrieve.
+  /// @param [in] CheckerName The *full name* of the checker. One may retrieve
+  /// this from the checker object's field \c Name, or through \c
+  /// CheckerManager::getCurrentCheckName within the checker's registry
+  /// function.
+  /// Checker options are retrieved in the following format:
+  /// `-analyzer-config CheckerName:OptionName=Value.
+  /// @param [in] OptionName Name for option to retrieve.
   /// @param [in] DefaultVal Default value returned if no such option was
   /// specified.
-  /// @param [in] C The checker object the option belongs to. Checker options
-  /// are retrieved in the following format:
-  /// `-analyzer-config <package and checker name>:OptionName=Value.
   /// @param [in] SearchInParents If set to true and the searched option was not
   /// specified for the given checker the options for the parent packages will
   /// be searched as well. The inner packages take precedence over the outer
   /// ones.
-  int getCheckerIntegerOption(StringRef Name, int DefaultVal,
-                         const ento::CheckerBase *C,
-                         bool SearchInParents = false) const;
+  int getCheckerIntegerOption(StringRef CheckerName, StringRef OptionName,
+                              int DefaultVal,
+                              bool SearchInParents = false) const;
+
+  int getCheckerIntegerOption(const ento::CheckerBase *C, StringRef OptionName,
+                              int DefaultVal,
+                              bool SearchInParents = false) const;
 
   /// Query an option's string value.
   ///
   /// If an option value is not provided, returns the given \p DefaultVal.
-  /// @param [in] Name Name for option to retrieve.
+  /// @param [in] CheckerName The *full name* of the checker. One may retrieve
+  /// this from the checker object's field \c Name, or through \c
+  /// CheckerManager::getCurrentCheckName within the checker's registry
+  /// function.
+  /// Checker options are retrieved in the following format:
+  /// `-analyzer-config CheckerName:OptionName=Value.
+  /// @param [in] OptionName Name for option to retrieve.
   /// @param [in] DefaultVal Default value returned if no such option was
   /// specified.
-  /// @param [in] C The checker object the option belongs to. Checker options
-  /// are retrieved in the following format:
-  /// `-analyzer-config <package and checker name>:OptionName=Value.
   /// @param [in] SearchInParents If set to true and the searched option was not
   /// specified for the given checker the options for the parent packages will
   /// be searched as well. The inner packages take precedence over the outer
   /// ones.
-  StringRef getCheckerStringOption(StringRef Name, StringRef DefaultVal,
-                              const ento::CheckerBase *C,
-                              bool SearchInParents = false) const;
+  StringRef getCheckerStringOption(StringRef CheckerName, StringRef OptionName,
+                                   StringRef DefaultVal,
+                                   bool SearchInParents = false) const;
+
+  StringRef getCheckerStringOption(const ento::CheckerBase *C,
+                                   StringRef OptionName, StringRef DefaultVal,
+                                   bool SearchInParents = false) const;
 
   /// Retrieves and sets the UserMode. This is a high-level option,
   /// which is used to set other low-level options. It is not accessible

lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp

@@ -44,8 +44,8 @@ class AnalysisOrderChecker
                      check::LiveSymbols> {
 
   bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const {
-    return Opts.getCheckerBooleanOption("*", false, this) ||
-        Opts.getCheckerBooleanOption(CallbackName, false, this);
+    return Opts.getCheckerBooleanOption(this, "*", false) ||
+        Opts.getCheckerBooleanOption(this, CallbackName, false);
   }
 
   bool isCallbackEnabled(CheckerContext &C, StringRef CallbackName) const {

lib/StaticAnalyzer/Checkers/CloneChecker.cpp

@@ -63,17 +63,17 @@ void CloneChecker::checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
   // the CloneDetector. The only thing left to do is to report the found clones.
 
   int MinComplexity = Mgr.getAnalyzerOptions().getCheckerIntegerOption(
-      "MinimumCloneComplexity", 50, this);
+      this, "MinimumCloneComplexity", 50);
   assert(MinComplexity >= 0);
 
   bool ReportSuspiciousClones = Mgr.getAnalyzerOptions()
-    .getCheckerBooleanOption("ReportSuspiciousClones", true, this);
+    .getCheckerBooleanOption(this, "ReportSuspiciousClones", true);
 
   bool ReportNormalClones = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
-      "ReportNormalClones", true, this);
+      this, "ReportNormalClones", true);
 
   StringRef IgnoredFilesPattern = Mgr.getAnalyzerOptions()
-    .getCheckerStringOption("IgnoredFilesPattern", "", this);
+    .getCheckerStringOption(this, "IgnoredFilesPattern", "");
 
   // Let the CloneDetector create a list of clones from all the analyzed
   // statements. We don't filter for matching variable patterns at this point

lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp

@@ -1397,8 +1397,8 @@ void ento::registerNonLocalizedStringChecker(CheckerManager &mgr) {
   NonLocalizedStringChecker *checker =
       mgr.registerChecker<NonLocalizedStringChecker>();
   checker->IsAggressive =
-      mgr.getAnalyzerOptions().getCheckerBooleanOption("AggressiveReport",
-                                                       false, checker);
+      mgr.getAnalyzerOptions().getCheckerBooleanOption(
+          checker, "AggressiveReport", false);
 }
 
 bool ento::shouldRegisterNonLocalizedStringChecker(const LangOptions &LO) {
@@ -1409,7 +1409,8 @@ void ento::registerEmptyLocalizationContextChecker(CheckerManager &mgr) {
   mgr.registerChecker<EmptyLocalizationContextChecker>();
 }
 
-bool ento::shouldRegisterEmptyLocalizationContextChecker(const LangOptions &LO) {
+bool ento::shouldRegisterEmptyLocalizationContextChecker(
+                                                        const LangOptions &LO) {
   return true;
 }
 

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -3098,7 +3098,7 @@ void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) {
 void ento::registerDynamicMemoryModeling(CheckerManager &mgr) {
   auto *checker = mgr.registerChecker<MallocChecker>();
   checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
-                                                  "Optimistic", false, checker);
+                                                  checker, "Optimistic", false);
 }
 
 bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) {

lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp

@@ -82,10 +82,10 @@ void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
       mgr.registerChecker<MmapWriteExecChecker>();
   Mwec->ProtExecOv =
     mgr.getAnalyzerOptions()
-      .getCheckerIntegerOption("MmapProtExec", 0x04, Mwec);
+      .getCheckerIntegerOption(Mwec, "MmapProtExec", 0x04);
   Mwec->ProtReadOv =
     mgr.getAnalyzerOptions()
-      .getCheckerIntegerOption("MmapProtRead", 0x01, Mwec);
+      .getCheckerIntegerOption(Mwec, "MmapProtRead", 0x01);
 }
 
 bool ento::shouldRegisterMmapWriteExecChecker(const LangOptions &LO) {

lib/StaticAnalyzer/Checkers/MoveChecker.cpp

@@ -735,7 +735,7 @@ void MoveChecker::printState(raw_ostream &Out, ProgramStateRef State,
 void ento::registerMoveChecker(CheckerManager &mgr) {
   MoveChecker *chk = mgr.registerChecker<MoveChecker>();
   chk->setAggressiveness(
-      mgr.getAnalyzerOptions().getCheckerStringOption("WarnOn", "", chk));
+      mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn", ""));
 }
 
 bool ento::shouldRegisterMoveChecker(const LangOptions &LO) {

lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

@@ -1208,7 +1208,7 @@ bool ento::shouldRegisterNullabilityBase(const LangOptions &LO) {
     checker->NoDiagnoseCallsToSystemHeaders =                                  \
         checker->NoDiagnoseCallsToSystemHeaders ||                             \
         mgr.getAnalyzerOptions().getCheckerBooleanOption(                      \
-                      "NoDiagnoseCallsToSystemHeaders", false, checker, true); \
+                      checker, "NoDiagnoseCallsToSystemHeaders", false, true); \
   }                                                                            \
                                                                                \
   bool ento::shouldRegister##name##Checker(const LangOptions &LO) {            \

lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp

@@ -346,7 +346,7 @@ void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
   NumberObjectConversionChecker *Chk =
       Mgr.registerChecker<NumberObjectConversionChecker>();
   Chk->Pedantic =
-      Mgr.getAnalyzerOptions().getCheckerBooleanOption("Pedantic", false, Chk);
+      Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic", false);
 }
 
 bool ento::shouldRegisterNumberObjectConversionChecker(const LangOptions &LO) {

lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

@@ -32,17 +32,14 @@ namespace {
 class PaddingChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
 private:
   mutable std::unique_ptr<BugType> PaddingBug;
-  mutable int64_t AllowedPad;
   mutable BugReporter *BR;
 
 public:
+  int64_t AllowedPad;
+
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
                     BugReporter &BRArg) const {
     BR = &BRArg;
-    AllowedPad =
-        MGR.getAnalyzerOptions()
-          .getCheckerIntegerOption("AllowedPad", 24, this);
-    assert(AllowedPad >= 0 && "AllowedPad option should be non-negative");
 
     // The calls to checkAST* from AnalysisConsumer don't
     // visit template instantiations or lambda classes. We
@@ -348,7 +345,11 @@ public:
 } // namespace
 
 void ento::registerPaddingChecker(CheckerManager &Mgr) {
-  Mgr.registerChecker<PaddingChecker>();
+  auto *Checker = Mgr.registerChecker<PaddingChecker>();
+  Checker->AllowedPad = Mgr.getAnalyzerOptions()
+          .getCheckerIntegerOption(Checker, "AllowedPad", 24);
+  assert(Checker->AllowedPad >= 0 &&
+         "AllowedPad option should be non-negative");
 }
 
 bool ento::shouldRegisterPaddingChecker(const LangOptions &LO) {

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

@@ -611,18 +611,17 @@ void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) {
   UninitObjCheckerOptions &ChOpts = Chk->Opts;
 
   ChOpts.IsPedantic =
-      AnOpts.getCheckerBooleanOption("Pedantic", /*DefaultVal*/ false, Chk);
-  ChOpts.ShouldConvertNotesToWarnings =
-      AnOpts.getCheckerBooleanOption("NotesAsWarnings",
-                                     /*DefaultVal*/ false, Chk);
+      AnOpts.getCheckerBooleanOption(Chk, "Pedantic", /*DefaultVal*/ false);
+  ChOpts.ShouldConvertNotesToWarnings = AnOpts.getCheckerBooleanOption(
+      Chk, "NotesAsWarnings", /*DefaultVal*/ false);
   ChOpts.CheckPointeeInitialization = AnOpts.getCheckerBooleanOption(
-      "CheckPointeeInitialization", /*DefaultVal*/ false, Chk);
+      Chk, "CheckPointeeInitialization", /*DefaultVal*/ false);
   ChOpts.IgnoredRecordsWithFieldPattern =
-      AnOpts.getCheckerStringOption("IgnoreRecordsWithField",
-                                    /*DefaultVal*/ "", Chk);
+      AnOpts.getCheckerStringOption(Chk, "IgnoreRecordsWithField",
+                                    /*DefaultVal*/ "");
   ChOpts.IgnoreGuardedFields =
-      AnOpts.getCheckerBooleanOption("IgnoreGuardedFields",
-                                     /*DefaultVal*/ false, Chk);
+      AnOpts.getCheckerBooleanOption(Chk, "IgnoreGuardedFields",
+                                     /*DefaultVal*/ false);
 }
 
 bool ento::shouldRegisterUninitializedObjectChecker(const LangOptions &LO) {

lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp

@@ -279,8 +279,8 @@ void ento::registerVirtualCallChecker(CheckerManager &mgr) {
   VirtualCallChecker *checker = mgr.registerChecker<VirtualCallChecker>();
 
   checker->IsPureOnly =
-      mgr.getAnalyzerOptions().getCheckerBooleanOption("PureOnly", false,
-                                                       checker);
+      mgr.getAnalyzerOptions().getCheckerBooleanOption(
+                                                    checker, "PureOnly", false);
 }
 
 bool ento::shouldRegisterVirtualCallChecker(const LangOptions &LO) {

lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

@@ -101,18 +101,14 @@ AnalyzerOptions::mayInlineCXXMemberFunction(
   return *K >= Param;
 }
 
-StringRef AnalyzerOptions::getCheckerStringOption(StringRef OptionName,
+StringRef AnalyzerOptions::getCheckerStringOption(StringRef CheckerName,
+                                                  StringRef OptionName,
                                                   StringRef DefaultVal,
-                                                  const CheckerBase *C,
-                                                  bool SearchInParents) const {
-  assert(C);
-  // Search for a package option if the option for the checker is not specified
-  // and search in parents is enabled.
-  StringRef CheckerName = C->getTagDescription();
-
+                                                  bool SearchInParents ) const {
   assert(!CheckerName.empty() &&
          "Empty checker name! Make sure the checker object (including it's "
          "bases!) if fully initialized before calling this function!");
+
   ConfigTable::const_iterator E = Config.end();
   do {
     ConfigTable::const_iterator I =
@@ -127,29 +123,56 @@ StringRef AnalyzerOptions::getCheckerStringOption(StringRef OptionName,
   return DefaultVal;
 }
 
-bool AnalyzerOptions::getCheckerBooleanOption(StringRef Name, bool DefaultVal,
-                                              const CheckerBase *C,
-                                              bool SearchInParents) const {
+StringRef AnalyzerOptions::getCheckerStringOption(const ento::CheckerBase *C,
+                                                  StringRef OptionName,
+                                                  StringRef DefaultVal,
+                                                  bool SearchInParents ) const {
+  return getCheckerStringOption(
+             C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+}
+
+bool AnalyzerOptions::getCheckerBooleanOption(StringRef CheckerName,
+                                              StringRef OptionName,
+                                              bool DefaultVal,
+                                              bool SearchInParents ) const {
   // FIXME: We should emit a warning here if the value is something other than
   // "true", "false", or the empty string (meaning the default value),
   // but the AnalyzerOptions doesn't have access to a diagnostic engine.
-  assert(C);
   return llvm::StringSwitch<bool>(
-      getCheckerStringOption(Name, DefaultVal ? "true" : "false", C,
+      getCheckerStringOption(CheckerName, OptionName,
+                             DefaultVal ? "true" : "false",
                              SearchInParents))
       .Case("true", true)
       .Case("false", false)
       .Default(DefaultVal);
 }
 
-int AnalyzerOptions::getCheckerIntegerOption(StringRef Name, int DefaultVal,
-                                        const CheckerBase *C,
-                                        bool SearchInParents) const {
+bool AnalyzerOptions::getCheckerBooleanOption(const ento::CheckerBase *C,
+                                              StringRef OptionName,
+                                              bool DefaultVal,
+                                              bool SearchInParents ) const {
+  return getCheckerBooleanOption(
+             C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+}
+
+int AnalyzerOptions::getCheckerIntegerOption(StringRef CheckerName,
+                                             StringRef OptionName,
+                                             int DefaultVal,
+                                             bool SearchInParents ) const {
   int Ret = DefaultVal;
-  bool HasFailed = getCheckerStringOption(Name, std::to_string(DefaultVal), C,
+  bool HasFailed = getCheckerStringOption(CheckerName, OptionName,
+                                          std::to_string(DefaultVal),
                                           SearchInParents)
                      .getAsInteger(10, Ret);
   assert(!HasFailed && "analyzer-config option should be numeric");
   (void)HasFailed;
   return Ret;
 }
+
+int AnalyzerOptions::getCheckerIntegerOption(const ento::CheckerBase *C,
+                                             StringRef OptionName,
+                                             int DefaultVal,
+                                             bool SearchInParents ) const {
+  return getCheckerIntegerOption(
+             C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+}

unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp

@@ -48,28 +48,28 @@ TEST(StaticAnalyzerOptions, SearchInParentPackageTests) {
     }
   };
 
-  // Checker one has Option specified as true. It should read true regardless of
-  // search mode.
+  // CheckerTwo one has Option specified as true. It should read true regardless
+  // of search mode.
   CheckerOneMock CheckerOne;
-  EXPECT_TRUE(Opts.getCheckerBooleanOption("Option", false, &CheckerOne));
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option", false));
   // The package option is overridden with a checker option.
-  EXPECT_TRUE(Opts.getCheckerBooleanOption("Option", false, &CheckerOne,
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option", false,
                                            true));
   // The Outer package option is overridden by the Inner package option. No
   // package option is specified.
-  EXPECT_TRUE(Opts.getCheckerBooleanOption("Option2", false, &CheckerOne,
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option2", false,
                                            true));
   // No package option is specified and search in packages is turned off. The
   // default value should be returned.
-  EXPECT_FALSE(Opts.getCheckerBooleanOption("Option2", false, &CheckerOne));
-  EXPECT_TRUE(Opts.getCheckerBooleanOption("Option2", true, &CheckerOne));
+  EXPECT_FALSE(Opts.getCheckerBooleanOption(&CheckerOne, "Option2", false));
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerOne, "Option2", true));
 
   // Checker true has no option specified. It should get the default value when
   // search in parents turned off and false when search in parents turned on.
   CheckerTwoMock CheckerTwo;
-  EXPECT_FALSE(Opts.getCheckerBooleanOption("Option", false, &CheckerTwo));
-  EXPECT_TRUE(Opts.getCheckerBooleanOption("Option", true, &CheckerTwo));
-  EXPECT_FALSE(Opts.getCheckerBooleanOption("Option", true, &CheckerTwo, true));
+  EXPECT_FALSE(Opts.getCheckerBooleanOption(&CheckerTwo, "Option", false));
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(&CheckerTwo, "Option", true));
+  EXPECT_FALSE(Opts.getCheckerBooleanOption(&CheckerTwo, "Option", true, true));
 }
 
 TEST(StaticAnalyzerOptions, StringOptions) {
@@ -84,9 +84,17 @@ TEST(StaticAnalyzerOptions, StringOptions) {
 
   CheckerOneMock CheckerOne;
   EXPECT_TRUE("StringValue" ==
-            Opts.getCheckerStringOption("Option", "DefaultValue", &CheckerOne));
+            Opts.getCheckerStringOption(&CheckerOne, "Option", "DefaultValue"));
   EXPECT_TRUE("DefaultValue" ==
-           Opts.getCheckerStringOption("Option2", "DefaultValue", &CheckerOne));
+           Opts.getCheckerStringOption(&CheckerOne, "Option2", "DefaultValue"));
 }
+
+TEST(StaticAnalyzerOptions, SubCheckerOptions) {
+  AnalyzerOptions Opts;
+  Opts.Config["Outer.Inner.CheckerOne:Option"] = "StringValue";
+  EXPECT_TRUE("StringValue" == Opts.getCheckerStringOption(
+        "Outer.Inner.CheckerOne", "Option", "DefaultValue"));
+}
+
 } // end namespace ento
 } // end namespace clang