[analyzer] Remove the default value arg from getChecker*Option

Since D57922, the config table contains every checker option, and it's default
value, so having it as an argument for getChecker*Option

[analyzer] Remove the default value arg from getChecker*Option

Since D57922, the config table contains every checker option, and it's default
value, so having it as an argument for getChecker*Option is redundant.

By the time any of the getChecker*Option function is called, we verified the
value in CheckerRegistry (after D57860), so we can confidently assert here, as
any irregularities detected at this point must be a programmer error. However,
in compatibility mode, verification won't happen, so the default value must be
restored.

This implies something else, other than adding removing one more potential point
of failure -- debug.ConfigDumper will always contain valid values for
checker/package options!

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

llvm-svn: 361042

show more ...

[analyzer] Enable subcheckers to possess checker options

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

[analyzer] Enable subcheckers to possess checker options

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

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

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

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

llvm-svn: 355297

show more ...

Fix file headers. NFC

llvm-svn: 355176

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

Misc typos fixes in ./lib folder

Summary: Found via `codespell -q 3 -I ../clang-whitelist.txt -L uint,importd,crasher,gonna,cant,ue,ons,orign,ned`

Reviewers: teemperor

Reviewed By: teemperor

Subs

Misc typos fixes in ./lib folder

Summary: Found via `codespell -q 3 -I ../clang-whitelist.txt -L uint,importd,crasher,gonna,cant,ue,ons,orign,ned`

Reviewers: teemperor

Reviewed By: teemperor

Subscribers: teemperor, jholewinski, jvesely, nhaehnle, whisperity, jfb, cfe-commits

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

llvm-svn: 348755

show more ...

Re-apply r347954 "[analyzer] Nullability: Don't detect post factum violation..."

Buildbot failures were caused by an unrelated UB that was introduced in r347943
and fixed in r347970.

Also the revis

Re-apply r347954 "[analyzer] Nullability: Don't detect post factum violation..."

Buildbot failures were caused by an unrelated UB that was introduced in r347943
and fixed in r347970.

Also the revision was incorrectly specified as r344580 during revert.

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

llvm-svn: 348188

show more ...

Revert r344580 "[analyzer] Nullability: Don't detect post factum violation..."

Fails under ASan!

llvm-svn: 347956

[analyzer] Nullability: Don't detect post factum violation on concrete values.

The checker suppresses warnings on paths on which a nonnull value is assumed
to be nullable. This probably deserves a w

[analyzer] Nullability: Don't detect post factum violation on concrete values.

The checker suppresses warnings on paths on which a nonnull value is assumed
to be nullable. This probably deserves a warning, but it's a separate story.

Now, because dead symbol collection fires in pretty random moments,
there sometimes was a situation when dead symbol collection fired after
computing a parameter but before actually evaluating call enter into the
function, which triggered the suppression when the argument was null
in the first place earlier than the obvious warning for null-to-nonnull
was emitted, causing false negatives.

Only trigger the suppression for symbols, not for concrete values.

It is impossible to constrain a concrete value post-factum because
it is impossible to constrain a concrete value at all.

This covers all the necessary cases because by the time we reach the call,
symbolic values should be either not constrained to null, or already collapsed
into concrete null values. Which in turn happens because they are passed through
the Store, and the respective collapse is implemented as part of getSVal(),
which is also weird.

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

llvm-svn: 347954

show more ...

[analyzer] Fix the "Zombie Symbols" bug.

It's an old bug that consists in stale references to symbols remaining in the
GDM if they disappear from other program state sections as a result of any
oper

[analyzer] Fix the "Zombie Symbols" bug.

It's an old bug that consists in stale references to symbols remaining in the
GDM if they disappear from other program state sections as a result of any
operation that isn't the actual dead symbol collection. The most common example
here is:

FILE *fp = fopen("myfile.txt", "w");
fp = 0; // leak of file descriptor

In this example the leak were not detected previously because the symbol
disappears from the public part of the program state due to evaluating
the assignment. For that reason the checker never receives a notification
that the symbol is dead, and never reports a leak.

This patch not only causes leak false negatives, but also a number of other
problems, including false positives on some checkers.

What's worse, even though the program state contains a finite number of symbols,
the set of symbols that dies is potentially infinite. This means that is
impossible to compute the set of all dead symbols to pass off to the checkers
for cleaning up their part of the GDM.

No longer compute the dead set at all. Disallow iterating over dead symbols.
Disallow querying if any symbols are dead. Remove the API for marking symbols
as dead, as it is no longer necessary. Update checkers accordingly.

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

llvm-svn: 347953

show more ...

[analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

One of the reasons why AnalyzerOptions is so chaotic is that options can be
retrieved from the comman

[analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

One of the reasons why AnalyzerOptions is so chaotic is that options can be
retrieved from the command line whenever and wherever. This allowed for some
options to be forgotten for a looooooong time. Have you ever heard of
"region-store-small-struct-limit"? In order to prevent this in the future, I'm
proposing to restrict AnalyzerOptions' interface so that only checker options
can be retrieved without special getters. I would like to make every option be
accessible only through a getter, but checkers from plugins are a thing, so I'll
have to figure something out for that.

This also forces developers who'd like to add a new option to register it
properly in the .def file.

This is done by

* making the third checker pointer parameter non-optional, and checked by an
assert to be non-null.
* I added new, but private non-checkers option initializers, meant only for
internal use,
* Renamed these methods accordingly (mind the consistent name for once with
getBooleanOption!):
- getOptionAsString -> getCheckerStringOption,
- getOptionAsInteger -> getCheckerIntegerOption
* The 3 functions meant for initializing data members (with the not very
descriptive getBooleanOption, getOptionAsString and getOptionAsUInt names)
were renamed to be overloads of the getAndInitOption function name.
* All options were in some way retrieved via getCheckerOption. I removed it, and
moved the logic to getStringOption and getCheckerStringOption. This did cause
some code duplication, but that's the only way I could do it, now that checker
and non-checker options are separated. Note that the non-checker version
inserts the new option to the ConfigTable with the default value, but the
checker version only attempts to find already existing entries. This is how
it always worked, but this is clunky and I might end reworking that too, so we
can eventually get a ConfigTable that contains the entire configuration of the
analyzer.

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

llvm-svn: 346113

show more ...

[analyzer] Rename trackNullOrUndefValue to trackExpressionValue

trackNullOrUndefValue is a long and confusing name,
and it does not actually reflect what the function is doing.
Give a function a new

[analyzer] Rename trackNullOrUndefValue to trackExpressionValue

trackNullOrUndefValue is a long and confusing name,
and it does not actually reflect what the function is doing.
Give a function a new name, with a relatively clear semantics.

Also remove some dead code.

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

llvm-svn: 345064

show more ...

[analyzer] [NFC] Remove unused parameters, as found by -Wunused-parameter

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

llvm-svn: 343353

Port getLocStart -> getBeginLoc

Reviewers: teemperor!

Subscribers: jholewinski, whisperity, jfb, cfe-commits

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

llvm-svn: 339385

[analyzer] Do not run visitors until the fixpoint, run only once.

In the current implementation, we run visitors until the fixed point is
reached.
That is, if a visitor adds another visitor, the cur

[analyzer] Do not run visitors until the fixpoint, run only once.

In the current implementation, we run visitors until the fixed point is
reached.
That is, if a visitor adds another visitor, the currently processed path
is destroyed, all diagnostics is discarded, and it is regenerated again,
until it's no longer modified.
This pattern has a few negative implications:

- This loop does not even guarantee to terminate.
E.g. just imagine two visitors bouncing a diagnostics around.
- Performance-wise, e.g. for sqlite3 all visitors are being re-run at
least 10 times for some bugs.
We have already seen a few reports where it leads to timeouts.
- If we want to add more computationally intense visitors, this will
become worse.
- From architectural standpoint, the current layout requires copying
visitors, which is conceptually wrong, and can be annoying (e.g. no
unique_ptr on visitors allowed).

The proposed change is a much simpler architecture: the outer loop
processes nodes upwards, and whenever the visitor is added it only
processes current nodes and above, thus guaranteeing termination.

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

llvm-svn: 335666

show more ...

Fix typos in clang

Found via codespell -q 3 -I ../clang-whitelist.txt
Where whitelist consists of:

archtype
cas
classs
checkk
compres
definit
frome
iff
inteval
ith
lod
metho

Fix typos in clang

Found via codespell -q 3 -I ../clang-whitelist.txt
Where whitelist consists of:

archtype
cas
classs
checkk
compres
definit
frome
iff
inteval
ith
lod
methode
nd
optin
ot
pres
statics
te
thru

Patch by luzpaz! (This is a subset of D44188 that applies cleanly with a few
files that have dubious fixes reverted.)

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

llvm-svn: 329399

show more ...

[analyzer] Trust _Nonnull annotations for system framework

Changes the analyzer to believe that methods annotated with _Nonnull
from system frameworks indeed return non null objects.
Local methods w

[analyzer] Trust _Nonnull annotations for system framework

Changes the analyzer to believe that methods annotated with _Nonnull
from system frameworks indeed return non null objects.
Local methods with such annotation are still distrusted.
rdar://24291919

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

llvm-svn: 328282

show more ...

[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] Nullability: fix notes around synthesized ObjC property accessors.

Nullable-to-nonnull checks used to crash when the custom bug visitor was trying
to add its notes to autosynthesized acce

[analyzer] Nullability: fix notes around synthesized ObjC property accessors.

Nullable-to-nonnull checks used to crash when the custom bug visitor was trying
to add its notes to autosynthesized accessors of Objective-C properties.

Now we avoid this, mostly automatically outside of checker control, by
moving the diagnostic to the parent stack frame where the accessor has been
called.

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

llvm-svn: 304710

show more ...

[analyzer] Fix memory error bug category capitalization.

It was written as "Memory Error" in most places and as "Memory error" in a few
other places, however it is the latter that is more consistent

[analyzer] Fix memory error bug category capitalization.

It was written as "Memory Error" in most places and as "Memory error" in a few
other places, however it is the latter that is more consistent with
other categories (such as "Logic error").

rdar://problem/31718115

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

llvm-svn: 302016

show more ...

Spelling mistakes in comments. NFCI. (PR27635)

llvm-svn: 299083

Migrate PathDiagnosticPiece to std::shared_ptr

Simplifies and makes explicit the memory ownership model rather than
implicitly passing/acquiring ownership.

llvm-svn: 291143