[analyzer] Clean checker options from bool to DefaultBool (NFC)

A recent review emphasized the preference to use DefaultBool instead of
bool for checker options. This change is a NFC and cleans up s

[analyzer] Clean checker options from bool to DefaultBool (NFC)

A recent review emphasized the preference to use DefaultBool instead of
bool for checker options. This change is a NFC and cleans up some of the
instances where bool was used, and could be changed to DefaultBool.

Reviewed By: steakhal

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

show more ...

[analyzer] Catch leaking stack addresses via stack variables

Not only global variables can hold references to dead stack variables.
Consider this example:

void write_stack_address_to(char **q) {

[analyzer] Catch leaking stack addresses via stack variables

Not only global variables can hold references to dead stack variables.
Consider this example:

void write_stack_address_to(char **q) {
char local;
*q = &local;
}

void test_stack() {
char *p;
write_stack_address_to(&p);
}

The address of 'local' is assigned to 'p', which becomes a dangling
pointer after 'write_stack_address_to()' returns.

The StackAddrEscapeChecker was looking for bindings in the store which
referred to variables of the popped stack frame, but it only considered
global variables in this regard. This patch relaxes this, catching
stack variable bindings as well.

---

This patch also works for temporary objects like:

struct Bar {
const int &ref;
explicit Bar(int y) : ref(y) {
// Okay.
} // End of the constructor call, `ref` is dangling now. Warning!
};

void test() {
Bar{33}; // Temporary object, so the corresponding memregion is
// *not* a VarRegion.
}

---

The return value optimization aka. copy-elision might kick in but that
is modeled by passing an imaginary CXXThisRegion which refers to the
parent stack frame which is supposed to be the 'return slot'.
Objects residing in the 'return slot' outlive the scope of the inner
call, thus we should expect no warning about them - except if we
explicitly disable copy-elision.

Reviewed By: NoQ, martong

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

show more ...

[analyzer][StackAddressEscape] Tie warnings to the diagnostic checkers rather then core.StackAddrEscapeBase

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

[analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions

Some checkers may not only depend on language options but also analyzer options.
To make this possible this patc

[analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions

Some checkers may not only depend on language options but also analyzer options.
To make this possible this patch changes the parameter of the shouldRegister*
function to CheckerManager to be able to query the analyzer options when
deciding whether the checker should be registered.

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

show more ...

[analyzer] NFC: Introduce sub-classes for path-sensitive and basic reports.

Checkers are now required to specify whether they're creating a
path-sensitive report or a path-insensitive report by cons

[analyzer] NFC: Introduce sub-classes for path-sensitive and basic reports.

Checkers are now required to specify whether they're creating a
path-sensitive report or a path-insensitive report by constructing an
object of the respective type.

This makes BugReporter more independent from the rest of the Static Analyzer
because all Analyzer-specific code is now in sub-classes.

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

llvm-svn: 371450

show more ...

[Clang] Migrate llvm::make_unique to std::make_unique

Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
o

[Clang] Migrate llvm::make_unique to std::make_unique

Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.

Differential revision: https://reviews.llvm.org/D66259

llvm-svn: 368942

show more ...

[analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

This patch effectively fixes the almost decade old checker naming issue.
The solution i

[analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

This patch effectively fixes the almost decade old checker naming issue.
The solution is to assert when CheckerManager::getChecker is called on an
unregistered checker, and assert when CheckerManager::registerChecker is called
on a checker that is already registered.

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

llvm-svn: 352292

show more ...

[analyzer] Reimplement dependencies between checkers

Unfortunately, up until now, the fact that certain checkers depended on one
another was known, but how these actually unfolded was hidden deep wi

[analyzer] Reimplement dependencies between checkers

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

show more ...

[analyzer] Supply all checkers with a shouldRegister function

Introduce the boolean ento::shouldRegister##CHECKERNAME(const LangOptions &LO)
function very similarly to ento::register##CHECKERNAME. T

[analyzer] Supply all checkers with a shouldRegister function

Introduce the boolean ento::shouldRegister##CHECKERNAME(const LangOptions &LO)
function very similarly to ento::register##CHECKERNAME. This will force every
checker to implement this function, but maybe it isn't that bad: I saw a lot of
ObjC or C++ specific checkers that should probably not register themselves based
on some LangOptions (mine too), but they do anyways.

A big benefit of this is that all registry functions now register their checker,
once it is called, registration is guaranteed.

This patch is a part of a greater effort to reinvent checker registration, more
info here: D54438#1315953

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

llvm-svn: 352277

show more ...

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the ne

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.

Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.

llvm-svn: 351636

show more ...

[analyzer][NFC] Move CheckerRegistry from the Core directory to Frontend

ClangCheckerRegistry is a very non-obvious, poorly documented, weird concept.
It derives from CheckerRegistry, and is placed

[analyzer][NFC] Move CheckerRegistry from the Core directory to Frontend

ClangCheckerRegistry is a very non-obvious, poorly documented, weird concept.
It derives from CheckerRegistry, and is placed in lib/StaticAnalyzer/Frontend,
whereas it's base is located in lib/StaticAnalyzer/Core. It was, from what I can
imagine, used to circumvent the problem that the registry functions of the
checkers are located in the clangStaticAnalyzerCheckers library, but that
library depends on clangStaticAnalyzerCore. However, clangStaticAnalyzerFrontend
depends on both of those libraries.

One can make the observation however, that CheckerRegistry has no place in Core,
it isn't used there at all! The only place where it is used is Frontend, which
is where it ultimately belongs.

This move implies that since
include/clang/StaticAnalyzer/Checkers/ClangCheckers.h only contained a single function:

class CheckerRegistry;

void registerBuiltinCheckers(CheckerRegistry &registry);

it had to re purposed, as CheckerRegistry is no longer available to
clangStaticAnalyzerCheckers. It was renamed to BuiltinCheckerRegistration.h,
which actually describes it a lot better -- it does not contain the registration
functions for checkers, but only those generated by the tblgen files.

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

llvm-svn: 349275

show more ...

Port getLocStart -> getBeginLoc

Reviewers: teemperor!

Subscribers: jholewinski, whisperity, jfb, cfe-commits

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

llvm-svn: 339385

Remove trailing space

sed -Ei 's/[[:space:]]+$//' include/**/*.{def,h,td} lib/**/*.{cpp,h}

llvm-svn: 338291

[analyzer] Make checkEndFunction() give access to the return statement.

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

llvm-svn: 337215

[analyzer] [NFC] A convenient getter for getting a current stack frame

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

llvm-svn: 335701

[analyzer] introduce getSVal(Stmt *) helper on ExplodedNode, make sure the helper is used consistently

In most cases using
`N->getState()->getSVal(E, N->getLocationContext())`
is ugly, verbose, and

[analyzer] introduce getSVal(Stmt *) helper on ExplodedNode, make sure the helper is used consistently

In most cases using
`N->getState()->getSVal(E, N->getLocationContext())`
is ugly, verbose, and also opens up more surface area for bugs if an
inconsistent location context is used.

This patch introduces a helper on an exploded node, and ensures
consistent usage of either `ExplodedNode::getSVal` or
`CheckContext::getSVal` across the codebase.
As a result, a large number of redundant lines is removed.

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

llvm-svn: 322753

show more ...

[analyzer] StackAddrEscape: For now, disable the new async escape checks.

The new check introduced in r318705 is useful, but suffers from a particular
class of false positives, namely, it does not a

[analyzer] StackAddrEscape: For now, disable the new async escape checks.

The new check introduced in r318705 is useful, but suffers from a particular
class of false positives, namely, it does not account for
dispatch_barrier_sync() API which allows one to ensure that the asyncronously
executed block that captures a pointer to a local variable does not actually
outlive that variable.

The new check is split into a separate checker, under the name of
alpha.core.StackAddressAsyncEscape, which is likely to get enabled by default
again once these positives are fixed. The rest of the StackAddressEscapeChecker
is still enabled by default.

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

llvm-svn: 320455

show more ...

[analyzer] Diagnose stack leaks via block captures

This diff extends StackAddrEscapeChecker
to catch stack addresses leaks via block captures
if the block is executed asynchronously or
returned from

[analyzer] Diagnose stack leaks via block captures

This diff extends StackAddrEscapeChecker
to catch stack addresses leaks via block captures
if the block is executed asynchronously or
returned from a function.

Differential revision: https://reviews.llvm.org/D39438

llvm-svn: 318705

show more ...

[Analyzer] Correct stack address escape diagnostic

Summary:
Leaking a stack address via a static variable refers to it in the diagnostic as a 'global'. This patch corrects the diagnostic for static

[Analyzer] Correct stack address escape diagnostic

Summary:
Leaking a stack address via a static variable refers to it in the diagnostic as a 'global'. This patch corrects the diagnostic for static variables.


Patch by Phil Camp, SN Systems

Reviewers: dcoughlin, zaks.anna

Subscribers: xazax.hun, cfe-commits

Differential Revision: http://reviews.llvm.org/D19866

Patch by Phil Camp

llvm-svn: 270849

show more ...

[analyzer] Suppress stack address escape on CK_CopyAndAutoreleaseBlockObject.

Don't warn about addresses of stack-allocated blocks escaping if the block
region was cast with CK_CopyAndAutoreleaseBlo

[analyzer] Suppress stack address escape on CK_CopyAndAutoreleaseBlockObject.

Don't warn about addresses of stack-allocated blocks escaping if the block
region was cast with CK_CopyAndAutoreleaseBlockObject. These casts, which
are introduced in the implicit conversion operator for lambda-to-block
conversions, cause the block to be copied to the heap -- so the warning is
spurious.

llvm-svn: 254639

show more ...

[analyzer] Add generateErrorNode() APIs to CheckerContext.

The analyzer trims unnecessary nodes from the exploded graph before reporting
path diagnostics. However, in some cases it can trim all node

[analyzer] Add generateErrorNode() APIs to CheckerContext.

The analyzer trims unnecessary nodes from the exploded graph before reporting
path diagnostics. However, in some cases it can trim all nodes (including the
error node), leading to an assertion failure (see
https://llvm.org/bugs/show_bug.cgi?id=24184).

This commit addresses the issue by adding two new APIs to CheckerContext to
explicitly create error nodes. Unless the client provides a custom tag, these
APIs tag the node with the checker's tag -- preventing it from being trimmed.
The generateErrorNode() method creates a sink error node, while
generateNonFatalErrorNode() creates an error node for a path that should
continue being explored.

The intent is that one of these two methods should be used whenever a checker
creates an error node.

This commit updates the checkers to use these APIs. These APIs
(unlike addTransition() and generateSink()) do not take an explicit Pred node.
This is because there are not any error nodes in the checkers that were created
with an explicit different than the default (the CheckerContext's Pred node).

It also changes generateSink() to require state and pred nodes (previously
these were optional) to reduce confusion.

Additionally, there were several cases where checkers did check whether a
generated node could be null; we now explicitly check for null in these places.

This commit also includes a test case written by Ying Yi as part of
http://reviews.llvm.org/D12163 (that patch originally addressed this issue but
was reverted because it introduced false positive regressions).

Differential Revision: http://reviews.llvm.org/D12780

llvm-svn: 247859

show more ...

[analyzer] Apply whitespace cleanups by Honggyu Kim.

llvm-svn: 246978

Clarify pointer ownership semantics by hoisting the std::unique_ptr creation to the caller instead of hiding it in emitReport. NFC.

llvm-svn: 240400

Revert r240270 ("Fixed/added namespace ending comments using clang-tidy").

llvm-svn: 240353

Fixed/added namespace ending comments using clang-tidy. NFC

The patch is generated using this command:

$ tools/extra/clang-tidy/tool/run-clang-tidy.py -fix \
-checks=-*,llvm-namespace-comme

Fixed/added namespace ending comments using clang-tidy. NFC

The patch is generated using this command:

$ tools/extra/clang-tidy/tool/run-clang-tidy.py -fix \
-checks=-*,llvm-namespace-comment -header-filter='llvm/.*|clang/.*' \
work/llvm/tools/clang

To reduce churn, not touching namespaces spanning less than 10 lines.

llvm-svn: 240270

show more ...