[Analyzer] Individual options for checkers #2

[xazax.hun]

Summary

This is a revised version of the original proposal that can be found here: http://reviews.llvm.org/D3967
The discussions on the mailing lists: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-October/039552.html

Changes since the original proposal:

• Option inheritance is now op-in
• Ability to query package options
• Updated to recent trunk
• Minor coding style fixes
• Replaced some std::string with StringRefs where appropriate

This patch may lack some regression tests but I wanted to get some input on the changes, documentation, API before writing those.

[zaks.anna]

I'd also have "Optimistic" be an option of malloc checker, not unix package since there are no other optimistic/pessimistic checkers in unix. The only reason to do that would be testing, but maybe we could test in other ways.

Also, It would be good to have this extra checking, but can come as port of a later commit:
"
To avoid ambiguities with regular options, we should enforce the following:

1. <Option Name> should be an identifier
2. Checker names should be identifiers.
3. Package names should be identifiers joined with '.’.
4. <Full Checker Name> has the same form as package names.

"

Thank you!
Anna.

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

191

We were trying to stay away from inheritance of options not to imply that the checkers will automatically inherit options. See the discussion on the proposal thread:
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-October/039576.html

291

I'd simplify this and have it take a book instead of the Kind.

[xazax.hun]

I agree that the Optimistic option for the malloc checker should not be inherited.

About the testing:
I think the most appropriate way to test this feature would be to write some unit tests for AnalyzerOptions class. However there are no unittests for the static analyzer yet. Should I create some as part of this patch?

About the validation:
Right now there is no way for a checker to specify the package or checker name when querying an option. The getCheckerOption function is private (the config table is public tough). The checkers can use getBooleanOption with a pointer to the checker, this means filling the package name and checker name is automatic. With this API a checker can not query the options of an unrelated package and there is no way to misspell the package or the checker name. Right now there is no validation for the option name. Should global option names for the static analyzer be validated as well?

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

191

I think the inheritance is explicit in this case, the checker have to specify it when querying the option. Should I change something here (e.g. remove the inherited versions or modify their semantics) or you only intended to link the discussion for other reviewers?

291

Could you elaborate on that?

[xazax.hun]

Changes since the last version:

• Added validation for the command line option parsing of checker options.
• The Optimistic option for the Malloc checker is no longer inherited.

[a.sidorin]

Thank you for your work Gabor. I'm sorry for the inconvenience but I was a little busy these days to continue this work. i didn't find any issues in this patch except a small nit.

[a.sidorin]

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

303

wether => whether :) And the same on the next line.

[zaks.anna]

Right now there is no way for a checker to specify the package or checker name when querying an option. The getCheckerOption function is private
(the config table is public tough). The checkers can use getBooleanOption with a pointer to the checker, this means filling the package name and >checker name is automatic. With this API a checker can not query the options of an unrelated package and there is no way to misspell the package or
the checker name.

The model I was going for was simpler. A checker would be allowed to query options that are set on itself or a given package. For that, we would want to expose the string based APIs to the checkers. However, this solution is good as well and does have a benefit of being less error prone. The only downside that I see with this solution is that a checker will not be able to search options of other checkers/packages. Maybe it's fine to disallow that. What do you think?

I would like to stay away from mentioning "inheritance" in these APIs. Here is a summary of the thread where this was discussed (on the "Static Analyzer Checker Options Proposal" thread):

Anna (initial proposal):
"However, there will be no inheritance (i.e. the setting 'unix:Optimistic' is entirely distinct from the setting ‘unix.Malloc:Optimistic’)."
Gabor Kozar:
"I think inheritance like that could be useful in some situations."
Anna:
"The main reason is that we currently do not have a need for it and allowing inheritance requires a design for it. For example, what happens when a package adds an option? How would the checkers access it? If we were to allow dynamically loaded checkers how would they inherit it? What happens if a checker overrides a package option?"
Gabor Kozar:
"I would expect such options to be inherited, i.e. if I set 'unix:Optimistic', then I expect this to be visible everywhere in the 'unix' package. Why are disallowing this?"
Anna:
"The package options will be visible to checkers; specifically, checkers could query the package options. For example, MallocChecker could call getOption(Optimistic, "unix") to check if the option has been set on the package. "
Gabor Kozar:
"How about the getOption() function getting a boolean parameter specifying whether to allow "inherited" options, with the default being true?

getOption("unix.stuff.moo.FooChecker", "MyOption", true) would then be equivalent to trying to all of these:
unix.stuff.moo.FooChecker:MyOption
unix.stuff.moo:MyOption
unix.stuff:MyOption
unix:MyOption
I think this behavior would be clear and straight-forward."
Anna:
"This looks good to me. I was mainly thinking/talking about implicit inheritance. Improving the way checkers can query options is a definite plus."

Given the above, the name "OptionKind" is confusing. (It implies that options have kinds, but we don't specify/enforce option kinds anywhere. We don't have a notion of an inherited option that will be automatically applied to all checkers in that package.) It's not an option kind but rather a search mode requested by the checker. It could be as easy as just a bool - search for the option in all parent packages or only search in this checker. In this patch, you allow many more ways of searching (OptionKind), which is fine if well documented and tested. However, if no one needs to all of these, implementing them could be an overkill at this point. It also complicated the API. We also need to document what happens when an option is overriden. The more kinds we have, the more complex the rule will be.

I think the most appropriate way to test this feature would be to write some unit tests for AnalyzerOptions class. However there are no
unittests for the static analyzer yet. Should I create some as part of this patch?

If you volunteer to do this, that would be awesome! There is no good way of testing these now.

[xazax.hun]

Changes since the last revision:

• Updated to current head
• Replaced option kind with a bool value
• Added a unit test

[xazax.hun]

In D7905#131369, @zaks.anna wrote:

The model I was going for was simpler. A checker would be allowed to query options that are set on itself or a given package. For that, we would want to expose the string based APIs to the checkers. However, this solution is good as well and does have a benefit of being less error prone. The only downside that I see with this solution is that a checker will not be able to search options of other checkers/packages. Maybe it's fine to disallow that. What do you think?

Right now the only way to query a package option is to query an option that is not specified for a given checker. I think that is not a problem, because that way every package options can be overridden for a specific checker. I think that the lack of API to query an option of a specific package is not a big limitation either. Checkers that need common knowledge of an option tend to be end up in the same package anyways. However, when such API is required, it can be added.

In D7905#131369, @zaks.anna wrote:

I would like to stay away from mentioning "inheritance" in these APIs.

Agreed.

[xazax.hun]

In D7905#131284, @a.sidorin wrote:

Thank you for your work Gabor. I'm sorry for the inconvenience but I was a little busy these days to continue this work. i didn't find any issues in this patch except a small nit.

No problem, thank you for the original patch :)

[zaks.anna]

Could you include more context with the patch? See: http://llvm.org/docs/Phabricator.html

Looks like we don't have a public API for options with string values.

Thanks!
Anna.

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

275

Extra "//".
Nit: Please, make sure all comments have proper punctuation. (A period is missing here and in a few other places.)

288

This is a bit confusing.

311

This might be more clear: "The optional checker parameter that can be used to restrict the search to the options of this particular checker."

lib/Frontend/CompilerInvocation.cpp

140

Shouldn't we use an existing clang check to check if substrings are identifiers?

lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

110

The comment needs to move and be rephrased (there is no inheritance and checkers are organized in packages, not categories).

unittests/Analysis/AnalyzerOptionsTest.cpp

47

I think we need more tests. Ex: overridden options, options set only in a package.

unittests/CMakeLists.txt

16

"Analysis" is too vague if we only plan to use this for the clang static analyzer.

[xazax.hun]

Thank you for the review.

Changes since last revision:

• Improved the comments as you recommended.
• Added a public function to query options as string values and appropriate test cases.
• Added some more test cases to package and checker options.
• Added more context to the diff.
• Updated to current head.

Regarding the options validation on the frontend:
Indeed, it would be nicer to use a clang method to check if a string is an identifier however I did not found a nice way to do that. The only way I came up with was to create a raw lexer and lex the option string. If you consider that better than using a RegEx I will alter the patch accordingly.

[zaks.anna]

Regarding the options validation on the frontend:
Indeed, it would be nicer to use a clang method to check if a string is an identifier however I did not found a >nice way to do that. The only way I came up with was to create a raw lexer and lex the option string. If you >consider that better than using a RegEx I will alter the patch accordingly.

It would also be good to check that the names of the checkers and packages are identifiers and that the option is a fully qualified checker name followed by the option name... Let's create a separate patch for validation and check if it can be done more elegantly.

The rest of the patch LGTM and can land. Thank you for working on this!

[xazax.hun]

Commited in r231266.

Thank you Aleksei for the original patch. Thank you Anna for your help and review.

include/clang/Basic/DiagnosticDriverKinds.td

   "analyzer-config option '%0' has a key but no value">;
 def err_analyzer_config_multiple_values : Error<
   "analyzer-config option '%0' should contain only one '='">;
+def err_analyzer_config_invalid_key : Error<
+  "analyzer-config key '%0' should have '(id.)*id:id' format">;
 
 def err_drv_modules_validate_once_requires_timestamp : Error<
   "option '-fmodules-validate-once-per-build-session' requires "

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

 class Preprocessor;
 class LangOptions;
 
+namespace ento {
+class CheckerBase;
+}
+
 /// Analysis - Set of available source code analyses.
 enum Analyses {
 #define ANALYSIS(NAME, CMDFLAG, DESC, SCOPE) NAME,

   /// \sa getMaxNodesPerTopLevelFunction
   Optional<unsigned> MaxNodesPerTopLevelFunction;
 
+  /// A helper function that retrieves option for a given full-qualified
+  /// checker name.
+  /// Options for checkers can be specified via 'analyzer-config' command-line
+  /// option.
+  /// Example:
+  /// @code-analyzer-config unix.Malloc:OptionName=CheckerOptionValue @endcode
+  /// or @code-analyzer-config unix:OptionName=GroupOptionValue @endcode
+  /// for groups of checkers
+  /// @param [in] CheckerName  Full-qualified checker name, like
+  /// alpha.unix.StreamChecker
+  /// @param [in] OptionName  Name of the option to get
+  /// @param [in] Default  Default value if no option is specified
+  /// @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.
+  ///// @retval CheckerOptionValue  An option for a checker if it was specified
+  /// @retval GroupOptionValue  An option for group if it was specified and no
+  /// checker-specific options were found. The closer group to checker,
+  /// the more priority it has. For example, @c coregroup.subgroup has more
+  /// priority than @c coregroup for @c coregroup.subgroup.CheckerName checker
+  /// @retval Default  If nor checker option, nor group option was found
+  StringRef getCheckerOption(StringRef CheckerName, StringRef OptionName,
+                             StringRef Default,
+                             bool SearchInParents = false);
+
 public:
   /// Interprets an option's string value as a boolean.
   ///
   /// Accepts the strings "true" and "false".
   /// If an option value is not provided, returns the given \p DefaultVal.
-  bool getBooleanOption(StringRef Name, bool DefaultVal);
+  /// @param [in] Name Name for option to retrieve
+  /// @param [in] DefaultVal Default value returned if no such option was
+  /// specified
+  /// @param [in] C Optional parameter that may be used to retrieve
+  /// checker-related option for a given checker
+  /// @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 getBooleanOption(StringRef Name, bool DefaultVal,
+                        const ento::CheckerBase *C = nullptr,
+                        bool SearchInParents = false);
 
   /// Variant that accepts a Optional value to cache the result.
-  bool getBooleanOption(Optional<bool> &V, StringRef Name, bool DefaultVal);
+  ///
+  /// @param [in,out] V Return value storage, returned if parameter contains
+  /// an existing valid option, else it is used to store a return value
+  /// @param [in] Name Name for option to retrieve
+  /// @param [in] DefaultVal Default value returned if no such option was
+  /// specified
+  /// @param [in] C Optional parameter that may be used to retrieve
+  /// checker-related option for a given checker
+  /// @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 getBooleanOption(Optional<bool> &V, StringRef Name, bool DefaultVal,
+                        const ento::CheckerBase *C  = nullptr,
+                        bool SearchInParents = false);
 
   /// Interprets an option's string value as an integer value.
-  int getOptionAsInteger(StringRef Name, int DefaultVal);
+  ///
+  /// If an option value is not provided, returns the given \p DefaultVal.
+  /// @param [in] Name Name for option to retrieve
+  /// @param [in] DefaultVal Default value returned if no such option was
+  /// specified
+  /// @param [in] C Optional parameter that may be used to retrieve
+  /// checker-related option for a given checker
+  /// @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 getOptionAsInteger(StringRef Name, int DefaultVal,
+                         const ento::CheckerBase *C = nullptr,
+                         bool SearchInParents = false);
 
   /// \brief 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/Frontend/CompilerInvocation.cpp

 #include "llvm/Support/Host.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/Regex.h"
 #include <atomic>
 #include <memory>
 #include <sys/stat.h>

   }
 }
 
+static bool IsValidCheckerOption(StringRef Key) {
+  llvm::Regex Re("^([_a-z][_a-z0-9]*\\.)*[_a-z][_a-z0-9]*:[_a-z][_a-z0-9]*$",
+                 llvm::Regex::IgnoreCase);
+  return Re.match(Key);
+}
+
 static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,
                               DiagnosticsEngine &Diags) {
   using namespace options;

         Success = false;
         break;
       }
+      // Validate checker and package specific arguments.
+      if (key.find(':') != StringRef::npos && !IsValidCheckerOption(key)) {
+        Diags.Report(SourceLocation(), diag::err_analyzer_config_invalid_key)
+            << key;
+      }
       Opts.Config[key] = val;
     }
   }

lib/StaticAnalyzer/Checkers/Checkers.td

   HelpText<"Check calls to various UNIX/Posix functions">,
   DescFile<"UnixAPIChecker.cpp">;
 
-def MallocPessimistic : Checker<"Malloc">,
+def MallocChecker: Checker<"Malloc">,
   HelpText<"Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free().">,
   DescFile<"MallocChecker.cpp">;
   

   HelpText<"Check improper use of chroot">,
   DescFile<"ChrootChecker.cpp">;
 
-def MallocOptimistic : Checker<"MallocWithAnnotations">,
-  HelpText<"Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free(). Assumes that all user-defined functions which might free a pointer are annotated.">,
-  DescFile<"MallocChecker.cpp">;
-
 def PthreadLockChecker : Checker<"PthreadLock">,
   HelpText<"Simple lock -> unlock checker">,
   DescFile<"PthreadLockChecker.cpp">;

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
   enum CheckKind {
-    CK_MallocPessimistic,
-    CK_MallocOptimistic,
+    CK_MallocChecker,
     CK_NewDeleteChecker,
     CK_NewDeleteLeaksChecker,
     CK_MismatchedDeallocatorChecker,

     MOK_Any
   };
 
+  DefaultBool IsOptimistic;
+
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
   CheckName CheckNames[CK_NumCheckKinds];
   typedef llvm::SmallVector<CheckKind, CK_NumCheckKinds> CKVecTy;

   if (Family != AF_Malloc)
     return false;
 
-  if (ChecksEnabled[CK_MallocOptimistic] && FD->hasAttrs()) {
+  if (IsOptimistic && FD->hasAttrs()) {
     for (const auto *I : FD->specific_attrs<OwnershipAttr>()) {
       OwnershipAttr::OwnershipKind OwnKind = I->getOwnKind();
       if(OwnKind == OwnershipAttr::Takes || OwnKind == OwnershipAttr::Holds) {

     }
   }
 
-  if (ChecksEnabled[CK_MallocOptimistic] ||
-      ChecksEnabled[CK_MismatchedDeallocatorChecker]) {
+  if (IsOptimistic || ChecksEnabled[CK_MismatchedDeallocatorChecker]) {
     // Check all the attributes, if there are any.
     // There can be multiple of these attributes.
     if (FD->hasAttrs())

   case AF_IfNameIndex:
   case AF_Alloca: {
     // C checkers.
-    if (CK == CK_MallocOptimistic ||
-        CK == CK_MallocPessimistic) {
+    if (CK == CK_MallocChecker) {
       return CK;
     }
     return Optional<MallocChecker::CheckKind>();

                                   SourceRange Range, 
                                   const Expr *DeallocExpr) const {
 
-  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocOptimistic,
-                                                   CK_MallocPessimistic,
+  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocChecker,
                                                    CK_NewDeleteChecker),
                                      C, DeallocExpr);
   if (!CheckKind.hasValue())

 void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal, 
                                      SourceRange Range) const {
 
-  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocOptimistic,
-                                               CK_MallocPessimistic,
+  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocChecker,
                                                CK_MismatchedDeallocatorChecker),
                                      AF_Alloca);
   if (!CheckKind.hasValue())

                                      const Expr *AllocExpr) const {
 
 
-  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocOptimistic,
-                                                   CK_MallocPessimistic,
+  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocChecker,
                                                    CK_NewDeleteChecker),
                                      C, AllocExpr);
   if (!CheckKind.hasValue())

 void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range,
                                        SymbolRef Sym) const {
 
-  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocOptimistic,
-                                                   CK_MallocPessimistic,
+  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocChecker,
                                                    CK_NewDeleteChecker),
                                      C, Sym);
   if (!CheckKind.hasValue())

                                      bool Released, SymbolRef Sym, 
                                      SymbolRef PrevSym) const {
 
-  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocOptimistic,
-                                                   CK_MallocPessimistic,
+  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocChecker,
                                                    CK_NewDeleteChecker),
                                      C, Sym);
   if (!CheckKind.hasValue())

 void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N,
                                CheckerContext &C) const {
 
-  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocOptimistic,
-                                                   CK_MallocPessimistic,
+  auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocChecker,
                                                    CK_NewDeleteLeaksChecker),
                                      C, Sym);
   if (!CheckKind.hasValue())

       return;
 
     ASTContext &Ctx = C.getASTContext();
-    if ((ChecksEnabled[CK_MallocOptimistic] ||
-         ChecksEnabled[CK_MallocPessimistic]) &&
+    if (ChecksEnabled[CK_MallocChecker] &&
         (isCMemFunction(FD, Ctx, AF_Malloc, MemoryOperationKind::MOK_Free) ||
          isCMemFunction(FD, Ctx, AF_IfNameIndex,
                         MemoryOperationKind::MOK_Free)))

     for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) {
       const RefState *RefS = State->get<RegionState>(I.getKey());
       AllocationFamily Family = RefS->getAllocationFamily();
-      auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocOptimistic,
-                                                       CK_MallocPessimistic,
+      auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocChecker,
                                                        CK_NewDeleteChecker),
                                          Family);
       I.getKey()->dumpToStream(Out);

 void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) {
   registerCStringCheckerBasic(mgr);
   MallocChecker *checker = mgr.registerChecker<MallocChecker>();
+  checker->IsOptimistic = mgr.getAnalyzerOptions().getBooleanOption(
+      "Optimistic", false, checker);
   checker->ChecksEnabled[MallocChecker::CK_NewDeleteLeaksChecker] = true;
   checker->CheckNames[MallocChecker::CK_NewDeleteLeaksChecker] =
       mgr.getCurrentCheckName();

   void ento::register##name(CheckerManager &mgr) {                             \
     registerCStringCheckerBasic(mgr);                                          \
     MallocChecker *checker = mgr.registerChecker<MallocChecker>();             \
+    checker->IsOptimistic = mgr.getAnalyzerOptions().getBooleanOption(         \
+        "Optimistic", false, checker);                                         \
     checker->ChecksEnabled[MallocChecker::CK_##name] = true;                   \
     checker->CheckNames[MallocChecker::CK_##name] = mgr.getCurrentCheckName(); \
   }
 
-REGISTER_CHECKER(MallocPessimistic)
-REGISTER_CHECKER(MallocOptimistic)
+REGISTER_CHECKER(MallocChecker)
 REGISTER_CHECKER(NewDeleteChecker)
 REGISTER_CHECKER(MismatchedDeallocatorChecker)

lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ErrorHandling.h"

 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
+using namespace ento;
 using namespace llvm;
 
 AnalyzerOptions::UserModeKind AnalyzerOptions::getUserMode() {

 
 static StringRef toString(bool b) { return b ? "true" : "false"; }
 
-bool AnalyzerOptions::getBooleanOption(StringRef Name, bool DefaultVal) {
+StringRef AnalyzerOptions::getCheckerOption(StringRef CheckerName,
+                                            StringRef OptionName,
+                                            StringRef Default,
+                                            bool SearchInParents) {
+  // Search for a category option if option for checker is not specified and
+  // inheritance is enabled.
+  ConfigTable::const_iterator E = Config.end();
+  do {
+    ConfigTable::const_iterator I =
+        Config.find((Twine(CheckerName) + ":" + OptionName).str());
+    if (I != E)
+      return StringRef(I->getValue());
+    size_t Pos = CheckerName.rfind('.');
+    if (Pos == StringRef::npos)
+      return Default;
+    CheckerName = CheckerName.substr(0, Pos);
+  } while (!CheckerName.empty() && SearchInParents);
+  return Default;
+}
+
+bool AnalyzerOptions::getBooleanOption(StringRef Name, bool DefaultVal,
+                                       const CheckerBase *C,
+                                       bool SearchInParents) {
   // 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.
+  StringRef Default = toString(DefaultVal);
   StringRef V =
-      Config.insert(std::make_pair(Name, toString(DefaultVal))).first->second;
+      C ? getCheckerOption(C->getTagDescription(), Name, Default,
+                           SearchInParents)
+        : StringRef(Config.insert(std::make_pair(Name, Default)).first->second);
   return llvm::StringSwitch<bool>(V)
       .Case("true", true)
       .Case("false", false)

 }
 
 bool AnalyzerOptions::getBooleanOption(Optional<bool> &V, StringRef Name,
-                                       bool DefaultVal) {
+                                       bool DefaultVal, const CheckerBase *C,
+                                       bool SearchInParents) {
   if (!V.hasValue())
-    V = getBooleanOption(Name, DefaultVal);
+    V = getBooleanOption(Name, DefaultVal, C, SearchInParents);
   return V.getValue();
 }
 

                           /* Default = */ false);
 }
 
-int AnalyzerOptions::getOptionAsInteger(StringRef Name, int DefaultVal) {
+int AnalyzerOptions::getOptionAsInteger(StringRef Name, int DefaultVal,
+                                        const CheckerBase *C,
+                                        bool SearchInParents) {
   SmallString<10> StrBuf;
   llvm::raw_svector_ostream OS(StrBuf);
   OS << DefaultVal;
 
-  StringRef V = Config.insert(std::make_pair(Name, OS.str())).first->second;
+  StringRef V = C ? getCheckerOption(C->getTagDescription(), Name, OS.str(),
+                                     SearchInParents)
+                  : StringRef(Config.insert(std::make_pair(Name, OS.str()))
+                                  .first->second);
+
   int Res = DefaultVal;
   bool b = V.getAsInteger(10, Res);
   assert(!b && "analyzer-config option should be numeric");
-  (void) b;
+  (void)b;
   return Res;
 }
 

 bool AnalyzerOptions::shouldConditionalizeStaticInitializers() {
   return getBooleanOption("cfg-conditional-static-initializers", true);
 }
-

test/Analysis/free.c

 // RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-checker=core,unix.Malloc -fblocks -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-checker=core,alpha.unix.MallocWithAnnotations -fblocks -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-checker=core,unix.Malloc -fblocks -verify -analyzer-config unix.Malloc:Optimistic=true %s
 typedef __typeof(sizeof(int)) size_t;
 void free(void *);
 void *alloca(size_t);

test/Analysis/malloc-annotations.c

-// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,alpha.unix.MallocWithAnnotations -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc -analyzer-store=region -verify -analyzer-config unix.Malloc:Optimistic=true %s
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void free(void *);

test/Analysis/outofbound.c

-// RUN: %clang_cc1 -Wno-array-bounds -analyze -analyzer-checker=core,alpha.unix,alpha.security.ArrayBound -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -Wno-array-bounds -analyze -analyzer-checker=core,unix,alpha.security.ArrayBound -analyzer-store=region -verify -analyzer-config unix:Optimistic=true %s
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);

test/Analysis/undef-buffers.c

-// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix,core.uninitialized -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix,core.uninitialized -analyzer-store=region -verify -analyzer-config unix:Optimistic=true %s
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void free(void *);

unittests/Analysis/AnalyzerOptionsTest.cpp

+//===- unittest/Analysis/AnalyzerOptionsTest.cpp - SA Options test --------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+
+TEST(StaticAnalyzerOptions, SearchInParentPackageTests) {
+  AnalyzerOptions Opts;
+  Opts.Config["Outer.Inner.CheckerOne:Option"] = "true";
+  Opts.Config["Outer.Inner:Option"] = "false";
+
+  struct CheckerOneMock : CheckerBase {
+    StringRef getTagDescription() const override {
+      return "Outer.Inner.CheckerOne";
+    }
+  };
+  struct CheckerTwoMock : CheckerBase {
+    StringRef getTagDescription() const override {
+      return "Outer.Inner.CheckerTwo";
+    }
+  };
+
+  // Checker one has Option specified as true. It should read true regardless of
+  // search mode.
+  CheckerOneMock CheckerOne;
+  EXPECT_TRUE(Opts.getBooleanOption("Option", false, &CheckerOne));
+  EXPECT_TRUE(Opts.getBooleanOption("Option", false, &CheckerOne, 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.getBooleanOption("Option", false, &CheckerTwo));
+  EXPECT_TRUE(Opts.getBooleanOption("Option", true, &CheckerTwo));
+  EXPECT_FALSE(Opts.getBooleanOption("Option", true, &CheckerTwo, true));
+}
+} // end namespace ast_matchers
+} // end namespace clang

unittests/Analysis/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_unittest(StaticAnalysisTests
+  AnalyzerOptionsTest.cpp
+  )
+
+target_link_libraries(StaticAnalysisTests
+  clangBasic
+  clangAnalysis
+  clangStaticAnalyzerCore 
+  )

unittests/Analysis/Makefile

+##===- unittests/Basic/Makefile ----------------------------*- Makefile -*-===##
+#
+#                     The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+##===----------------------------------------------------------------------===##
+
+CLANG_LEVEL = ../..
+TESTNAME = StaticAnalysis
+LINK_COMPONENTS := support mc
+USEDLIBS = clangBasic.a clangAnalysis.a clangStaticAnalyzerCore.a
+
+include $(CLANG_LEVEL)/unittests/Makefile

unittests/CMakeLists.txt

 add_subdirectory(Lex)
 add_subdirectory(Driver)
 if(CLANG_ENABLE_STATIC_ANALYZER)
+  add_subdirectory(Analysis)
   add_subdirectory(Frontend)
 endif()
 add_subdirectory(ASTMatchers)

unittests/Makefile

 include $(CLANG_LEVEL)/../..//Makefile.config
 
 ifeq ($(ENABLE_CLANG_ARCMT),1)
-PARALLEL_DIRS += Frontend libclang
+PARALLEL_DIRS += Frontend libclang Analysis
 endif
 
 endif  # CLANG_LEVEL