[Clang][NFC] Rename CXXMethodDecl::isPure -> is VirtualPure (#78463)

To avoid any possible confusion with the notion of pure function and the
gnu::pure attribute.

[clang][nearly-NFC] Remove some superfluous uses of NamedDecl::getNameAsString

`OS << ND->getDeclName();` is equivalent to `OS << ND->getNameAsString();`
without the extra temporary string.

This is

[clang][nearly-NFC] Remove some superfluous uses of NamedDecl::getNameAsString

`OS << ND->getDeclName();` is equivalent to `OS << ND->getNameAsString();`
without the extra temporary string.

This is not quite a NFC since two uses of `getNameAsString` in a
diagnostic are replaced, which results in the named entity being
quoted with additional "'"s (ie: 'var' instead of var).

show more ...

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

Remove Expr.h include from ASTContext.h, NFC

ASTContext.h is popular, prune its includes. Expr.h brings in Attr.h,
which is also expensive.

Move BlockVarCopyInit to Expr.h to accomplish this.

[analyzer][NFC] Fix inconsistent references to checkers as "checks"

Traditionally, clang-tidy uses the term check, and the analyzer uses checker,
but in the very early years, this wasn't the case, a

[analyzer][NFC] Fix inconsistent references to checkers as "checks"

Traditionally, clang-tidy uses the term check, and the analyzer uses checker,
but in the very early years, this wasn't the case, and code originating from the
early 2010's still incorrectly refer to checkers as checks.

This patch attempts to hunt down most of these, aiming to refer to checkers as
checkers, but preserve references to callback functions (like checkPreCall) as
checks.

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

llvm-svn: 371760

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

[analyzer] Add minimal support for fix-it hints.

Allow attaching fixit hints to Static Analyzer BugReports.

Fixits are attached either to the bug report itself or to its notes
(path-sensitive event

[analyzer] Add minimal support for fix-it hints.

Allow attaching fixit hints to Static Analyzer BugReports.

Fixits are attached either to the bug report itself or to its notes
(path-sensitive event notes or path-insensitive extra notes).

Add support for fixits in text output (including the default text output that
goes without notes, as long as the fixit "belongs" to the warning).

Add support for fixits in the plist output mode.

Implement a fixit for the path-insensitive DeadStores checker. Only dead
initialization warning is currently covered.

Implement a fixit for the path-sensitive VirtualCall checker when the virtual
method is not pure virtual (in this case the "fix" is to suppress the warning
by qualifying the call).

Both fixits are under an off-by-default flag for now, because they
require more careful testing.

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

llvm-svn: 371257

show more ...

[analyzer] Fix analyzer warnings on analyzer.

Write tests for the actual crash that was found. Write comments and refactor
code around 17 style bugs and suppress 3 false positives.

Differential Rev

[analyzer] Fix analyzer warnings on analyzer.

Write tests for the actual crash that was found. Write comments and refactor
code around 17 style bugs and suppress 3 false positives.

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

llvm-svn: 370246

show more ...

[analyzer] Improve VirtualCallChecker and enable parts of it by default.

Calling a pure virtual method during construction or destruction
is undefined behavior. It's worth it to warn about it by def

[analyzer] Improve VirtualCallChecker and enable parts of it by default.

Calling a pure virtual method during construction or destruction
is undefined behavior. It's worth it to warn about it by default.
That part is now known as the cplusplus.PureVirtualCall checker.

Calling a normal virtual method during construction or destruction
may be fine, but does behave unexpectedly, as it skips virtual dispatch.
Do not warn about this by default, but let projects opt in into it
by enabling the optin.cplusplus.VirtualCall checker manually.

Give the two parts differentiated warning text:

Before:

Call to virtual function during construction or destruction:
Call to pure virtual function during construction

Call to virtual function during construction or destruction:
Call to virtual function during destruction

After:

Pure virtual method call:
Call to pure virtual method 'X::foo' during construction
has undefined behavior

Unexpected loss of virtual dispatch:
Call to virtual method 'Y::bar' during construction
bypasses virtual dispatch

Also fix checker names in consumers that support them (eg., clang-tidy)
because we now have different checker names for pure virtual calls and
regular virtual calls.

Also fix capitalization in the bug category.

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

llvm-svn: 369449

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][NFC] Refactoring BugReporter.cpp P3.: std::shared_pointer<PathDiagnosticPiece> -> PathDiagnosticPieceRef

find clang/ -type f -exec sed -i 's/std::shared_ptr<PathDiagnosticPiece>/PathDiagn

[analyzer][NFC] Refactoring BugReporter.cpp P3.: std::shared_pointer<PathDiagnosticPiece> -> PathDiagnosticPieceRef

find clang/ -type f -exec sed -i 's/std::shared_ptr<PathDiagnosticPiece>/PathDiagnosticPieceRef/g' {} \;
git diff -U3 --no-color HEAD^ | clang-format-diff-6.0 -p1 -i

Just as C++ is meant to be refactored, right?

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

llvm-svn: 368717

show more ...

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

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

[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] [NFC] Remove unused parameters, as found by -Wunused-parameter

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

llvm-svn: 343353

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

[analyzer] Fix an assertion fail in VirtualCallChecker

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

llvm-svn: 313866

[analyzer][GSoC] Re-implemente current virtual calls checker in a path-sensitive way

Patch by: Xin Wang

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

llvm-svn: 311877

In VirtualCallChecker, handle indirect calls

Summary:
In VirtualCallChecker, handle indirect calls.

getDirectCallee() can be nullptr, and dyn_cast(nullptr) is UB

Reviewers: bkramer

Subscribers: c

In VirtualCallChecker, handle indirect calls

Summary:
In VirtualCallChecker, handle indirect calls.

getDirectCallee() can be nullptr, and dyn_cast(nullptr) is UB

Reviewers: bkramer

Subscribers: cfe-commits

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

llvm-svn: 293604

show more ...

[analyzer] Improve VirtualCallChecker diagnostics and move into optin package.

The VirtualCallChecker is in alpha because its interprocedural diagnostics
represent the call path textually in the dia

[analyzer] Improve VirtualCallChecker diagnostics and move into optin package.

The VirtualCallChecker is in alpha because its interprocedural diagnostics
represent the call path textually in the diagnostic message rather than with a
path sensitive diagnostic.

This patch turns off the AST-based interprocedural analysis in the checker so
that no call path is needed and improves with diagnostic text. With these
changes, the checker is ready to be moved into the optin package.

Ultimately the right fix is to rewrite this checker to be path sensitive -- but
there is still value in enabling the checker for intraprocedural analysis only
The interprocedural mode can be re-enabled with an -analyzer-config flag.

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

llvm-svn: 289309

show more ...