[clang][dataflow] Fix bug in `buildContainsExprConsumedInDifferentBlock()`. (#100874)

This was missing a call to `ignoreCFGOmittedNodes()`. As a result, the
function
would erroneously conclude that

[clang][dataflow] Fix bug in `buildContainsExprConsumedInDifferentBlock()`. (#100874)

This was missing a call to `ignoreCFGOmittedNodes()`. As a result, the
function
would erroneously conclude that a block did not contain an expression
consumed
in a different block if the expression in question was surrounded by a
`ParenExpr` in the consuming block. The patch adds a test that triggers
this
scenario (and fails without the fix).

To prevent this kind of bug in the future, the patch also adds a new
method
`blockForStmt()` to `AdornedCFG` that calls `ignoreCFGOmittedNodes()`
and is
preferred over accessing `getStmtToBlock()` directly.

show more ...

[clang][nullability] Improve modeling of `++`/`--` operators. (#96601)

We definitely know that these operations change the value of their
operand, so
clear out any value associated with it. We don't

[clang][nullability] Improve modeling of `++`/`--` operators. (#96601)

We definitely know that these operations change the value of their
operand, so
clear out any value associated with it. We don't create a new value,
instead
leaving it to the analysis to do this if desired.

show more ...

[clang][dataflow] Propagate storage location of compound assignment operators. (#94332)

To avoid generating unnecessary values, we don't create a new value but
instead
leave it to the specific analy

[clang][dataflow] Propagate storage location of compound assignment operators. (#94332)

To avoid generating unnecessary values, we don't create a new value but
instead
leave it to the specific analysis to do this if desired.

show more ...

[clang][nullability] Propagate storage location / value of `++`/`--` operators. (#94217)

To avoid generating unnecessary values, we don't create a new value but
instead
leave it to the specific anal

[clang][nullability] Propagate storage location / value of `++`/`--` operators. (#94217)

To avoid generating unnecessary values, we don't create a new value but
instead
leave it to the specific analysis to do this if desired.

show more ...

[clang][dataflow] Strengthen pointer comparison. (#75170)

- Instead of comparing the identity of the `PointerValue`s, compare the
underlying `StorageLocation`s.

- If the `StorageLocation`s a

[clang][dataflow] Strengthen pointer comparison. (#75170)

- Instead of comparing the identity of the `PointerValue`s, compare the
underlying `StorageLocation`s.

- If the `StorageLocation`s are the same, return a definite "true" as
the
result of the comparison. Before, if the `PointerValue`s were different,
we
would return an atom, even if the storage locations themselves were the
same.

- If the `StorageLocation`s are different, return an atom (as before).
Pointers
that have different storage locations may still alias, so we can't
return a
definite "false" in this case.

The application-level gains from this are relatively modest. For the
Crubit
nullability check running on an internal codebase, this change reduces
the
number of functions on which the SAT solver times out from 223 to 221;
the
number of "pointer expression not modeled" errors reduces from 3815 to
3778.

Still, it seems that the gain in precision is generally worthwhile.

@Xazax-hun inspired me to think about this with his

[comments](https://github.com/llvm/llvm-project/pull/73860#pullrequestreview-1761484615)
on a different PR.

show more ...

[clang][dataflow] Fix crash when `operator=` result type is not destination type. (#90898)

The existing code was full of comments about how we assume this is
always the
case, but it's not mandated b

[clang][dataflow] Fix crash when `operator=` result type is not destination type. (#90898)

The existing code was full of comments about how we assume this is
always the
case, but it's not mandated by the standard, and there is code out there
that
returns a different type. So check that the result type is in fact the
same as
the destination type before attempting to copy to the result.

To make sure that we don't bail out in more cases than intended, I've
extended
existing tests to verify that in the common case, we do return the
destination
object (by reference or value, as the case may be).

show more ...

[clang][dataflow] Fix crash when `ConstantExpr` is used in conditional operator. (#90112)

`ConstantExpr` does not appear as a `CFGStmt` in the CFG, so
`StmtToEnvMap::getEnvironment()` was not findin

[clang][dataflow] Fix crash when `ConstantExpr` is used in conditional operator. (#90112)

`ConstantExpr` does not appear as a `CFGStmt` in the CFG, so
`StmtToEnvMap::getEnvironment()` was not finding an entry for it in the
map,
causing a crash when we tried to access the iterator resulting from the
map
lookup.

The fix is to make `ignoreCFGOmittedNodes()` ignore `ConstantExpr`, but
in
addition, I'm hardening `StmtToEnvMap::getEnvironment()` to make sure
release
builds don't crash in similar situations in the future.

show more ...

Reapply "[clang][dataflow] Model conditional operator correctly." with fixes (#89596)

I reverted https://github.com/llvm/llvm-project/pull/89213 beause it was
causing buildbots to fail with assertio

Reapply "[clang][dataflow] Model conditional operator correctly." with fixes (#89596)

I reverted https://github.com/llvm/llvm-project/pull/89213 beause it was
causing buildbots to fail with assertion failures.

Embarrassingly, it turns out I had been running tests locally in
`Release` mode, i.e. with `assert()` compiled away.

This PR re-lands #89213 with fixes for the failing assertions.

show more ...

Revert "[clang][dataflow] Model conditional operator correctly." (#89577)

Reverts llvm/llvm-project#89213

This is causing buildbot failures.

[clang][dataflow] Model conditional operator correctly. (#89213)

[clang][nullability] Remove `RecordValue`. (#89052)

This class no longer serves any purpose; see also the discussion here:
https://reviews.llvm.org/D155204#inline-1503204

A lot of existing tests in

[clang][nullability] Remove `RecordValue`. (#89052)

This class no longer serves any purpose; see also the discussion here:
https://reviews.llvm.org/D155204#inline-1503204

A lot of existing tests in TransferTest.cpp check for the existence of
`RecordValue`s. Some of these checks are now simply redundant and have
been
removed. In other cases, tests were checking for the existence of a
`RecordValue` as a way of testing whether a record has been initialized.
I have
typically changed these test to instead check whether a field of the
record has
a value.

show more ...

[clang][dataflow] Expose getReferencedDecls and relocate free functions. (#88754)

Moves free functions from DataflowEnvironment.h/cc and
DataflowAnalysisContext.h/cc to RecordOps and a new ASTOps a

[clang][dataflow] Expose getReferencedDecls and relocate free functions. (#88754)

Moves free functions from DataflowEnvironment.h/cc and
DataflowAnalysisContext.h/cc to RecordOps and a new ASTOps and exposes
them as needed for current use and to expose getReferencedDecls for
out-of-tree use.

Minimal change in functionality, only to modify the return type of
getReferenceDecls to return the collected decls instead of using output
params.

Tested with `ninja check-clang-tooling`.

show more ...

[clang][dataflow] Reland #87320: Propagate locations from result objects to initializers. (#88316)

This relands #87320 and additionally removes the now-unused function
`isOriginalRecordConstructor()

[clang][dataflow] Reland #87320: Propagate locations from result objects to initializers. (#88316)

This relands #87320 and additionally removes the now-unused function
`isOriginalRecordConstructor()`, which was causing buildbots to fail.

show more ...

Revert "[clang][dataflow] Propagate locations from result objects to initializers." (#88315)

Reverts llvm/llvm-project#87320

This is causing buildbots to fail because
`isOriginalRecordConstructo

Revert "[clang][dataflow] Propagate locations from result objects to initializers." (#88315)

Reverts llvm/llvm-project#87320

This is causing buildbots to fail because
`isOriginalRecordConstructor()` is now unused.

show more ...

[clang][dataflow] Propagate locations from result objects to initializers. (#87320)

Previously, we were propagating storage locations the other way around,
i.e.
from initializers to result objects,

[clang][dataflow] Propagate locations from result objects to initializers. (#87320)

Previously, we were propagating storage locations the other way around,
i.e.
from initializers to result objects, using `RecordValue::getLoc()`. This
gave
the wrong behavior in some cases -- see the newly added or fixed tests
in this
patch.

In addition, this patch now unblocks removing the `RecordValue` class
entirely,
as we no longer need `RecordValue::getLoc()`.

With this patch, the test `TransferTest.DifferentReferenceLocInJoin`
started to
fail because the framework now always uses the same storge location for
a
`MaterializeTemporaryExpr`, meaning that the code under test no longer
set up
the desired state where a variable of reference type is mapped to two
different
storage locations in environments being joined. Rather than trying to
modify
this test to set up the test condition again, I have chosen to replace
the test
with an equivalent test in DataflowEnvironmentTest.cpp that sets up the
test
condition directly; because this test is more direct, it will also be
less
brittle in the face of future changes.

show more ...

[clang][dataflow] Introduce a helper class for handling record initializer lists. (#86675)

This is currently only used in one place, but I'm working on a patch
that will
use this from a second place

[clang][dataflow] Introduce a helper class for handling record initializer lists. (#86675)

This is currently only used in one place, but I'm working on a patch
that will
use this from a second place. And I think this already improves the
readability
of the one place this is used so far.

show more ...

[clang][dataflow] Model assignment to derived class from base. (#85064)

This is a relatively rare case, but

- It's still nice to get this right,
- We can remove the special case for this in
`VisitC

[clang][dataflow] Model assignment to derived class from base. (#85064)

This is a relatively rare case, but

- It's still nice to get this right,
- We can remove the special case for this in
`VisitCXXOperatorCallExpr()` (that
simply bails out), and
- With this in place, I can avoid having to add a similar special case
in an
upcoming patch.

show more ...

[clang][dataflow][NFC] Rename `ControlFlowContext` to `AdornedCFG`. (#85640)

This expresses better what the class actually does, and it reduces the
number of
`Context`s that we have in the codebase.

[clang][dataflow][NFC] Rename `ControlFlowContext` to `AdornedCFG`. (#85640)

This expresses better what the class actually does, and it reduces the
number of
`Context`s that we have in the codebase.

A deprecated alias `ControlFlowContext` is available from the old
header.

show more ...

[clang][dataflow] Fix `getResultObjectLocation()` on `CXXDefaultArgExpr`. (#85072)

This patch includes a test that causes an assertion failure without the
other
changes in this patch.

[clang][dataflow] Correctly treat empty initializer lists for unions. (#82986)

This fixes a crash introduced by
https://github.com/llvm/llvm-project/pull/82348
but also adds additional handling to m

[clang][dataflow] Correctly treat empty initializer lists for unions. (#82986)

This fixes a crash introduced by
https://github.com/llvm/llvm-project/pull/82348
but also adds additional handling to make sure that we treat empty
initializer
lists for both unions and structs/classes correctly (see tests added in
this
patch).

show more ...

[clang][dataflow] Skip array types when handling InitListExprs. (#83013)

Crashes resulted from single-element InitListExprs for arrays with
elements of a record type after #80970.

Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (#82856)

Reverts llvm/llvm-project#82348, which caused crashes when analyzing
empty InitListExprs for unions, e.g.

```cc

Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (#82856)

Reverts llvm/llvm-project#82348, which caused crashes when analyzing
empty InitListExprs for unions, e.g.

```cc
union U {
double double_value;
int int_value;
};

void target() {
U value;
value = {};
}
```

Co-authored-by: Samira Bazuzi <bazuzi@users.noreply.github.com>

show more ...

[clang][dataflow] Correctly handle `InitListExpr` of union type. (#82348)

[clang][dataflow][NFC] Add a FIXME to handling of union initialization. (#82239)

We want to make it clear that the current behavior doesn't yet handle
unions
properly.

[clang][dataflow] Fix crash on unions introduced in ba279934c6ab09d5394a89d8318651aefd8d565b (#81918)

The commit was itself a crash fix, but inadvertently changed the
behavior for unions, which res

[clang][dataflow] Fix crash on unions introduced in ba279934c6ab09d5394a89d8318651aefd8d565b (#81918)

The commit was itself a crash fix, but inadvertently changed the
behavior for unions, which results in crashes.

show more ...