Skip to content

Commit

Permalink
[analyzer] Reimplement dependencies between checkers
Browse files Browse the repository at this point in the history
Unfortunately, up until now, the fact that certain checkers depended on one
another was known, but how these actually unfolded was hidden deep within the
implementation. For example, many checkers (like RetainCount, Malloc or CString)
modelled a certain functionality, and exposed certain reportable bug types to
the user. For example, while MallocChecker models many many different types of
memory handling, the actual "unix.MallocChecker" checker the user was exposed to
was merely and option to this modeling part.

Other than this being an ugly mess, this issue made resolving the checker naming
issue almost impossible. (The checker naming issue being that if a checker
registered more than one checker within its registry function, both checker
object recieved the same name) Also, if the user explicitly disabled a checker
that was a dependency of another that _was_ explicitly enabled, it implicitly,
without "telling" the user, reenabled it.

Clearly, changing this to a well structured, declarative form, where the
handling of dependencies are done on a higher level is very much preferred.

This patch, among the detailed things later, makes checkers declare their
dependencies within the TableGen file Checkers.td, and exposes the same
functionality to plugins and statically linked non-generated checkers through
CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies,
makes sure that checkers are added to CheckerManager in the correct order,
and makes sure that if a dependency is disabled, so will be every checker that
depends on it.

In detail:

* Add a new field to the Checker class in CheckerBase.td called Dependencies,
which is a list of Checkers.
* Move unix checkers before cplusplus, as there is no forward declaration in
tblgen :/
* Add the following new checkers:
  - StackAddrEscapeBase
  - StackAddrEscapeBase
  - CStringModeling
  - DynamicMemoryModeling (base of the MallocChecker family)
  - IteratorModeling (base of the IteratorChecker family)
  - ValistBase
  - SecuritySyntaxChecker (base of bcmp, bcopy, etc...)
  - NSOrCFErrorDerefChecker (base of NSErrorChecker and  CFErrorChecker)
  - IvarInvalidationModeling (base of IvarInvalidation checker family)
  - RetainCountBase (base of RetainCount and OSObjectRetainCount)
* Clear up and registry functions in MallocChecker, happily remove old FIXMEs.
* Add a new addDependency function to CheckerRegistry.
* Neatly format RUN lines in files I looked at while debugging.

Big thanks to Artem Degrachev for all the guidance through this project!

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

llvm-svn: 352287
  • Loading branch information
Szelethus committed Jan 26, 2019
1 parent be03018 commit 8fd74eb
Show file tree
Hide file tree
Showing 31 changed files with 1,048 additions and 700 deletions.
23 changes: 19 additions & 4 deletions clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
Expand Up @@ -48,9 +48,24 @@ class Documentation<DocumentationEnum val> {
/// Note that a checker has a name (e.g.: 'NullDereference'), and a fullname,
/// that is autogenerated with the help of the ParentPackage field, that also
/// includes package names (e.g.: 'core.NullDereference').
/// Example:
/// def DereferenceChecker : Checker<"NullDereference">,
/// HelpText<"Check for dereferences of null pointers">;
class Checker<string name = ""> {
string CheckerName = name;
string HelpText;
bits<2> Documentation;
Package ParentPackage;
string CheckerName = name;
string HelpText;
list<Checker> Dependencies;
bits<2> Documentation;
Package ParentPackage;
}

/// Describes dependencies in between checkers. For example, InnerPointerChecker
/// relies on information MallocBase gathers.
/// Example:
/// def InnerPointerChecker : Checker<"InnerPointer">,
/// HelpText<"Check for inner pointers of C++ containers used after "
/// "re/deallocation">,
/// Dependencies<[MallocBase]>;
class Dependencies<list<Checker> Deps = []> {
list<Checker> Dependencies = Deps;
}

0 comments on commit 8fd74eb

Please sign in to comment.