[analyzer] Simplify CallEvent castArgToParamTypeIfNeeded (#120981)

I noticed recently that this code (that I wrote xD) uses the
`getRuntimeDefinition()` which isn't quite necessary for the simple ta

[analyzer] Simplify CallEvent castArgToParamTypeIfNeeded (#120981)

I noticed recently that this code (that I wrote xD) uses the
`getRuntimeDefinition()` which isn't quite necessary for the simple task
this function was designed for.

Why would it be better not using this API here?
I'm experimenting with improving how virtual functions are inlined,
where depending on our ability of deducing the dynamic type of the
object we may end up with inaccurate type information. Such inaccuracy
would mean that we may have multiple runtime definitions. After that,
this code would become ambiguous.

To resolve this, I decided to refactor this and use a simpler - but
equivalent approach.

show more ...

[analyzer] Use dynamic type when invalidating by a member function call (#111138)

When instantiating "callable<T>", the "class CallableType" nested type
will only have a declaration in the copy for

[analyzer] Use dynamic type when invalidating by a member function call (#111138)

When instantiating "callable<T>", the "class CallableType" nested type
will only have a declaration in the copy for the instantiation - because
it's not refereed to directly by any other code that would need a
complete definition.

However, in the past, when conservative eval calling member function, we
took the static type of the "this" expr, and looked up the CXXRecordDecl
it refereed to to see if it has any mutable members (to decide if it
needs to refine invalidation or not). Unfortunately, that query needs a
definition, and it asserts otherwise, thus we crashed.

To fix this, we should consult the dynamic type of the object, because
that will have the definition.
I anyways added a check for "hasDefinition" just to be on the safe side.

Fixes #77378

show more ...

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

Follow up to #109133.

[analyzer] Fix false positive for mutexes inheriting mutex_base (#106240)

If a mutex interface is split in inheritance chain, e.g. struct mutex
has `unlock` and inherits `lock` from __mutex_base th

[analyzer] Fix false positive for mutexes inheriting mutex_base (#106240)

If a mutex interface is split in inheritance chain, e.g. struct mutex
has `unlock` and inherits `lock` from __mutex_base then calls m.lock()
and m.unlock() have different "this" targets: m and the __mutex_base of
m, which used to confuse the `ActiveCritSections` list.

Taking base region canonicalizes the region used to identify a critical
section and enables search in ActiveCritSections list regardless of
which class the callee is the member of.

This likely fixes #104241

CPP-5541

show more ...

[analyzer] Don't invalidate the super region when a std object ctor runs (#100405)

CPP-5269

[analyzer] Support C++23 static operator calls (#84972)

Made by following:
https://github.com/llvm/llvm-project/pull/83585#issuecomment-1980340866

Thanks for the details Tomek!

CPP-5080

[analyzer] Fix crash on dereference invalid return value of getAdjustedParameterIndex() (#83585)

Fixes #78810
Thanks for Snape3058 's comment

---------

Co-authored-by: miaozhiyuan <miaozhiyu

[analyzer] Fix crash on dereference invalid return value of getAdjustedParameterIndex() (#83585)

Fixes #78810
Thanks for Snape3058 's comment

---------

Co-authored-by: miaozhiyuan <miaozhiyuan@feysh.com>

show more ...

[clang] Use StringRef::{starts,ends}_with (NFC) (#75149)

This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}:

[clang] Use StringRef::{starts,ends}_with (NFC) (#75149)

This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::{starts,ends}_with in C++20.

I'm planning to deprecate and eventually remove
StringRef::{starts,ends}with.

show more ...

[analyzer] Add std::variant checker (#66481)

As my BSc thesis I've implemented a checker for std::variant and
std::any, and in the following weeks I'll upload a revised version of
them here.

#

[analyzer] Add std::variant checker (#66481)

As my BSc thesis I've implemented a checker for std::variant and
std::any, and in the following weeks I'll upload a revised version of
them here.

# Prelude

@Szelethus and I sent out an email with our initial plans here:
https://discourse.llvm.org/t/analyzer-new-checker-for-std-any-as-a-bsc-thesis/65613/2
We also created a stub checker patch here:
https://reviews.llvm.org/D142354.

Upon the recommendation of @haoNoQ , we explored an option where instead
of writing a checker, we tried to improve on how the analyzer natively
inlined the methods of std::variant and std::any. Our attempt is in this
patch https://reviews.llvm.org/D145069, but in a nutshell, this is what
happened: The analyzer was able to model much of what happened inside
those classes, but our false positive suppression machinery erroneously
suppressed it. After months of trying, we could not find a satisfying
enhancement on the heuristic without introducing an allowlist/denylist
of which functions to not suppress.

As a result (and partly on the encouragement of @Xazax-hun) I wrote a
dedicated checker!

The advantage of the checker is that it is not dependent on the
standard's implementation and won't put warnings in the standard library
definitions. Also without the checker it would be difficult to create
nice user-friendly warnings and NoteTags -- as per the standard's
specification, the analysis is sinked by an exception, which we don't
model well now.

# Design ideas

The working of the checker is straightforward: We find the creation of
an std::variant instance, store the type of the variable we want to
store in it, then save this type for the instance. When retrieving type
from the instance we check what type we want to retrieve as, and compare
it to the actual type. If the two don't march we emit an error.

Distinguishing variants by instance (e.g. MemRegion *) is not the most
optimal way. Other checkers, like MallocChecker uses a symbol-to-trait
map instead of region-to-trait. The upside of using symbols (which would
be the value of a variant, not the variant itself itself) is that the
analyzer would take care of modeling copies, moves, invalidation, etc,
out of the box. The problem is that for compound types, the analyzer
doesn't create a symbol as a result of a constructor call that is fit
for this job. MallocChecker in contrast manipulates simple pointers.

My colleges and I considered the option of making adjustments directly
to the memory model of the analyzer, but for the time being decided
against it, and go with the bit more cumbersome, but immediately viable
option of simply using MemRegions.

# Current state and review plan

This patch contains an already working checker that can find and report
certain variant/any misuses, but still lands it in alpha. I plan to
upload the rest of the checker in later patches.

The full checker is also able to "follow" the symbolic value held by the
std::variant and updates the program state whenever we assign the value
stored in the variant. I have also built a library that is meant to
model union-like types similar to variant, hence some functions being a
bit more multipurpose then is immediately needed.

I also intend to publish my std::any checker in a later commit.

---------

Co-authored-by: Gabor Spaits <gabor.spaits@ericsson.com>
Co-authored-by: Balazs Benics <benicsbalazs@gmail.com>

show more ...

[analyzer] Fix assertion failure in `CXXInstanceCall::getCXXThisVal` (#70837)

Workaround the case when the `this` pointer is actually a `NonLoc`, by
returning `Unknown` instead.
The solution isn't

[analyzer] Fix assertion failure in `CXXInstanceCall::getCXXThisVal` (#70837)

Workaround the case when the `this` pointer is actually a `NonLoc`, by
returning `Unknown` instead.
The solution isn't ideal, as `this` should be really a `Loc`, but due to
how casts work, I feel this is our easiest and best option.

As this patch presents, I'm evaluating a cast to transform the `NonLoc`.
However, given that `evalCast()` can't be cast from `NonLoc` to a
pointer type thingy (`Loc`), we end up with `Unknown`.
It is because `EvalCastVisitor::VisitNonLocSymbolVal()` only evaluates
casts that happen from NonLoc to NonLocs.

When I tried to actually implement that case, I figured:
1) Create a `SymbolicRegion` from that `nonloc::SymbolVal`; but
`SymbolRegion` ctor expects a pointer type for the symbol.
2) Okay, just have a `SymbolCast`, getting us the pointer type; but
`SymbolRegion` expects `SymbolData` symbols, not generic `SymExpr`s, as
stated:

> // Because pointer arithmetic is represented by ElementRegion layers,
> // the base symbol here should not contain any arithmetic.

3) We can't use `ElementRegion`s to perform this cast because to have an
`ElementRegion`, you already have to have a `SubRegion` that you want to
cast, but the point is that we don't have that.

At this point, I gave up, and just left a FIXME instead, while still
returning `Unknown` on that path.
IMO this is still better than having a crash.

Fixes #69922

show more ...

[analyzer] Remove rdar links from static analyzer and libAnalysis sources. NFC.

I actually visited each link and added relevant context directly to the code.

This is related to the effort to elimin

[analyzer] Remove rdar links from static analyzer and libAnalysis sources. NFC.

I actually visited each link and added relevant context directly to the code.

This is related to the effort to eliminate internal bug tracker links
(d618f1c, e0ac46e).

Test files still have a lot of rdar links and ids in them.
I haven't touched them yet.

show more ...

Revert "Remove rdar links; NFC"

This reverts commit d618f1c3b12effd0c2bdb7d02108d3551f389d3d.
This commit wasn't reviewed ahead of time and significant concerns were
raised immediately after it land

Revert "Remove rdar links; NFC"

This reverts commit d618f1c3b12effd0c2bdb7d02108d3551f389d3d.
This commit wasn't reviewed ahead of time and significant concerns were
raised immediately after it landed. According to our developer policy
this warrants immediate revert of the commit.

https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy

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

show more ...

Remove rdar links; NFC

This removes links to rdar, which is an internal bug tracker that the
community doesn't have visibility into.

See further discussion at:
https://discourse.llvm.org/t/code-rev

Remove rdar links; NFC

This removes links to rdar, which is an internal bug tracker that the
community doesn't have visibility into.

See further discussion at:
https://discourse.llvm.org/t/code-review-reminder-about-links-in-code-commit-messages/71847

show more ...

[analyzer] Remove the loop from the exploded graph caused by missing information in program points

This patch adds CFGElementRef to ProgramPoints
and helps the analyzer to differentiate between
two

[analyzer] Remove the loop from the exploded graph caused by missing information in program points

This patch adds CFGElementRef to ProgramPoints
and helps the analyzer to differentiate between
two otherwise identically looking ProgramPoints.

Fixes #60412

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

show more ...

[clang] Remove remaining uses of llvm::Optional (NFC)

This patch removes several "using" declarations and #include
"llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to

[clang] Remove remaining uses of llvm::Optional (NFC)

This patch removes several "using" declarations and #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] 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 ...

[clang] Don't including None.h (NFC)

These source files no longer use None, so they do not need to include
None.h.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https:

[clang] Don't including None.h (NFC)

These source files no longer use None, so they do not need to include
None.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 ...

[StaticAnalyzer] Use std::nullopt instead of None (NFC)

This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the am

[StaticAnalyzer] Use std::nullopt instead of None (NFC)

This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
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] Fix assertion failure with conflicting prototype calls

It turns out we can reach the `Init.castAs<nonlock::CompoundVal>()`
expression with other kinds of SVals. Such as by `nonloc::Concre

[analyzer] Fix assertion failure with conflicting prototype calls

It turns out we can reach the `Init.castAs<nonlock::CompoundVal>()`
expression with other kinds of SVals. Such as by `nonloc::ConcreteInt`
in this example: https://godbolt.org/z/s4fdxrcs9

```lang=C++
int buffer[10];
void b();
void top() {
b(&buffer);
}
void b(int *c) {
*c = 42; // would crash
}
```
In this example, we try to store `42` to the `Elem{buffer, 0}`.

This situation can appear if the CallExpr refers to a function
declaration without prototype. In such cases, the engine will pick the
redecl of the referred function decl which has function body, hence has
a function prototype.

This weird situation will have an interesting effect to the AST, such as
the argument at the callsite will miss a cast, which would cast the
`int (*)[10]` expression into `int *`, which means that when we evaluate
the `*c = 42` expression, we want to bind `42` to an array, causing the
crash.

Look at the AST of the callsite with and without the function prototype:
https://godbolt.org/z/Gncebcbdb
The only difference is that without the proper function prototype, we
will not have the `ImplicitCastExpr` `BitCasting` from `int (*)[10]`
to `int *` to match the expected type of the parameter declaration.

In this patch, I'm proposing to emit a cast in the mentioned edge-case,
to bind the argument value of the expected type to the parameter.

I'm only proposing this if the runtime definition has exactly the same
number of parameters as the callsite feeds it by arguments.
If that's not the case, I believe, we are better off by binding `Unknown`
to those parameters.

Reviewed By: martong

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

show more ...

[analyzer] Pass correct bldrCtx to computeObjectUnderConstruction

In case when the prvalue is returned from the function (kind is one
of `SimpleReturnedValueKind`, `CXX17ElidedCopyReturnedValueKind`

[analyzer] Pass correct bldrCtx to computeObjectUnderConstruction

In case when the prvalue is returned from the function (kind is one
of `SimpleReturnedValueKind`, `CXX17ElidedCopyReturnedValueKind`),
then it construction happens in context of the caller.
We pass `BldrCtx` explicitly, as `currBldrCtx` will always refer to callee
context.

In the following example:
```
struct Result {int value; };
Result create() { return Result{10}; }
int accessValue(Result r) { return r.value; }

void test() {
for (int i = 0; i < 2; ++i)
accessValue(create());
}
```

In case when the returned object was constructed directly into the
argument to a function call `accessValue(create())`, this led to
inappropriate value of `blockCount` being used to locate parameter region,
and as a consequence resulting object (from `create()`) was constructed
into a different region, that was later read by inlined invocation of
outer function (`accessValue`).
This manifests itself only in case when calling block is visited more
than once (loop in above example), as otherwise there is no difference
in `blockCount` value between callee and caller context.
This happens only in case when copy elision is disabled (before C++17).

Reviewed By: NoQ

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

show more ...

[clang] Qualify auto in range-based for loops (NFC)

[clang] Don't use Optional::getValue (NFC)

Don't use Optional::hasValue (NFC)

[clang] Don't use Optional::hasValue (NFC)