[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 ↗

(On Diff #20746)

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 ↗

(On Diff #20746)

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 ↗

(On Diff #20746)

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 ↗

(On Diff #20746)

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 ↗

(On Diff #20841)

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 ↗

(On Diff #20998)

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

288 ↗

(On Diff #20998)

This is a bit confusing.

311 ↗

(On Diff #20998)

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 ↗

(On Diff #20998)

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

lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

110 ↗

(On Diff #20998)

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 ↗

(On Diff #20998)

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

unittests/CMakeLists.txt

16 ↗

(On Diff #20998)

"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.

cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

 #include <vector>
 
 namespace clang {
 class ASTConsumer;
 class DiagnosticsEngine;
 class Preprocessor;
 class LangOptions;
 
+namespace ento {
+class CheckerBase;
+}
+
 /// Analysis - Set of available source code analyses.
 enum Analyses {
 #define ANALYSIS(NAME, CMDFLAG, DESC, SCOPE) NAME,
 #include "clang/StaticAnalyzer/Core/Analyses.def"
 NumAnalyses
 };
 
 /// AnalysisStores - Set of available analysis store models.
private:
   Optional<unsigned> GraphTrimInterval;
 
   /// \sa getMaxTimesInlineLarge
   Optional<unsigned> MaxTimesInlineLarge;
 
   /// \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.
+  /// Interprets an option's string value as a boolean. The "true" string is
+  /// interpreted as true and the "false" string is interpreted as false.
   ///
-  /// 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 The optional checker parameter that can be used to restrict
+  /// the search to the options of this particular checker (and its parents
+  /// dependening on search mode).
+  /// @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 The optional checker parameter that can be used to restrict
+  /// the search to the options of this particular checker (and its parents
+  /// dependening on search mode).
+  /// @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 The optional checker parameter that can be used to restrict
+  /// the search to the options of this particular checker (and its parents
+  /// dependening on search mode).
+  /// @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);
+
+  /// 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] DefaultVal Default value returned if no such option was
+  /// specified.
+  /// @param [in] C The optional checker parameter that can be used to restrict
+  /// the search to the options of this particular checker (and its parents
+  /// dependening on search mode).
+  /// @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 getOptionAsString(StringRef Name, StringRef 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
   /// outside of AnalyzerOptions.
   UserModeKind getUserMode();
 
   /// \brief Returns the inter-procedural analysis mode.
   IPAKind getIPAMode();

cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td

 //===----------------------------------------------------------------------===//
 
 let ParentPackage = Unix in {
 
 def UnixAPIChecker : Checker<"API">,
   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">;
   
 def MallocSizeofChecker : Checker<"MallocSizeof">,
   HelpText<"Check for dubious malloc arguments involving sizeof">,
   DescFile<"MallocSizeofChecker.cpp">;
 
 def MismatchedDeallocatorChecker : Checker<"MismatchedDeallocator">,
   HelpText<"Check for mismatched deallocators.">,
   DescFile<"MallocChecker.cpp">;
   
 } // end "unix"
 
 let ParentPackage = UnixAlpha in {
 
 def ChrootChecker : Checker<"Chroot">,
   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">;
 
 def StreamChecker : Checker<"Stream">,
   HelpText<"Check stream handling functions">,
   DescFile<"StreamChecker.cpp">;
 

cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

MallocChecker()
         II_realloc(nullptr), II_calloc(nullptr), II_valloc(nullptr),
         II_reallocf(nullptr), II_strndup(nullptr), II_strdup(nullptr), 
         II_kmalloc(nullptr), II_if_nameindex(nullptr),
         II_if_freenameindex(nullptr) {}
 
   /// 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,
     CK_NumCheckKinds
   };
 
   enum class MemoryOperationKind { 
     MOK_Allocate,
     MOK_Free,
     MOK_Any
   };
 
+  DefaultBool IsOptimistic;
+
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
   CheckName CheckNames[CK_NumCheckKinds];
   typedef llvm::SmallVector<CheckKind, CK_NumCheckKinds> CKVecTy;
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
   void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
   void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
if (Family == AF_Alloca && CheckAlloc) {
       if (FunI == II_alloca)
         return true;
     }
   }
 
   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 (CheckFree)
           return true;
       } else if (OwnKind == OwnershipAttr::Returns) {
         if (CheckAlloc)
           return true;
if (FunI == II_malloc) {
       // elements with zeros in the last one.
       State = MallocMemAux(C, CE, UnknownVal(), UnknownVal(), State,
                            AF_IfNameIndex);
     } else if (FunI == II_if_freenameindex) {
       State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
     }
   }
 
-  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())
       for (const auto *I : FD->specific_attrs<OwnershipAttr>()) {
         switch (I->getOwnKind()) {
         case OwnershipAttr::Returns:
           State = MallocMemReturnsAttr(C, CE, I, State);
           break;
MallocChecker::getCheckIfTracked(MallocChecker::CheckKind CK,
   if (CK == CK_MismatchedDeallocatorChecker)
     return CK;
 
   switch (Family) {
   case AF_Malloc:
   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>();
   }
   case AF_CXXNew:
   case AF_CXXNewArray: {
     // C++ checkers.
     if (CK == CK_NewDeleteChecker ||
bool MallocChecker::SummarizeRegion(raw_ostream &os,
   }
   }
 }
 
 void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, 
                                   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())
     return;
 
   if (ExplodedNode *N = C.generateSink()) {
     if (!BT_BadFree[*CheckKind])
       BT_BadFree[*CheckKind].reset(
if (ExplodedNode *N = C.generateSink()) {
     R->addRange(Range);
     C.emitReport(R);
   }
 }
 
 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())
     return;
 
   if (ExplodedNode *N = C.generateSink()) {
     if (!BT_FreeAlloca[*CheckKind])
       BT_FreeAlloca[*CheckKind].reset(
void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
   }
 }
 
 void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
                                      SourceRange Range, const Expr *DeallocExpr,
                                      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())
     return;
 
   ExplodedNode *N = C.generateSink();
   if (!N)
     return;
void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
   R->markInteresting(MR->getBaseRegion());
   R->addRange(Range);
   C.emitReport(R);
 }
 
 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())
     return;
 
   if (ExplodedNode *N = C.generateSink()) {
     if (!BT_UseFree[*CheckKind])
       BT_UseFree[*CheckKind].reset(new BugType(
           CheckNames[*CheckKind], "Use-after-free", "Memory Error"));
 
     BugReport *R = new BugReport(*BT_UseFree[*CheckKind],
                                  "Use of memory after it is freed", N);
 
     R->markInteresting(Sym);
     R->addRange(Range);
     R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym));
     C.emitReport(R);
   }
 }
 
 void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range,
                                      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())
     return;
 
   if (ExplodedNode *N = C.generateSink()) {
     if (!BT_DoubleFree[*CheckKind])
       BT_DoubleFree[*CheckKind].reset(
MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
   }
 
   return LeakInfo(AllocNode, ReferenceRegion);
 }
 
 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;
 
   assert(N);
   if (!BT_Leak[*CheckKind]) {
     BT_Leak[*CheckKind].reset(
void MallocChecker::checkPreCall(const CallEvent &Call,
 
   // We will check for double free in the post visit.
   if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) {
     const FunctionDecl *FD = FC->getDecl();
     if (!FD)
       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)))
       return;
 
     if (ChecksEnabled[CK_NewDeleteChecker] &&
         isStandardNewDelete(FD, Ctx))
       return;
void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State,
 
   RegionStateTy RS = State->get<RegionState>();
 
   if (!RS.isEmpty()) {
     Out << Sep << "MallocChecker :" << NL;
     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);
       Out << " : ";
       I.getData().dump(Out);
       if (CheckKind.hasValue())
         Out << " (" << CheckNames[*CheckKind].getName() << ")";
       Out << NL;
     }
   }
 }
 
 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();
   // We currently treat NewDeleteLeaks checker as a subchecker of NewDelete 
   // checker.
   if (!checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker])
     checker->ChecksEnabled[MallocChecker::CK_NewDeleteChecker] = true;
 }
 
 #define REGISTER_CHECKER(name)                                                 \
   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)

cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

 //===-- AnalyzerOptions.cpp - Analysis Engine Options -----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains special accessors for analyzer configuration options
 // with string representations.
 //
 //===----------------------------------------------------------------------===//
 
 #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() {
   if (UserMode == UMK_NotSet) {
     StringRef ModeStr =
         Config.insert(std::make_pair("mode", "deep")).first->second;
     UserMode = llvm::StringSwitch<UserModeKind>(ModeStr)
       .Case("shallow", UMK_Shallow)
if (!CXXMemberInliningMode) {
     }
   }
 
   return CXXMemberInliningMode >= K;
 }
 
 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 package option if the option for the checker is not specified
+  // and search in parents 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)
       .Default(DefaultVal);
 }
 
 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();
 }
 
 bool AnalyzerOptions::includeTemporaryDtorsInCFG() {
   return getBooleanOption(IncludeTemporaryDtorsInCFG,
                           "cfg-temporary-dtors",
                           /* Default = */ false);
 }

 
 
 bool AnalyzerOptions::shouldWriteStableReportFilename() {
   return getBooleanOption(StableReportFilename,
                           "stable-report-filename",
                           /* 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;
 }
 
+StringRef AnalyzerOptions::getOptionAsString(StringRef Name,
+                                             StringRef DefaultVal,
+                                             const CheckerBase *C,
+                                             bool SearchInParents) {
+  return C ? getCheckerOption(C->getTagDescription(), Name, DefaultVal,
+                              SearchInParents)
+           : StringRef(
+                 Config.insert(std::make_pair(Name, DefaultVal)).first->second);
+}
+
 unsigned AnalyzerOptions::getAlwaysInlineSize() {
   if (!AlwaysInlineSize.hasValue())
     AlwaysInlineSize = getOptionAsInteger("ipa-always-inline-size", 3);
   return AlwaysInlineSize.getValue();
 }
 
 unsigned AnalyzerOptions::getMaxInlinableSize() {
   if (!MaxInlinableSize.hasValue()) {

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

cfe/trunk/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);
 
 void t1 () {
   int a[] = { 1 };
   free(a); // expected-warning {{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}}
 }

cfe/trunk/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 *);
 void *realloc(void *ptr, size_t size);
 void *calloc(size_t nmemb, size_t size);
 void __attribute((ownership_returns(malloc))) *my_malloc(size_t);
 void __attribute((ownership_takes(malloc, 1))) my_free(void *);
 void my_freeBoth(void *, void *)

cfe/trunk/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);
 void *calloc(size_t, size_t);
 
 char f1() {
   char* s = "abcd";
   char c = s[4]; // no-warning

cfe/trunk/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 *);
 
 char stackBased1 () {
   char buf[2];
   buf[0] = 'a';
   return buf[1]; // expected-warning{{Undefined}}

cfe/trunk/unittests/CMakeLists.txt

 add_custom_target(ClangUnitTests)
 set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang tests")
 
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang
 # Produces a binary named 'basename(test_dirname)'.
 function(add_clang_unittest test_dirname)
   add_unittest(ClangUnitTests ${test_dirname} ${ARGN})
 endfunction()
 
 add_subdirectory(Basic)
 add_subdirectory(Lex)
 add_subdirectory(Driver)
 if(CLANG_ENABLE_STATIC_ANALYZER)
+  add_subdirectory(StaticAnalyzer)
   add_subdirectory(Frontend)
 endif()
 add_subdirectory(ASTMatchers)
 add_subdirectory(AST)
 add_subdirectory(Tooling)
 add_subdirectory(Format)
 add_subdirectory(Sema)
 add_subdirectory(CodeGen)
 # FIXME: Why are the libclang unit tests disabled on Windows?
 if(NOT WIN32) 
   add_subdirectory(libclang)
 endif()

cfe/trunk/unittests/Makefile

 IS_UNITTEST_LEVEL := 1
 CLANG_LEVEL := ..
 PARALLEL_DIRS = CodeGen Basic Lex Driver Format ASTMatchers AST Tooling \
           Sema
 
 include $(CLANG_LEVEL)/../..//Makefile.config
 
 ifeq ($(ENABLE_CLANG_ARCMT),1)
-PARALLEL_DIRS += Frontend libclang
+PARALLEL_DIRS += Frontend libclang StaticAnalyzer
 endif
 
 endif  # CLANG_LEVEL
 
 include $(CLANG_LEVEL)/Makefile
 
 ifndef IS_UNITTEST_LEVEL
 
 MAKEFILE_UNITTEST_NO_INCLUDE_COMMON := 1
 include $(LLVM_SRC_ROOT)/unittests/Makefile.unittest
 
 endif  # IS_UNITTEST_LEVEL

cfe/trunk/unittests/StaticAnalyzer/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";
+  Opts.Config["Outer.Inner:Option2"] = "true";
+  Opts.Config["Outer:Option2"] = "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));
+  // The package option is overriden with a checker option.
+  EXPECT_TRUE(Opts.getBooleanOption("Option", false, &CheckerOne, true));
+  // The Outer package option is overriden by the Inner package option. No
+  // package option is specified.
+  EXPECT_TRUE(Opts.getBooleanOption("Option2", false, &CheckerOne, true));
+  // No package option is specified and search in packages is turned off. The
+  // default value should be returned.
+  EXPECT_FALSE(Opts.getBooleanOption("Option2", false, &CheckerOne));
+  EXPECT_TRUE(Opts.getBooleanOption("Option2", true, &CheckerOne));
+
+  // 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));
+}
+
+TEST(StaticAnalyzerOptions, StringOptions) {
+  AnalyzerOptions Opts;
+  Opts.Config["Outer.Inner.CheckerOne:Option"] = "StringValue";
+
+  struct CheckerOneMock : CheckerBase {
+    StringRef getTagDescription() const override {
+      return "Outer.Inner.CheckerOne";
+    }
+  };
+
+  CheckerOneMock CheckerOne;
+  EXPECT_TRUE("StringValue" ==
+              Opts.getOptionAsString("Option", "DefaultValue", &CheckerOne));
+  EXPECT_TRUE("DefaultValue" ==
+              Opts.getOptionAsString("Option2", "DefaultValue", &CheckerOne));
+}
+} // end namespace ento
+} // end namespace clang

cfe/trunk/unittests/StaticAnalyzer/CMakeLists.txt

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

cfe/trunk/unittests/StaticAnalyzer/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