[analyzer] Don't assume third iteration in loops (#119388)

This commit ensures that if the loop condition is opaque (the analyzer
cannot determine whether it's true or false) and there were at leas

[analyzer] Don't assume third iteration in loops (#119388)

This commit ensures that if the loop condition is opaque (the analyzer
cannot determine whether it's true or false) and there were at least two
iterations, then the analyzer doesn't make the unjustified assumption
that it can enter yet another iteration.

Note that the presence of a loop suggests that the developer thought
that two iterations can happen (otherwise an `if` would've been
sufficient), but it does not imply that the developer expected three or
four iterations -- and in fact there are many false positives where a
loop iterates over a two-element (or three-element) data structure, but
the analyzer cannot understand the loop condition and blindly assumes
that there may be three or more iterations. (In particular, analyzing
the FFMPEG project produces 100+ such false positives.)

Moreover, this provides some performance improvements in the sense that
the analyzer won't waste time on traversing the execution paths with 3
or 4 iterations in a loop (which are very similar to the paths with 2
iterations) and therefore will be able to traverse more branches
elsewhere on the `ExplodedGraph`.

This logic is disabled if the user enables the widen-loops analyzer
option (which is disabled by default), because the "simulate one final
iteration after the invalidation" execution path would be suppressed by
the "exit the loop if the loop condition is opaque and there were at
least two iterations" logic. If we want to support loop widening, we
would need to create a follow-up commit which ensures that it "plays
nicely" with this logic.

show more ...

[analyzer][NFC] Cleanup BranchNodeBuilder (#117898)

Previously `BranchNodeBuilder` had a machinery to mark the two possible
branches (true, false) as infeasible, but this was completely useless in

[analyzer][NFC] Cleanup BranchNodeBuilder (#117898)

Previously `BranchNodeBuilder` had a machinery to mark the two possible
branches (true, false) as infeasible, but this was completely useless in
practice, because the `BranchNodeBuilder` objects where short-lived
local variables so the `markInfeasible()` calls did not affect any later
calls.

The only theoretical exception was that in `ExprEngine::processBranch`
the methods of `BranchNodeBuilder` were called within a `for` loop that
iterates over the nodes created by the `check::BranchCondition`
callbacks.

However, currently only two checkers listen to `check::BranchCondition`
and neither of them produces multiple out nodes. This is fortunate,
because if the `for` loop had multiple iterations, then the lingering
effects of `markInfeasible()` would have caused wildly incorrect
behavior.

_For example, let's assume that a hypothetical `check::BranchCondition`
callback transitions to two different states, and the condition
expression happens to be true in the first and false in the second. In
this situation the first iteration of the loop would mark the false
branch as 'infeasible' and then in the second iteration the analyzer
would skip creating the transition to the false branch (from the state
where that is the 'right' path forward)._

After removing `markInfeasible()`, it became clear that the
`isFeasible()` calls in `ExprEngine::processBranch` are redundant
because they only guarded a `generateNode` call -- which immediately
calls `isFeasible()` and does nothing on an infeasible branch.

At this point in the refactoring the only code writing the feasibility
data members is their initialization in the constructors of
`BranchNodeBuilder` and the only code reading them is the check at the
beginning of `BranchNodeBuilder::generateNode`, so it was
straightforward to remove them completely and simplify the logic of
`generateNode()` to let it directly check the nullness of the
`CFGBlock*` pointer that it wants to use.

Finally, after these changes it became clear that in
`ExprEngine::processBranch` the `else` branch
(corresponding to the case when `assumeCondition` fails) is equivalent
to the "normal" case, so I eliminated it as well.

I also update the capitalization of a few variables that are already
affected by this change.

show more ...

[analyzer][NFC] Turn NodeBuilderContext into a class (#84638)

From issue #73088. I changed `NodeBuilderContext` into a class.
Additionally, there were some other mentions of the former being a
str

[analyzer][NFC] Turn NodeBuilderContext into a class (#84638)

From issue #73088. I changed `NodeBuilderContext` into a class.
Additionally, there were some other mentions of the former being a
struct which I also changed into a class. This is my first time working
with an issue so I will be open to hearing any advice or changes that
need to be done.

show more ...

[analyzer][NFC] Remove dead code (#83968)

Remove the unused method `CoreEngine::ExecuteWorkListWithInitialState`.

[clang][NFC] Refactor `CXXConstructExpr::ConstructionKind`

This patch converts `CXXConstructExpr::ConstructionKind` into a scoped enum in namespace scope, making it eligible for forward declaring. T

[clang][NFC] Refactor `CXXConstructExpr::ConstructionKind`

This patch converts `CXXConstructExpr::ConstructionKind` into a scoped enum in namespace scope, making it eligible for forward declaring. This is useful in cases like annotating bit-fields with `preferred_type`.

show more ...

[clang] Remove remaining uses of llvm::Optional (NFC)

This patch removes several "using" declarations and #include
"llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to

[clang] Remove remaining uses of llvm::Optional (NFC)

This patch removes several "using" declarations and #include
"llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...

[clang] Use std::optional instead of llvm::Optional (NFC)

This patch replaces (llvm::|)Optional< with std::optional<. I'll post
a separate patch to remove #include "llvm/ADT/Optional.h".

This is p

[clang] Use std::optional instead of llvm::Optional (NFC)

This patch replaces (llvm::|)Optional< with std::optional<. I'll post
a separate patch to remove #include "llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...

[clang] Add #include <optional> (NFC)

This patch adds #include <optional> to those files containing
llvm::Optional<...> or Optional<...>.

I'll post a separate patch to actually replace llvm::Option

[clang] Add #include <optional> (NFC)

This patch adds #include <optional> to those files containing
llvm::Optional<...> or Optional<...>.

I'll post a separate patch to actually replace llvm::Optional with
std::optional.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...

[clang] Qualify auto in range-based for loops (NFC)

[clang][analyzer][ctu] Make CTU a two phase analysis

This new CTU implementation is the natural extension of the normal single TU
analysis. The approach consists of two analysis phases. During the f

[clang][analyzer][ctu] Make CTU a two phase analysis

This new CTU implementation is the natural extension of the normal single TU
analysis. The approach consists of two analysis phases. During the first phase,
we do a normal single TU analysis. During this phase, if we find a foreign
function (that could be inlined from another TU) then we don’t inline that
immediately, we rather mark that to be analysed later.
When the first phase is finished then we start the second phase, the CTU phase.
In this phase, we continue the analysis from that point (exploded node)
which had been enqueued during the first phase. We gradually extend the
exploded graph of the single TU analysis with the new node that was
created by the inlining of the foreign function.

We count the number of analysis steps of the first phase and we limit the
second (ctu) phase with this number.

This new implementation makes it convenient for the users to run the
single-TU and the CTU analysis in one go, they don't need to run the two
analysis separately. Thus, we name this new implementation as "onego" CTU.

Discussion:
https://discourse.llvm.org/t/rfc-much-faster-cross-translation-unit-ctu-analysis-implementation/61728

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

show more ...

[NFC][clang]Inclusive language: remove remaining uses of sanity

Missed some uses of sanity check in previous commits.

[analyzer] Decouple NoteTag from its Factory

This allows us to create other types of tags that carry useful
bits of information alongside.

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

[analyzer] Engine: fix crash with SEH __leave keyword

MSVC has a `try-except` statement.
This statement could containt a `__leave` keyword, which is similar to
`goto` to the end of the try block. Th

[analyzer] Engine: fix crash with SEH __leave keyword

MSVC has a `try-except` statement.
This statement could containt a `__leave` keyword, which is similar to
`goto` to the end of the try block. The semantic of this keyword is not
implemented.

We should at least parse such code without crashing.

https://docs.microsoft.com/en-us/cpp/cpp/try-except-statement?view=msvc-160

Patch By: AbbasSabra!

Reviewed By: steakhal

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

show more ...

[Analyzer][NFC] Remove the SubEngine interface

The `SubEngine` interface is an interface with only one implementation
`EpxrEngine`. Adding other implementations are difficult and very
unlikely in th

[Analyzer][NFC] Remove the SubEngine interface

The `SubEngine` interface is an interface with only one implementation
`EpxrEngine`. Adding other implementations are difficult and very
unlikely in the near future. Currently, if anything from `ExprEngine` is
to be exposed to other classes it is moved to `SubEngine` which
restricts the alternative implementations. The virtual methods are have
a slight perofrmance impact. Furthermore, instead of the `LLVM`-style
inheritance a native inheritance is used here, which renders `LLVM`
functions like e.g. `cast<T>()` unusable here. This patch removes this
interface and allows usage of `ExprEngine` directly.

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

show more ...

[Analyzer][NFC] Change parameter of NoteTag lambdas to PathSensitiveBugReport

Lambdas creating path notes using NoteTags still take BugReport as their
parameter. Since path notes obviously only appe

[Analyzer][NFC] Change parameter of NoteTag lambdas to PathSensitiveBugReport

Lambdas creating path notes using NoteTags still take BugReport as their
parameter. Since path notes obviously only appear in PathSensitiveBugReports
it is straightforward that lambdas of NoteTags take PathSensitiveBugReport
as their parameter.

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

show more ...

[analyzer] Fix clang-tidy crash on GCCAsmStmt

Summary:
Added entry in switch statement to recognize GCCAsmStmt
as a possible block terminator.

Handling to build CFG using GCCAsmStmt was already imp

[analyzer] Fix clang-tidy crash on GCCAsmStmt

Summary:
Added entry in switch statement to recognize GCCAsmStmt
as a possible block terminator.

Handling to build CFG using GCCAsmStmt was already implemented.

Reviewers: nickdesaulniers, george.karpenkov, NoQ

Reviewed By: nickdesaulniers, NoQ

Subscribers: xbolva00, tmroeder, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, Charusso, cfe-commits

Tags: #clang

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

llvm-svn: 364605

show more ...

[analyzer] Add a prunable note for skipping vbase inits in subclasses.

When initialization of virtual base classes is skipped, we now tell the user
about it, because this aspect of C++ isn't very we

[analyzer] Add a prunable note for skipping vbase inits in subclasses.

When initialization of virtual base classes is skipped, we now tell the user
about it, because this aspect of C++ isn't very well-known.

The implementation is based on the new "note tags" feature (r358781).
In order to make use of it, allow note tags to produce prunable notes,
and move the note tag factory to CoreEngine.

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

llvm-svn: 361682

show more ...

[CFG] Add branch to skip vbase inits when they're handled by superclass.

This patch adds the run-time CFG branch that would skip initialization of
virtual base classes depending on whether the const

[CFG] Add branch to skip vbase inits when they're handled by superclass.

This patch adds the run-time CFG branch that would skip initialization of
virtual base classes depending on whether the constructor is called from a
superclass constructor or not. Previously the Static Analyzer was already
skipping virtual base-class initializers in such constructors, but it wasn't
skipping their arguments and their potential side effects, which was causing
pr41300 (and was generally incorrect). The previous skipping behavior is
now replaced with a hard assertion that we're not even getting there due
to how our CFG works.

The new CFG element is under a CFG build option so that not to break other
consumers of the CFG by this change. Static Analyzer support for this change
is implemented.

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

llvm-svn: 361681

show more ...

[CFG] NFC: Remove implicit conversion from CFGTerminator to Stmt *.

Turn it into a variant class instead. This conversion does indeed save some code
but there's a plan to add support for more kinds

[CFG] NFC: Remove implicit conversion from CFGTerminator to Stmt *.

Turn it into a variant class instead. This conversion does indeed save some code
but there's a plan to add support for more kinds of terminators that aren't
necessarily based on statements, and with those in mind it becomes more and more
confusing to have CFGTerminators implicitly convertible to a Stmt *.

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

llvm-svn: 361586

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] Collect all -analyzer-config options in a .def file

I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-con

[analyzer][NFC] Collect all -analyzer-config options in a .def file

I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from the
command line, and be able to list them all.

In this patch, I'm moving all analyzer options to a def file, and move 2 enums
to global namespace.

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

llvm-svn: 345986

show more ...

Fix MSVC "not all control paths return a value" warning. NFCI.

llvm-svn: 344892

[analyzer][NFC] Fix inconsistencies in AnalyzerOptions

I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is

[analyzer][NFC] Fix inconsistencies in AnalyzerOptions

I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from the
command line, and be able to list them all.

This first NFC patch contains small modifications to make AnalyzerOptions.cpp a
little more consistent.

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

llvm-svn: 344870

show more ...

[analyzer] Experiment with an iteration order only based on location, and not using the stack frame

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

llvm-svn: 344313

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

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

llvm-svn: 343353