[clang][dataflow] Eliminate `SkipPast::ReferenceThenPointer`.

As a replacement, we provide the accessors `getImplicitObjectLocation()` and
`getBaseObjectLocation()`, which are higher-level construct

[clang][dataflow] Eliminate `SkipPast::ReferenceThenPointer`.

As a replacement, we provide the accessors `getImplicitObjectLocation()` and
`getBaseObjectLocation()`, which are higher-level constructs that cover the use
cases in which `SkipPast::ReferenceThenPointer` was typically used.

Unfortunately, it isn't possible to use these accessors in
UncheckedOptionalAccessModel.cpp; I've added a FIXME to the code explaining the
details. I initially attempted to resolve the issue as part of this patch, but
it turned out to be non-trivial to fix. Instead, I have therefore added a
lower-level replacement for `SkipPast::ReferenceThenPointer` that is used only
within this file.

The wider context of this change is that `SkipPast` will be going away entirely.
See also the RFC at https://discourse.llvm.org/t/70086.

Reviewed By: ymandel, gribozavr2

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

show more ...

[clang][dataflow][NFC] Remove `SkipPast` param from `getValue(const ValueDecl &)`.

This parameter was already a no-op, so removing it doesn't change behavior.

Reviewed By: ymandel

Differential Rev

[clang][dataflow][NFC] Remove `SkipPast` param from `getValue(const ValueDecl &)`.

This parameter was already a no-op, so removing it doesn't change behavior.

Reviewed By: ymandel

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

show more ...

[clang][dataflow][NFC] Remove `SkipPast` parameter from `getStorageLocation(const ValueDecl &).

This parameter was already a no-op, so removing it doesn't change behavior.

Depends On D149144

Revie

[clang][dataflow][NFC] Remove `SkipPast` parameter from `getStorageLocation(const ValueDecl &).

This parameter was already a no-op, so removing it doesn't change behavior.

Depends On D149144

Reviewed By: ymandel, xazax.hun, gribozavr2

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

show more ...

[clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

For the wider context of this change, see the RFC at
https://discourse.llvm.org/t/70086.

After this change,

[clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

For the wider context of this change, see the RFC at
https://discourse.llvm.org/t/70086.

After this change, global and local variables of reference type are associated
directly with the `StorageLocation` of the referenced object instead of the
`StorageLocation` of a `ReferenceValue`.

Some tests that explicitly check for an existence of `ReferenceValue` for a
variable of reference type have been modified accordingly.

As discussed in the RFC, I have added an assertion to `Environment::join()` to
check that if both environments contain an entry for the same declaration in
`DeclToLoc`, they both map to the same `StorageLocation`. As discussed in
https://discourse.llvm.org/t/70086/5, this also necessitates removing
declarations from `DeclToLoc` when they go out of scope.

In the RFC, I proposed a gradual migration for this change, but it appears
that all of the callers of `Environment::setStorageLocation(const ValueDecl &,
SkipPast` are in the dataflow framework itself, and that there are only a few of
them.

As this is the function whose semantics are changing in a way that callers
potentially need to adapt to, I've decided to change the semantics of the
function directly.

The semantics of `getStorageLocation(const ValueDecl &, SkipPast SP` now no
longer depend on the behavior of the `SP` parameter. (There don't appear to be
any callers that use `SkipPast::ReferenceThenPointer`, so I've added an
assertion that forbids this usage.)

This patch adds a default argument for the `SP` parameter and removes the
explicit `SP` argument at the callsites that are touched by this change. A
followup patch will remove the argument from the remaining callsites,
allowing the `SkipPast` parameter to be removed entirely. (I don't want to do
that in this patch so that semantics-changing changes can be reviewed separately
from semantics-neutral changes.)

Reviewed By: ymandel, xazax.hun, gribozavr2

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

show more ...

[clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

This will eliminate the need for more pass-through APIs. Also replace pass-through usages with this exposure.

Reviewed By:

[clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

This will eliminate the need for more pass-through APIs. Also replace pass-through usages with this exposure.

Reviewed By: ymandel, gribozavr2, xazax.hun

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

show more ...

[dataflow] Don't crash in Environment::dump when decls have weird names

[dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext

DataflowAnalysisContext has a few too many responsibilities, this narrows them.

It also allows the Arena to be shar

[dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext

DataflowAnalysisContext has a few too many responsibilities, this narrows them.

It also allows the Arena to be shared with analysis steps, which need to create
Values, without exposing the whole DACtx API (flow conditions etc).
This means Environment no longer needs to proxy all these methods.
(For now it still does, because there are many callsites to update, and maybe
if we separate bool formulas from values we can avoid churning them twice)

In future, if we untangle the concepts of Values from boolean formulas/atoms,
Arena would also be responsible for creating formulas and managing atom IDs.

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

show more ...

[clang][dataflow] Associate `FunctionToPointerDecay` nodes with a value.

To ensure that we have a pointee for the `PointerValue`, we also create
storage locations for `FunctionDecl`s referenced in t

[clang][dataflow] Associate `FunctionToPointerDecay` nodes with a value.

To ensure that we have a pointee for the `PointerValue`, we also create
storage locations for `FunctionDecl`s referenced in the function under analysis.

Reviewed By: gribozavr2

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

show more ...

[clang][dataflow][NFC] Remove unused parameter from `insertIfGlobal()`.

Reviewed By: gribozavr2

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

[clang][dataflow] Use `isRecordType()` where appropriate.

This is less verbose than checking for class, struct, and union individually,
and I believe it's also more efficient (not that that should b

[clang][dataflow] Use `isRecordType()` where appropriate.

This is less verbose than checking for class, struct, and union individually,
and I believe it's also more efficient (not that that should be the overriding
concern).

Reviewed By: sammccall, xazax.hun

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

show more ...

[clang][dataflow] Eliminate code duplication in Environment::createValueUnlessSelfReferential().

Reviewed By: sammccall

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

[clang][dataflow] Add `create<T>()` methods to `Environment` and `DataflowAnalysisContext`.

These methods provide a less verbose way of allocating `StorageLocation`s and
`Value`s than the existing `

[clang][dataflow] Add `create<T>()` methods to `Environment` and `DataflowAnalysisContext`.

These methods provide a less verbose way of allocating `StorageLocation`s and
`Value`s than the existing `takeOwnership(make_unique(...))` pattern.

In addition, because allocation of `StorageLocation`s and `Value`s now happens
within the `DataflowAnalysisContext`, the `create<T>()` open up the possibility
of using `BumpPtrAllocator` to allocate these objects if it turns out this
helps performance.

Reviewed By: ymandel, xazax.hun, gribozavr2

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

show more ...

[clang][dataflow][NFC] Share code between Environment ctor and pushCallInternal().

The deduplicated code is moved into initVars().

As an added bonus, pushCallInternal() now also gets the "Add all f

[clang][dataflow][NFC] Share code between Environment ctor and pushCallInternal().

The deduplicated code is moved into initVars().

As an added bonus, pushCallInternal() now also gets the "Add all fields
mentioned in default member initializers" behavior, which apparently had been
added to the Environment ctor but not pushCallInternal().

Reviewed By: xazax.hun, ymandel

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

show more ...

[dataflow] Log flow condition to the correct stream.

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

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

[clang][dataflow] Fix missed fields in field set construction.

When building the set of referenced fields for the `DataflowAnalysisContext`,
include fields referenced only in default member initiali

[clang][dataflow] Fix missed fields in field set construction.

When building the set of referenced fields for the `DataflowAnalysisContext`,
include fields referenced only in default member initializers. These
initializers are visited in the CFGs of constructors and so the fields must be
included when analysing constructor bodies.

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

show more ...

[clang][dataflow] Fix bug in joining bool values.

Currently, the code assumes that all boolean-typed values are an instance of
`BoolValue` (or its subclasses). Yet, lvalues violate this assumption.

[clang][dataflow] Fix bug in joining bool values.

Currently, the code assumes that all boolean-typed values are an instance of
`BoolValue` (or its subclasses). Yet, lvalues violate this assumption. This
patch drops the assumption and strengthens the check to confirm the shape of
both values being joined.

The patch also notes as FIXMES a number of problems discovered fixing this bug.

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

show more ...

[clang][dataflow] Add (initial) debug printing for `Value` and `Environment`.

Also adds uses of the new printing in analysis inner loop.

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

[clang][dataflow] Fix 2 bugs in `MemberExpr` interpretation.

There were two (small) bugs causing crashes in the analysis. This patch fixes both of them.

1. An enum value was accessed as a class me

[clang][dataflow] Fix 2 bugs in `MemberExpr` interpretation.

There were two (small) bugs causing crashes in the analysis. This patch fixes both of them.

1. An enum value was accessed as a class member. Now, the engine gracefully
ignores such member expressions.

2. Field access in `MemberExpr` of struct/class-typed global variables. Analysis
didn't interpret fields of global vars, because the vars were initialized before
the fields were added to the "allowlist". Now, the allowlist is set _before_
init of globals.

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

show more ...

[clang][dataflow][NFC] Refine names and comments for field filtering.

Tweaks elements of the new API for filtering the set of modeled fields.

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

Revert "Revert "[clang][dataflow] Only model struct fields that are used in the function being analyzed.""

This reverts commit 2b1a517a92bfdfa3b692a660e19a2bb22513a567. It's a fix forward
with two m

Revert "Revert "[clang][dataflow] Only model struct fields that are used in the function being analyzed.""

This reverts commit 2b1a517a92bfdfa3b692a660e19a2bb22513a567. It's a fix forward
with two memory errors fixed, one of which was the cause of the build breakage
in the buildbots.

Original message:

Previously, the model for structs modeled all fields in a struct when
`createValue` was called for that type. This patch adds a prepass on the
function under analysis to discover the fields referenced in the scope and then
limits modeling to only those fields. This reduces wasted memory usage
(modeling unused fields) which can be important for programs that use large
structs.

Note: This patch obviates the need for https://reviews.llvm.org/D123032.

show more ...

Move from llvm::makeArrayRef to ArrayRef deduction guides - clang/ part

This is a follow-up to https://reviews.llvm.org/D140896, split into
several parts as it touches a lot of files.

Differential

Move from llvm::makeArrayRef to ArrayRef deduction guides - clang/ part

This is a follow-up to https://reviews.llvm.org/D140896, split into
several parts as it touches a lot of files.

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

show more ...

Revert "[clang][dataflow] Only model struct fields that are used in the function being analyzed."

This reverts commit 5e8f597c2fedc740b71f07dfdb1ef3c2d348b193. It caused msan and ubsan breakages.

[clang][dataflow] Only model struct fields that are used in the function being analyzed.

Previously, the model for structs modeled all fields in a struct when
`createValue` was called for that type.

[clang][dataflow] Only model struct fields that are used in the function being analyzed.

Previously, the model for structs modeled all fields in a struct when
`createValue` was called for that type. This patch adds a prepass on the
function under analysis to discover the fields referenced in the scope and then
limits modeling to only those fields. This reduces wasted memory usage
(modeling unused fields) which can be important for programss that use large
structs.

Note: This patch obviates the need for https://reviews.llvm.org/D123032.

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

show more ...

[clang][dataflow] Treat unions as structs.

This is a straightfoward way to handle unions in dataflow analysis. Without this change, nullability verification crashes on files that contain unions.

Re

[clang][dataflow] Treat unions as structs.

This is a straightfoward way to handle unions in dataflow analysis. Without this change, nullability verification crashes on files that contain unions.

Reviewed By: gribozavr2, ymandel

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

show more ...