[analyzer][NFC] Factor out SymbolManager::get<*> (#121781)

Replace the family of `SymbolManager::get*Symbol(...)` member functions
with a single generic `SymbolManager::get<*>` member function.

Reapply "[clang][analyzer] Stable order for SymbolRef-keyed containers" (#121749)

Generalize the SymbolIDs used for SymbolData to all SymExprs and use
these IDs for comparison SymbolRef keys in var

Reapply "[clang][analyzer] Stable order for SymbolRef-keyed containers" (#121749)

Generalize the SymbolIDs used for SymbolData to all SymExprs and use
these IDs for comparison SymbolRef keys in various containers, such as
ConstraintMap. These IDs are superior to raw pointer values because they
are more controllable and are not randomized across executions (unlike
[pointers](https://en.wikipedia.org/wiki/Address_space_layout_randomization)).

These IDs order is stable across runs because SymExprs are allocated in
the same order.

Stability of the constraint order is important for the stability of the
analyzer results. I evaluated this change on a set of 200+ open-source C
and C++ projects with the total number of ~78 000 symbolic-execution
issues passing Z3 refutation.

This patch reduced the run-to-run churn (flakiness) in SE issues from
80-90 to 30-40 (out of 78K) in our CSA deployment (in our setting flaky
issues are mostly due to Z3 refutation instability).

Note, most of the issue churn (flakiness) is caused by the mentioned Z3
refutation. With Z3 refutation disabled, issue churn goes down to ~10
issues out of 83K and this patch has no effect on appearing/disappearing
issues between runs. It however, seems to reduce the volatility of the
execution flow: before we had 40-80 issues with changed execution flow,
after - 10-30.

Importantly, this change is necessary for the next step in stabilizing
analysis results by caching Z3 query outcomes between analysis runs
(work in progress).

Across our admittedly noisy CI runs, I detected no significant effect on
memory footprint or analysis time.

This PR reapplies https://github.com/llvm/llvm-project/pull/121551 with
a fix to a g++ compiler error reported on some build bots

CPP-5919

show more ...

Revert "[clang][analyzer] Stable order for SymbolRef-keyed containers" (#121592)

Reverts llvm/llvm-project#121551

We had a bunch of build errors caused by this PR.
https://lab.llvm.org/buildbot/

Revert "[clang][analyzer] Stable order for SymbolRef-keyed containers" (#121592)

Reverts llvm/llvm-project#121551

We had a bunch of build errors caused by this PR.
https://lab.llvm.org/buildbot/#/builders/144/builds/14875

show more ...

[clang][analyzer] Stable order for SymbolRef-keyed containers (#121551)

Generalize the `SymbolID`s used for `SymbolData` to all `SymExpr`s and
use these IDs for comparison `SymbolRef` keys in vario

[clang][analyzer] Stable order for SymbolRef-keyed containers (#121551)

Generalize the `SymbolID`s used for `SymbolData` to all `SymExpr`s and
use these IDs for comparison `SymbolRef` keys in various containers,
such as `ConstraintMap`. These IDs are superior to raw pointer values
because they are more controllable and are not randomized across
executions (unlike
[pointers](https://en.wikipedia.org/wiki/Address_space_layout_randomization)).

These IDs order is stable across runs because SymExprs are allocated in
the same order.

Stability of the constraint order is important for the stability of the
analyzer results. I evaluated this change on a set of 200+ open-source C
and C++ projects with the total number of ~78 000 symbolic-execution
issues passing Z3 refutation.

This patch reduced the run-to-run churn (flakiness) in SE issues from
80-90 to 30-40 (out of 78K) in our CSA deployment (in our setting flaky
issues are mostly due to Z3 refutation instability).

Note, most of the issue churn (flakiness) is caused by the mentioned Z3
refutation. With Z3 refutation disabled, issue churn goes down to ~10
issues out of 83K and this patch has no effect on appearing/disappearing
issues between runs. It however, seems to reduce the volatility of the
execution flow: before we had 40-80 issues with changed execution flow,
after - 10-30.

Importantly, this change is necessary for the next step in stabilizing
analysis results by caching Z3 query outcomes between analysis runs
(work in progress).

Across our admittedly noisy CI runs, I detected no significant effect on
memory footprint or analysis time.

CPP-5919

show more ...

[analyzer][NFC] Migrate {SymInt,IntSym}Expr to use APSIntPtr (4/4) (#120438)

[analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

I'm involved with the Static Analyzer for the most part.
I think we should embrace newer language standard features and gradu

[analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

I'm involved with the Static Analyzer for the most part.
I think we should embrace newer language standard features and gradually
move forward.

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

show more ...

[clang][analyzer][NFC] Use the operator new directly with the `BumpPtrAllocator`

Reviewed By: xazax.hun

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

[clang][analyzer][NFC] Remove unnecessary casts around Allocate function calls

Reviewed By: steakhal

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

[clang] Use *{Set,Map}::contains (NFC)

[analyzer] Fix the liveness of Symbols for values in regions referred by LazyCompoundVal

To illustrate our current understanding, let's start with the following program:
https://godbolt.org/z/33f6vh

[analyzer] Fix the liveness of Symbols for values in regions referred by LazyCompoundVal

To illustrate our current understanding, let's start with the following program:
https://godbolt.org/z/33f6vheh1
```lang=c++
void clang_analyzer_printState();

struct C {
int x;
int y;
int more_padding;
};

struct D {
C c;
int z;
};

C foo(D d, int new_x, int new_y) {
d.c.x = new_x; // B1
assert(d.c.x < 13); // C1

C c = d.c; // L

assert(d.c.y < 10); // C2
assert(d.z < 5); // C3

d.c.y = new_y; // B2

assert(d.c.y < 10); // C4

return c; // R
}
```
In the code, we create a few bindings to subregions of root region `d` (`B1`, `B2`), a constrain on the values (`C1`, `C2`, ….), and create a `lazyCompoundVal` for the part of the region `d` at point `L`, which is returned at point `R`.

Now, the question is which of these should remain live as long the return value of the `foo` call is live. In perfect a word we should preserve:

# only the bindings of the subregions of `d.c`, which were created before the copy at `L`. In our example, this includes `B1`, and not `B2`. In other words, `new_x` should be live but `new_y` shouldn’t.

# constraints on the values of `d.c`, that are reachable through `c`. This can be created both before the point of making the copy (`L`) or after. In our case, that would be `C1` and `C2`. But not `C3` (`d.z` value is not reachable through `c`) and `C4` (the original value of`d.c.y` was overridden at `B2` after the creation of `c`).

The current code in the `RegionStore` covers the use case (1), by using the `getInterestingValues()` to extract bindings to parts of the referred region present in the store at the point of copy. This also partially covers point (2), in case when constraints are applied to a location that has binding at the point of the copy (in our case `d.c.x` in `C1` that has value `new_x`), but it fails to preserve the constraints that require creating a new symbol for location (`d.c.y` in `C2`).

We introduce the concept of //lazily copied// locations (regions) to the `SymbolReaper`, i.e. for which a program can access the value stored at that location, but not its address. These locations are constructed as a set of regions referred to by `lazyCompoundVal`. A //readable// location (region) is a location that //live// or //lazily copied// . And symbols that refer to values in regions are alive if the region is //readable//.

For simplicity, we follow the current approach to live regions and mark the base region as //lazily copied//, and consider any subregions as //readable//. This makes some symbols falsy live (`d.z` in our example) and keeps the corresponding constraints alive.

The rename `Regions` to `LiveRegions` inside `RegionStore` is NFC change, that was done to make it clear, what is difference between regions stored in this two sets.

Regression Test: https://reviews.llvm.org/D134941
Co-authored-by: Balazs Benics <benicsbalazs@gmail.com>

Reviewed By: martong, xazax.hun

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

show more ...

[analyzer] Add UnarySymExpr

This patch adds a new descendant to the SymExpr hierarchy. This way, now
we can assign constraints to symbolic unary expressions. Only the unary
minus and bitwise negatio

[analyzer] Add UnarySymExpr

This patch adds a new descendant to the SymExpr hierarchy. This way, now
we can assign constraints to symbolic unary expressions. Only the unary
minus and bitwise negation are handled.

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

show more ...

[clang] Add a raw_ostream operator<< overload for QualType

Under the hood this prints the same as `QualType::getAsString()` but cuts out the middle-man when that string is sent to another raw_ostrea

[clang] Add a raw_ostream operator<< overload for QualType

Under the hood this prints the same as `QualType::getAsString()` but cuts out the middle-man when that string is sent to another raw_ostream.

Also cleaned up all the call sites where this occurs.

Reviewed By: aaron.ballman

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

show more ...

[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
x && isa<T1>(x) ||

[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
x && isa<T1>(x) || isa<T2>(x) ...
Will be replaced by:
isa_and_non_null<T1, T2, ...>(x)

Sometimes it caused further simplifications, when it would cause even
more code smell.

Aside from this, keep in mind that within `assert()` or any macro
functions, we need to wrap the isa<> expression within a parenthesis,
due to the parsing of the comma.

Reviewed By: martong

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

show more ...

[analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

The summary and very short discussion in D82122 summarizes whats happening here.

In short, liveness talks

[analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

The summary and very short discussion in D82122 summarizes whats happening here.

In short, liveness talks about variables, or expressions, anything that
has a value. Well, statements just simply don't have a one.

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

show more ...

[analyzer][z3] Use more elaborate Z3 variable names

Previously, it was a tedious task to comprehend Z3 dumps.
We will use the same name prefix just as we use in the corresponding dump method

For al

[analyzer][z3] Use more elaborate Z3 variable names

Previously, it was a tedious task to comprehend Z3 dumps.
We will use the same name prefix just as we use in the corresponding dump method

For all `SymbolData` values:
`$###` -> `conj_$###`
`$###` -> `derived_$###`
`$###` -> `extent_$###`
`$###` -> `meta_$###`
`$###` -> `reg_$###`

Reviewed By: xazax.hun,mikhail.ramalho

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

show more ...

[analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

Based on the discussion in D82598#2171312. Thanks @NoQ!

D82598 is titled "Get rid of statement liveness, beca

[analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

Based on the discussion in D82598#2171312. Thanks @NoQ!

D82598 is titled "Get rid of statement liveness, because such a thing doesn't
exist", and indeed, expressions express a value, non-expression statements
don't.

if (a && get() || []{ return true; }())
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ has a value
~ has a value
~~~~~~~~~~ has a value
~~~~~~~~~~~~~~~~~~~~ has a value
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ doesn't have a value

That is simple enough, so it would only make sense if we only assigned symbolic
values to expressions in the static analyzer. Yet the interface checkers can
access presents, among other strange things, the following two methods:

ProgramState::BindExpr(const Stmt *S, const LocationContext *LCtx, SVal V,
bool Invalidate=true)
ProgramState::getSVal(const Stmt *S, const LocationContext *LCtx)

So, what gives? Turns out, we make an exception for ReturnStmt (which we'll
leave for another time) and ObjCForCollectionStmt. For any other loops, in order
to know whether we should analyze another iteration, among other things, we
evaluate it's condition. Which is a problem for ObjCForCollectionStmt, because
it simply doesn't have one (CXXForRangeStmt has an implicit one!). In its
absence, we assigned the actual statement with a concrete 1 or 0 to indicate
whether there are any more iterations left. However, this is wildly incorrect,
its just simply not true that the for statement has a value of 1 or 0, we can't
calculate its liveness because that doesn't make any sense either, so this patch
turns it into a GDM trait.

Fixing this allows us to reinstate the assert removed in
https://reviews.llvm.org/rG032b78a0762bee129f33e4255ada6d374aa70c71.

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

show more ...

[analyzer] Merge implementations of SymInt, IntSym, and SymSym exprs

Summary:
SymIntExpr, IntSymExpr, and SymSymExpr share a big portion of logic
that used to be duplicated across all three classes.

[analyzer] Merge implementations of SymInt, IntSym, and SymSym exprs

Summary:
SymIntExpr, IntSymExpr, and SymSymExpr share a big portion of logic
that used to be duplicated across all three classes. New
implementation also adds an easy way of introducing another type of
operands into the mix.

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

show more ...

SymbolManager::SymbolDependencies: Use unique_ptr to simplify memory management

[analyzer] Add support for CXXInheritedCtorInitExpr.

So far we've been dropping coverage every time we've encountered
a CXXInheritedCtorInitExpr. This patch attempts to add some
initial support for

[analyzer] Add support for CXXInheritedCtorInitExpr.

So far we've been dropping coverage every time we've encountered
a CXXInheritedCtorInitExpr. This patch attempts to add some
initial support for it.

Constructors for arguments of a CXXInheritedCtorInitExpr are still
not fully supported.

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

show more ...

[analyzer] DynamicSize: Remove 'getExtent()' from regions

Summary:
This patch introduces a placeholder for representing the dynamic size of
regions. It also moves the `getExtent()` method of `SubReg

[analyzer] DynamicSize: Remove 'getExtent()' from regions

Summary:
This patch introduces a placeholder for representing the dynamic size of
regions. It also moves the `getExtent()` method of `SubRegions` to the
`MemRegionManager` as `getStaticSize()`.

Reviewed By: NoQ

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

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] Make sure base-region and its sub-regions are either all alive or all dead.

SymbolReaper now realizes that our liveness analysis isn't sharp enough
to discriminate between liveness of, sa

[analyzer] Make sure base-region and its sub-regions are either all alive or all dead.

SymbolReaper now realizes that our liveness analysis isn't sharp enough
to discriminate between liveness of, say, variables and their fields.
Surprisingly, this didn't quite work before: having a variable live only
through Environment (eg., calling a C++ method on a local variable
as the last action ever performed on that variable) would not keep the
region value symbol of a field of that variable alive.

It would have been broken in the opposite direction as well, but both
Environment and RegionStore use the scanReachableSymbols mechanism for finding
live symbols regions within their values, and due to that they accidentally
end up marking the whole chain of super-regions as live when at least one
sub-region is known to be live.

It is now a direct responsibility of SymbolReaper to maintain this invariant,
and a unit test was added in order to make sure it stays that way.

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

rdar://problem/46914108

llvm-svn: 351499

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] Fix dumping for SymbolConjured conjured at no particular statement.

llvm-svn: 344944

[analyzer] Further printing improvements: use declarations,

skip pointers whenever redundant, use unique prefixes.

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

llvm-svn: 342316