[clang][dataflow] Use smart pointer caching in unchecked optional accessor (#120249)

Part 2 (and final part) following
https://github.com/llvm/llvm-project/pull/120102
Allows users to do things li

[clang][dataflow] Use smart pointer caching in unchecked optional accessor (#120249)

Part 2 (and final part) following
https://github.com/llvm/llvm-project/pull/120102
Allows users to do things like:

```
if (o->x.has_value()) {
((*o).x).value();
}
```
where the `->` and `*` are operator overload calls.

A user could instead extract the nested optional into a local variable
once instead of doing two accessor calls back to back, but currently
they are unsure why the code is flagged.

show more ...

[clang][dataflow] Cache accessors returning pointers in bugprone-unchecked-optional-access (#113922)

Previously, we covered returning refs, or copies of optional, and bools.
Now cover returning poi

[clang][dataflow] Cache accessors returning pointers in bugprone-unchecked-optional-access (#113922)

Previously, we covered returning refs, or copies of optional, and bools.
Now cover returning pointers (to any type).
This is useful for cases like operator-> of smart pointers.
Addresses more of issue llvm#58510

show more ...

[clang][dataflow] Don't clear cached field state if field is const (#113698)

... in the unchecked optional access model.

[clang] Use {} instead of std::nullopt to initialize empty ArrayRef (#109399)

Follow up to #109133.

[clang][dataflow] Cache accessors for bugprone-unchecked-optional-access (#112605)

Treat calls to zero-param const methods as having stable return values
(with a cache) to address issue #58510. The

[clang][dataflow] Cache accessors for bugprone-unchecked-optional-access (#112605)

Treat calls to zero-param const methods as having stable return values
(with a cache) to address issue #58510. The cache is invalidated when
non-const methods are called. This uses the infrastructure from PR
#111006.

For now we cache methods returning:
- ref to optional
- optional by value
- booleans

We can extend that to pointers to optional in a next change.

show more ...

[clang][dataflow] Add a test demonstrating an issue in unchecked-optional-access-check (#110870)

createStorageLocation used in transferCallReturningOptional:

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

[clang][dataflow] Add a test demonstrating an issue in unchecked-optional-access-check (#110870)

createStorageLocation used in transferCallReturningOptional:

https://github.com/llvm/llvm-project/blob/09ba83be0ac178851e3c9c9c8fefddbdd4d8353f/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp#L515
can stop recursively creating storage locations when it hits a field of
reference type for a non-optional record:

https://github.com/llvm/llvm-project/blob/3ca5d8082a0c6bd9520544ce3bca11bf3e02a5fa/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp#L67

If an optional is reached through that field then it may not have a
storage location by the type we handle has_value in a transfer function.

show more ...

[clang][dataflow] Add a callback run on the pre-transfer state. (#96140)

At the same time, rename `PostVisitCFG` to the more descriptive
`PostAnalysisCallbacks` (which emphasizes the fact that thes

[clang][dataflow] Add a callback run on the pre-transfer state. (#96140)

At the same time, rename `PostVisitCFG` to the more descriptive
`PostAnalysisCallbacks` (which emphasizes the fact that these callbacks
are run
after the dataflow analysis itself has converged).

Before this patch, it was only possible to run a callback on the state
_after_
the transfer function had been applied, but for many analyses, it's more
natural
to to check the state _before_ the transfer function has been applied,
because we
are usually checking the preconditions for some operation. Some checks
are
impossible to perform on the "after" state because we can no longer
check the
precondition; for example, the `++` / `--` operators on raw pointers
require the
operand to be nonnull, but after the transfer function for the operator
has been
applied, the original value of the pointer can no longer be accessed.

`UncheckedOptionalAccessModelTest` has been modified to run the
diagnosis
callback on the "before" state. In this particular case, diagnosis can
be run
unchanged on either the "before" or "after" state, but we want this test
to
demonstrate that running diagnosis on the "before" state is usually the
preferred approach.

This change is backwards-compatible; all existing analyses will continue
to run
the callback on the "after" state.

show more ...

[Clang] [Sema] Diagnose unknown std::initializer_list layout in SemaInit (#95580)

This checks if the layout of `std::initializer_list` is something Clang
can handle much earlier and deduplicates th

[Clang] [Sema] Diagnose unknown std::initializer_list layout in SemaInit (#95580)

This checks if the layout of `std::initializer_list` is something Clang
can handle much earlier and deduplicates the checks in
CodeGen/CGExprAgg.cpp and AST/ExprConstant.cpp

Also now diagnose `union initializer_list` (Fixes #95495), bit-field for
the size (Fixes a crash that would happen during codegen if it were
unnamed), base classes (that wouldn't be initialized) and polymorphic
classes (whose vtable pointer wouldn't be initialized).

show more ...

[clang][dataflow] Fix for value constructor in class derived from optional. (#86942)

The constructor `Derived(int)` in the newly added test
`ClassDerivedFromOptionalValueConstructor` is not a templa

[clang][dataflow] Fix for value constructor in class derived from optional. (#86942)

The constructor `Derived(int)` in the newly added test
`ClassDerivedFromOptionalValueConstructor` is not a template, and this
used to
cause an assertion failure in `valueOrConversionHasValue()` because
`F.getTemplateSpecializationArgs()` returns null.

(This is modeled after the `MaybeAlign(Align Value)` constructor, which
similarly causes an assertion failure in the analysis when assigning an
`Align`
to a `MaybeAlign`.)

To fix this, we can simply look at the type of the destination type
which we're
constructing or assigning to (instead of the function template
argument), and
this not only fixes this specific case but actually simplifies the
implementation.

I've added some additional tests for the case of assigning to a nested
optional
because we didn't have coverage for these and I wanted to make sure I
didn't
break anything.

show more ...

[clang][dataflow] Make optional checker work for types derived from optional. (#84138)

`llvm::MaybeAlign` does this, for example.

It's not an option to simply ignore these derived classes because

[clang][dataflow] Make optional checker work for types derived from optional. (#84138)

`llvm::MaybeAlign` does this, for example.

It's not an option to simply ignore these derived classes because they
get cast
back to the optional classes (for example, simply when calling the
optional
member functions), and our transfer functions will then run on those
optional
classes and therefore require them to be properly initialized.

show more ...

[clang][dataflow] Add support for `CXXRewrittenBinaryOperator`. (#81086)

This occurs in rewritten candidates for binary operators (a C++20
feature).

The patch modifies UncheckedOptionalAccessModelT

[clang][dataflow] Add support for `CXXRewrittenBinaryOperator`. (#81086)

This occurs in rewritten candidates for binary operators (a C++20
feature).

The patch modifies UncheckedOptionalAccessModelTest to run in C++20 mode
(as
well as C++17 mode, as before) and to use rewritten candidates. The
modified
test fails without the newly added support for
`CXXRewrittenBinaryOperator`.

show more ...

[clang][dataflow] Add synthetic fields to `RecordStorageLocation` (#73860)

Synthetic fields are intended to model the internal state of a class
(e.g. the value stored in a `std::optional`) without

[clang][dataflow] Add synthetic fields to `RecordStorageLocation` (#73860)

Synthetic fields are intended to model the internal state of a class
(e.g. the value stored in a `std::optional`) without having to depend on
that class's implementation details.

Today, this is typically done with properties on `RecordValue`s, but
these have several drawbacks:

* Care must be taken to call `refreshRecordValue()` before modifying a
property so that the modified property values aren’t seen by other
environments that may have access to the same `RecordValue`.

* Properties aren’t associated with a storage location. If an analysis
needs to associate a location with the value stored in a property (e.g.
to model the reference returned by `std::optional::value()`), it needs
to manually add an indirection using a `PointerValue`. (See for example
the way this is done in UncheckedOptionalAccessModel.cpp, specifically
in `maybeInitializeOptionalValueMember()`.)

* Properties don’t participate in the builtin compare, join, and widen
operations. If an analysis needs to apply these operations to
properties, it needs to override the corresponding methods of
`ValueModel`.

* Longer-term, we plan to eliminate `RecordValue`, as by-value
operations on records aren’t really “a thing” in C++ (see
https://discourse.llvm.org/t/70086#changed-structvalue-api-14). This
would obviously eliminate the ability to set properties on
`RecordValue`s.

To demonstrate the advantages of synthetic fields, this patch converts
UncheckedOptionalAccessModel.cpp to synthetic fields. This greatly
simplifies the implementation of the check.

This PR is pretty big; to make it easier to review, I have broken it
down into a stack of three commits, each of which contains a set of
logically related changes. I considered submitting each of these as a
separate PR, but the commits only really make sense when taken together.

To review, I suggest first looking at the changes in
UncheckedOptionalAccessModel.cpp. This gives a flavor for how the
various API changes work together in the context of an analysis. Then,
review the rest of the changes.

show more ...

[clang][dataflow]Use cast_or_null instead of cast to prevent crash (#68510)

`getStorageLocation` may return `nullptr` and this will produce crash
when use `cast`, use `dyn_cast_or_null` instead. I

[clang][dataflow]Use cast_or_null instead of cast to prevent crash (#68510)

`getStorageLocation` may return `nullptr` and this will produce crash
when use `cast`, use `dyn_cast_or_null` instead. I test it locally using
[FTXUI](https://github.com/ArthurSonzogni/FTXUI) and it may be the cause
of issue [issue](https://github.com/llvm/llvm-project/issues/68412), but
I am not sure.

Co-authored-by: huqizhi <huqizhi@836744285@qq.com>

show more ...

[clang][dataflow] Add support for lambda captures (#68558)

This adds support for copy, ref, and this lambda captures to the core
framework and also adds relevant tests in UncheckedOptionalAccessTes

[clang][dataflow] Add support for lambda captures (#68558)

This adds support for copy, ref, and this lambda captures to the core
framework and also adds relevant tests in UncheckedOptionalAccessTest.

show more ...

[clang-tidy] Update unchecked-optiona-access-check to use convenience function for diagnosing `FunctionDecl`s.

Also changes code in the underlying model to fit the type expected by the convenience f

[clang-tidy] Update unchecked-optiona-access-check to use convenience function for diagnosing `FunctionDecl`s.

Also changes code in the underlying model to fit the type expected by the convenience function.

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

show more ...

[clang][dataflow] Add an `operator<<` for `OptionalTypeIdentifier`.

When tests fail in UncheckedOptionalAccessModelTest.cpp, this prints the name of the optional type instead of a blob of hex.

Revi

[clang][dataflow] Add an `operator<<` for `OptionalTypeIdentifier`.

When tests fail in UncheckedOptionalAccessModelTest.cpp, this prints the name of the optional type instead of a blob of hex.

Reviewed By: ymandel

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

show more ...

[clang][dataflow] Print the source line if we saw unexpected diagnostics in tests.

This makes it easier to determine which line the unexpected happened on; previously, we would only get the line num

[clang][dataflow] Print the source line if we saw unexpected diagnostics in tests.

This makes it easier to determine which line the unexpected happened on; previously, we would only get the line number.

Reviewed By: ymandel

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

show more ...

[clang][dataflow] Fix a bug in handling of `operator->` for optional checker.

Prior to this patch, `operator->` was being handled like `operator*`: It was
associating a `Value` of type `T` with the

[clang][dataflow] Fix a bug in handling of `operator->` for optional checker.

Prior to this patch, `operator->` was being handled like `operator*`: It was
associating a `Value` of type `T` with the expression result (where `T` is the
template argument of the `optional<T>`). This is correct for `operator*`, which
returns a reference (of some flavor) to `T`, so that the result of the
`CXXOperatorCallExpr` is a glvalue of type `T`. However, `operator*` returns a
`T*`, so the result of the `CXXOperatorCallExpr` is a prvalue `T*`, which should
therefore be associated with `PointerValue` that in turn refers to a `T`.

I noticed this issue while working on the migration to strict handling of
value categories (see https://discourse.llvm.org/t/70086). The current behavior
also seems problematic more generally because it's plausible that the framework
may at some point introduce behavior that assumes an `Expr` of pointer type is
always associated with a `PointerValue`.

As it turns out, this patch fixes an existing FIXME in the test
`OptionalValueInitialization`.

Depends On D150657

Reviewed By: ymandel

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

show more ...

[clang][dataflow] Drop optional model's dependency on libc++ internals.

Adjusts the matchers in the optional model to avoid dependency on internal
implementation details of libc++'s `std::optional`.

[clang][dataflow] Drop optional model's dependency on libc++ internals.

Adjusts the matchers in the optional model to avoid dependency on internal
implementation details of libc++'s `std::optional`. In the process, factors out
the code to check the name of these types so that it's shared throughout.

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

show more ...

[clang][dataflow] Refine matching of optional types to anchor at top level.

This patch refines the matching of the relevant optional types to anchor on the
global namespace. Previously, we could mat

[clang][dataflow] Refine matching of optional types to anchor at top level.

This patch refines the matching of the relevant optional types to anchor on the
global namespace. Previously, we could match anything with the right name
(e.g. `base::Optional`) even if nested within other namespaces. This over
matching resulted in an assertion violation when _different_ `base::Optional`
was encountered nested inside another namespace.

Fixes issue #57036.

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

show more ...

[clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

Currently, the interpretation of `swap` calls in the optional model assumes the
optional arguments are modeled (and th

[clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

Currently, the interpretation of `swap` calls in the optional model assumes the
optional arguments are modeled (and therefore have valid storage locations and
values). This assumption is incorrect, for example, in the case of unmodeled
optional fields (which can be missing either value or location). This patch
relaxes these assumptions, to return rather than assert when either argument is
not modeled.

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

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][dataflow] In optional model, implement `widen` and make `compare` sound.

This patch includes two related changes:

1. Rewrite `compare` operation to be sound. Current version checks for equa

[clang][dataflow] In optional model, implement `widen` and make `compare` sound.

This patch includes two related changes:

1. Rewrite `compare` operation to be sound. Current version checks for equality
of `isNonEmptyOptional` on both values, judging the values `Same` when the
results are equal. While that works when both are true, it is problematic when
they are both false, because there are four cases in which that's can occur:
both empty, one empty and one unknown (which is two cases), and both unknown. In
the latter three cases, it is unsound to judge them `Same`. This patch changes
`compare` to explicitly check for case of `both empty` and then judge any other
case `Different`.

2. With the change to `compare`, a number of common cases will no longer
terminate. So, we also implement widening to properly handle those cases and
recover termination.

Drive-by: improve performance of `merge` operation.

Of the new tests, the code before the patch fails
* ReassignValueInLoopToSetUnsafe, and
* ReassignValueInLoopToUnknownUnsafe.

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

show more ...

[clang][dataflow] Fix bug in optional-checker's handling of nullopt constructor.

Currently, the checker only recognizes the nullopt constructor when it is called
without sugar, resulting in a crash

[clang][dataflow] Fix bug in optional-checker's handling of nullopt constructor.

Currently, the checker only recognizes the nullopt constructor when it is called
without sugar, resulting in a crash in the (rare) case where it has been wrapped
in sugar. This relaxes the constraint by checking the constructor decl directly
(which always contains the same, desugared form) rather than the construct
expression (where the spelling depends on the context).

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

show more ...

[clang][dataflow] Fix crash when having boolean-to-integral casts.

Since now we just ignore all (implicit) integral casts, treating the
resulting value as the same as the underlying value, it could

[clang][dataflow] Fix crash when having boolean-to-integral casts.

Since now we just ignore all (implicit) integral casts, treating the
resulting value as the same as the underlying value, it could cause
inconsistency between values after `Join` if in some paths the type
doesn't strictly match. This could cause intermittent crashes.

std::optional<bool> o;
int x;
if (o.has_value()) {
x = o.value();
}

Fixes: https://github.com/llvm/llvm-project/issues/59728

Signed-off-by: Jun Zhang <jun@junz.org>

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

show more ...