[analyzer][NFC] Migrate nonloc::ConcreteInt to use APSIntPtr (2/4) (#120436)

[analyzer] Never create Regions wrapping reference TypedValueRegions (NFCI) (#118096)

Like in the test case:
```c++
struct String {
String(const String &) {}
};

struct MatchComponent {
unsigned

[analyzer] Never create Regions wrapping reference TypedValueRegions (NFCI) (#118096)

Like in the test case:
```c++
struct String {
String(const String &) {}
};

struct MatchComponent {
unsigned numbers[2];
String prerelease;
MatchComponent(MatchComponent const &) = default;
};

MatchComponent get();
void consume(MatchComponent const &);

MatchComponent parseMatchComponent() {
MatchComponent component = get();
component.numbers[0] = 10;
component.numbers[1] = 20;
return component; // We should have no stack addr escape warning here.
}

void top() {
consume(parseMatchComponent());
}
```

When calling `consume(parseMatchComponent())` the
`parseMatchComponent()` would return a copy of a temporary of
`component`. That copy would invoke the
`MatchComponent::MatchComponent(const MatchComponent &)` ctor.

That ctor would have a (reference typed) ParamVarRegion, holding the
location (lvalue) of the object we are about to copy (&component). So
far so good, but just before evaluating the binding operation for
initializing the `numbers` field of the temporary, we evaluate the
ArrayInitLoopExpr representing the by-value elementwise copy of the
array `component.numbers`. This is represented by a LazyCompoundVal,
because we (usually) don't just copy large arrays and bind many
individual direct bindings. Rather, we take a snapshot by using a LCV.

However, notice that the LCV representing this copy would look like
this:
lazyCompoundVal{ParamVarRegion{"reference param of cctor"}.numbers}

Notice that it refers to the numbers field of a reference. It would be
much better to desugar the reference to the actual object, thus it
should be: `lazyCompoundVal{component.numbers}`

Actually, when binding the result of the ArrayInitLoopExpr to the
`temp_object.numbers` in the compiler-generated member initializer of
the cctor, we should have two individual direct bindings because this is
a "small array":
```
binding &Element{temp_object.numbers, 0} <- loadFrom(&Element{component.numbers, 0})
binding &Element{temp_object.numbers, 1} <- loadFrom(&Element{component.numbers, 1})
```
Where `loadFrom(...)` would be:
```
loadFrom(&Element{component.numbers, 0}): 10 U32b
loadFrom(&Element{component.numbers, 1}): 20 U32b
```
So the store should look like this, after PostInitializer of
`temp_object.numbers`:
```
temp_object at offset 0: 10 U32b
temp_object at offset 32: 20 U32b
```

The lesson is that it's okay to have TypedValueRegions of references as
long as we don't form subregions of those. If we ever want to refer to a
subregion of a "reference" we actually meant to "desugar" the reference
and slice a subregion of the pointee of the reference instead.

Once this canonicalization is in place, we can also drop the special
handling of references in `ProcessInitializer`, because now reference
TypedValueRegions are eagerly desugared into their referee region when
forming a subregion of it.

There should be no practical differences, but there are of course bugs
that this patch may surface.

show more ...

[NFC][analyzer] Make `invalidateRegions` accept `Stmt` instead of `Expr` (#109792)

As was reported
[here](https://github.com/llvm/llvm-project/pull/103714#pullrequestreview-2238037812),
`invalidateR

[NFC][analyzer] Make `invalidateRegions` accept `Stmt` instead of `Expr` (#109792)

As was reported
[here](https://github.com/llvm/llvm-project/pull/103714#pullrequestreview-2238037812),
`invalidateRegions` should accept `Stmt` instead of `Expr`. This
conversion is possible, since `Expr` was anyway converted back to `Stmt`
later.

This refactoring is needed to fix another FP related to use of inline
assembly. The fix would be to change `State->bindLoc` to
`state->invalidateRegions` inside inline assembly visitor, since
`bindLoc` only binds to offset 0, which is not really correct semantics
in case of inline assembly.

show more ...

[analyzer][NFC] Trivial refactoring of region invalidation (#102456)

This commit removes `invalidateRegionsImpl()`, moving its body to
`invalidateRegions(ValueList Values, ...)`, because it was a c

[analyzer][NFC] Trivial refactoring of region invalidation (#102456)

This commit removes `invalidateRegionsImpl()`, moving its body to
`invalidateRegions(ValueList Values, ...)`, because it was a completely
useless layer of indirection.

Moreover I'm fixing some strange indentation within this function body
and renaming two variables to the proper `UpperCamelCase` format.

show more ...

[analyzer] Wrap SymbolicRegions by ElementRegions before getting a FieldRegion (#85211)

Inside the ExprEngine when we process the initializers, we create a
PostInitializer program-point, which will

[analyzer] Wrap SymbolicRegions by ElementRegions before getting a FieldRegion (#85211)

Inside the ExprEngine when we process the initializers, we create a
PostInitializer program-point, which will refer to the field being
initialized, see `FieldLoc` inside `ExprEngine::ProcessInitializer`.

When a constructor (of which we evaluate the initializer-list) is
analyzed in top-level context, then the `this` pointer will be
represented by a `SymbolicRegion`, (as it should be).

This means that we will form a `FieldRegion{SymbolicRegion{.}}` as the
initialized region.

```c++
class Bear {
public:
void brum() const;
};
class Door {
public:
// PostInitializer would refer to "FieldRegion{SymRegion{this}}"
// whereas in the store and everywhere else it would be:
// "FieldRegion{ELementRegion{SymRegion{Ty*, this}, 0, Ty}".
Door() : ptr(nullptr) {
ptr->brum(); // Bug
}
private:
Bear* ptr;
};
```

We (as CSA folks) decided to avoid the creation of FieldRegions directly
of symbolic regions in the past:

https://github.com/llvm/llvm-project/commit/f8643a9b31c4029942f67d4534c9139b45173504

---

In this patch, I propose to also canonicalize it as in the mentioned
patch, into this: `FieldRegion{ElementRegion{SymbolicRegion{Ty*, .}, 0,
Ty}`

This would mean that FieldRegions will/should never simply wrap a
SymbolicRegion directly, but rather an ElementRegion that is sitting in
between.

This patch should have practically no observable effects, as the store
(due to the mentioned patch) was made resilient to this issue, but we
use `PostInitializer::getLocationValue()` for an alternative reporting,
where we faced this issue.

Note that in really rare cases it suppresses now dereference bugs, as
demonstrated in the test. It is because in the past we failed to follow
the region of the PostInitializer inside the StoreSiteFinder visitor -
because it was using this code:
```c++
// If this is a post initializer expression, initializing the region, we
// should track the initializer expression.
if (std::optional<PostInitializer> PIP =
Pred->getLocationAs<PostInitializer>()) {
const MemRegion *FieldReg = (const MemRegion *)PIP->getLocationValue();
if (FieldReg == R) {
StoreSite = Pred;
InitE = PIP->getInitializer()->getInit();
}
}
```
Notice that the equality check didn't pass for the regions I'm
canonicalizing in this patch.

Given the nature of this change, we would rather upstream this patch.

CPP-4954

show more ...

[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] Remove unnecessary casts around Allocate function calls

Reviewed By: steakhal

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

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

[analyzer] Process non-POD array element destructors

The constructors of non-POD array elements are evaluated under
certain conditions. This patch makes sure that in such cases
we also evaluate the

[analyzer] Process non-POD array element destructors

The constructors of non-POD array elements are evaluated under
certain conditions. This patch makes sure that in such cases
we also evaluate the destructors.

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

show more ...

[clang][clang-tools-extra] LLVM_NODISCARD => [[nodiscard]]. NFC

[analyzer][NFC] Prefer using isa<> instead getAs<> in conditions

Depends on D125709

Reviewed By: martong

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

[analyzer] Replace adjacent assumeInBound calls to assumeInBoundDual

This is to minimize superfluous assume calls.

Depends on D124758

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

[analyzer] Indicate if a parent state is infeasible

In some cases a parent State is already infeasible, but we recognize
this only if an additonal constraint is added. This patch is the first
of a s

[analyzer] Indicate if a parent state is infeasible

In some cases a parent State is already infeasible, but we recognize
this only if an additonal constraint is added. This patch is the first
of a series to address this issue. In this patch `assumeDual` is changed
to clone the parent State but with an `Infeasible` flag set, and this
infeasible-parent is returned both for the true and false case. Then
when we add a new transition in the exploded graph and the destination
is marked as infeasible, the node will be a sink node.

Related bug:
https://github.com/llvm/llvm-project/issues/50883
Actually, this patch does not solve that bug in the solver, rather with
this patch we can handle the general parent-infeasible cases.

Next step would be to change the State API and require all checkers to
use the `assume*Dual` API and deprecate the simple `assume` calls.

Hopefully, the next patch will introduce `assumeInBoundDual` and will
solve the CRASH we have here:
https://github.com/llvm/llvm-project/issues/54272

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

show more ...

[clang] Remove redundant member initialization (NFC)

Identified with readability-redundant-member-init.

[analyzer] Simplify function SVal::getAsSymbolicExpression and similar ones

Summary: Simplify functions SVal::getAsSymbolicExpression SVal::getAsSymExpr and SVal::getAsSymbol. After revision I concl

[analyzer] Simplify function SVal::getAsSymbolicExpression and similar ones

Summary: Simplify functions SVal::getAsSymbolicExpression SVal::getAsSymExpr and SVal::getAsSymbol. After revision I concluded that `getAsSymbolicExpression` and `getAsSymExpr` repeat functionality of `getAsSymbol`, thus them can be removed.

Fix: Remove functions SVal::getAsSymbolicExpression and SVal::getAsSymExpr.

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

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] Track runtime types represented by Obj-C Class objects

Summary:
Objective-C Class objects can be used to do a dynamic dispatch on
class methods. The analyzer had a few places where we tri

[analyzer] Track runtime types represented by Obj-C Class objects

Summary:
Objective-C Class objects can be used to do a dynamic dispatch on
class methods. The analyzer had a few places where we tried to overcome
the dynamic nature of it and still guess the actual function that
is being called. That was done mostly using some simple heuristics
covering the most widespread cases (e.g. [[self class] classmethod]).
This solution introduces a way to track types represented by Class
objects and work with that instead of direct AST matching.

rdar://problem/50739539

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

show more ...

Revert "[analyzer] Keep track of escaped locals"

It was a step in the right direction but it is not clear how can this
fit into the checker API at this point. The pre-escape happens in the
analyzer

Revert "[analyzer] Keep track of escaped locals"

It was a step in the right direction but it is not clear how can this
fit into the checker API at this point. The pre-escape happens in the
analyzer core and the checker has no control over it. If the checker
is not interestd in a pre-escape it would need to do additional work
on each escape to check if the escaped symbol is originated from an
"uninteresting" pre-escaped memory region. In order to keep the
checker API simple we abandoned this solution for now.

We will reland this once we have a better answer for what to do on the
checker side.

This reverts commit f3a28202ef58551db15818f8f51afd21e0f3e231.

show more ...

[analyzer] Keep track of escaped locals

We want to escape all symbols that are stored into escaped regions.
The problem is, we did not know which local regions were escaped. Until now.
This should f

[analyzer] Keep track of escaped locals

We want to escape all symbols that are stored into escaped regions.
The problem is, we did not know which local regions were escaped. Until now.
This should fix some false positives like the one in the tests.

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

show more ...

[analyzer] NFC: Don't clean up range constraints twice.

Slightly improves static analysis speed.

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

[analyzer] CastValueChecker: Store the dynamic types and casts

Summary:
This patch introduces `DynamicCastInfo` similar to `DynamicTypeInfo` which
is stored in `CastSets` which are storing the dynam

[analyzer] CastValueChecker: Store the dynamic types and casts

Summary:
This patch introduces `DynamicCastInfo` similar to `DynamicTypeInfo` which
is stored in `CastSets` which are storing the dynamic cast informations of
objects based on memory regions. It could be used to store and check the
casts and prevent infeasible paths.

Reviewed By: NoQ

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

llvm-svn: 369605

show more ...

[analyzer] print() JSONify: Program state implementation

Summary: -

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, a.sidor

[analyzer] print() JSONify: Program state implementation

Summary: -

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy,
dkrupp

Tags: #clang

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

llvm-svn: 361983

show more ...

[analyzer] print() JSONify: Constructing objects implementation

Summary: -

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs,

[analyzer] print() JSONify: Constructing objects implementation

Summary: -

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy,
dkrupp

Tags: #clang

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

llvm-svn: 361980

show more ...

[analyzer] print() JSONify: Type information implementation

Summary: -

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, a.si

[analyzer] print() JSONify: Type information implementation

Summary: -

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy,
dkrupp

Tags: #clang

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

llvm-svn: 361979

show more ...