[analyzer][NFC] Use `SValVisitor` instead of explicit helper functions

Summary: Get rid of explicit function splitting in favor of specifically designed Visitor. Move logic from a family of `evalCas

[analyzer][NFC] Use `SValVisitor` instead of explicit helper functions

Summary: Get rid of explicit function splitting in favor of specifically designed Visitor. Move logic from a family of `evalCastKind` and `evalCastSubKind` helper functions to `SValVisitor`.

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

show more ...

[analyzer] Fix assertion in simplifySymbolCast

Depends on D128068.
Added a new test code that fails an assertion in the baseline.
That is because `getAPSIntType` works only with integral types.

Dif

[analyzer] Fix assertion in simplifySymbolCast

Depends on D128068.
Added a new test code that fails an assertion in the baseline.
That is because `getAPSIntType` works only with integral types.

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

show more ...

[analyzer] Do not emit redundant SymbolCasts

In `RegionStore::getBinding` we call `evalCast` unconditionally to align
the stored value's type to the one that is being queried. However, the
stored ty

[analyzer] Do not emit redundant SymbolCasts

In `RegionStore::getBinding` we call `evalCast` unconditionally to align
the stored value's type to the one that is being queried. However, the
stored type might be the same, so we may end up having redundant
`SymbolCasts` emitted.

The solution is to check whether the `to` and `from` type are the same
in `makeNonLoc`.

Note, we can't just do type equivalence check at the beginning of `evalCast`
because when `evalCast` is called from `getBinding` then the original type
(`OriginalTy`) is not set, so one operand is missing for the comparison. In
`evalCastSubKind(nonloc::SymbolVal)` when the original type is not set,
we get the `from` type via `SymbolVal::getType()`.

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

show more ...

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

Depends on D125709

Reviewed By: martong

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

[analyzer][NFC] Substitute the SVal::evalMinus and evalComplement functions

Depends on D126127

Reviewed By: martong

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

[analyzer][NFC] Relocate unary transfer functions

This is an initial step of removing the SimpleSValBuilder abstraction. The SValBuilder alone should be enough.

Reviewed By: martong

Differential R

[analyzer][NFC] Relocate unary transfer functions

This is an initial step of removing the SimpleSValBuilder abstraction. The SValBuilder alone should be enough.

Reviewed By: martong

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

show more ...

[analyzer] Add UnarySymExpr

This patch adds a new descendant to the SymExpr hierarchy. This way, now
we can assign constraints to symbolic unary expressions. Only the unary
minus and bitwise negatio

[analyzer] Add UnarySymExpr

This patch adds a new descendant to the SymExpr hierarchy. This way, now
we can assign constraints to symbolic unary expressions. Only the unary
minus and bitwise negation are handled.

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

show more ...

[analyzer][NFC] Tighten some of the SValBuilder return types

This is purely a cosmetic change.

Reviewed By: martong

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

[analyzer] Fix cast evaluation on scoped enums in ExprEngine

We ignored the cast if the enum was scoped.
This is bad since there is no implicit conversion from the scoped enum to the corresponding u

[analyzer] Fix cast evaluation on scoped enums in ExprEngine

We ignored the cast if the enum was scoped.
This is bad since there is no implicit conversion from the scoped enum to the corresponding underlying type.

The fix is basically: isIntegralOrEnumerationType() -> isIntegralOr**Unscoped**EnumerationType()

This materialized in crashes on analyzing the LLVM itself using the Z3 refutation.
Refutation synthesized the given Z3 Binary expression (`BO_And` of `unsigned char` aka. 8 bits
and an `int` 32 bits) with the wrong bitwidth in the end, which triggered an assert.

Now, we evaluate the cast according to the standard.

This bug could have been triggered using the Z3 CM according to
https://bugs.llvm.org/show_bug.cgi?id=44030

Fixes #47570 #43375

Reviewed By: martong

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

show more ...

[analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

clang: <root>/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:727:
void assertEqualBitWidths(clang::ento::ProgramStateRef,
clang::ento:

[analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

clang: <root>/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:727:
void assertEqualBitWidths(clang::ento::ProgramStateRef,
clang::ento::Loc, clang::ento::Loc): Assertion `RhsBitwidth ==
LhsBitwidth && "RhsLoc and LhsLoc bitwidth must be same!"'

This change adjusts the bitwidth of the smaller operand for an evalBinOp
as a result of a comparison operation. This can occur in the specific
case represented by the test cases for a target with different pointer
sizes.

Reviewed By: NoQ

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

show more ...

[analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

This is a NFC refactoring to change makeIntValWithPtrWidth
and remove getZeroWithPtrWidth to use types when forming value

[analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

This is a NFC refactoring to change makeIntValWithPtrWidth
and remove getZeroWithPtrWidth to use types when forming values to match
pointer widths. Some targets may have different pointer widths depending
upon address space, so this needs to be comprehended.

Reviewed By: steakhal

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

show more ...

[analyzer] Refactor makeNull to makeNullWithWidth (NFC)

Usages of makeNull need to be deprecated in favor of makeNullWithWidth
for architectures where the pointer size should not be assumed. This ca

[analyzer] Refactor makeNull to makeNullWithWidth (NFC)

Usages of makeNull need to be deprecated in favor of makeNullWithWidth
for architectures where the pointer size should not be assumed. This can
occur when pointer sizes can be of different sizes, depending on address
space for example. See https://reviews.llvm.org/D118050 as an example.

This was uncovered initially in a downstream compiler project, and
tested through those systems tests.

steakhal performed systems testing across a large set of open source
projects.

Co-authored-by: steakhal
Resolves: https://github.com/llvm/llvm-project/issues/53664

Reviewed By: NoQ, steakhal

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

show more ...

[analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

Summary: Produce SymbolCast for integral types in `evalCast` function. Apply several simplification techniques while

[analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

Summary: Produce SymbolCast for integral types in `evalCast` function. Apply several simplification techniques while producing the symbols. Added a boolean option `handle-integral-cast-for-ranges` under `-analyzer-config` flag. Disabled the feature by default.

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

show more ...

[analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

Previously, the `SValBuilder` could not encounter expressions of the
following kind:

NonLoc OP Loc
Loc OP NonLoc

Where the `Op

[analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

Previously, the `SValBuilder` could not encounter expressions of the
following kind:

NonLoc OP Loc
Loc OP NonLoc

Where the `Op` is other than `BO_Add`.

As of now, due to the smarter simplification and the fixedpoint
iteration, it turns out we can.
It can happen if the `Loc` was perfectly constrained to a concrete
value (`nonloc::ConcreteInt`), thus the simplifier can do
constant-folding in these cases as well.

Unfortunately, this could cause assertion failures, since we assumed
that the operator must be `BO_Add`, causing a crash.

---

In the patch, I decided to preserve the original behavior (aka. swap the
operands (if the operator is commutative), but if the `RHS` was a
`loc::ConcreteInt` call `evalBinOpNN()`.

I think this interpretation of the arithmetic expression is closer to
reality.

I also tried naively introducing a separate handler for
`loc::ConcreteInt` RHS, before doing handling the more generic `Loc` RHS
case. However, it broke the `zoo1backwards()` test in the `nullptr.cpp`
file. This highlighted for me the importance to preserve the original
behavior for the `BO_Add` at least.

PS: Sorry for introducing yet another branch into this `evalBinOpXX`
madness. I've got a couple of ideas about refactoring these.
We'll see if I can get to it.

The test file demonstrates the issue and makes sure nothing similar
happens. The `no-crash` annotated lines show, where we crashed before
applying this patch.

Reviewed By: martong

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

show more ...

[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
x && isa<T1>(x) ||

[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
x && isa<T1>(x) || isa<T2>(x) ...
Will be replaced by:
isa_and_non_null<T1, T2, ...>(x)

Sometimes it caused further simplifications, when it would cause even
more code smell.

Aside from this, keep in mind that within `assert()` or any macro
functions, we need to wrap the isa<> expression within a parenthesis,
due to the parsing of the comma.

Reviewed By: martong

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

show more ...

[analyzer] SValBuilder should have an easy access to AnalyzerOptions

`SVB.getStateManager().getOwningEngine().getAnalysisManager().getAnalyzerOptions()`
is quite a mouthful and might involve a few p

[analyzer] SValBuilder should have an easy access to AnalyzerOptions

`SVB.getStateManager().getOwningEngine().getAnalysisManager().getAnalyzerOptions()`
is quite a mouthful and might involve a few pointer indirections to get
such a simple thing like an analyzer option.

This patch introduces an `AnalyzerOptions` reference to the `SValBuilder`
abstract class, while refactors a few cases to use this /simpler/ accessor.

Reviewed By: martong, Szelethus

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

show more ...

[analyzer] Cleanup a FIXME in SValBuilder.cpp

This change follows up on a FIXME submitted with D105974. This change simply let's the reference case fall through to return a concrete 'true'
instead o

[analyzer] Cleanup a FIXME in SValBuilder.cpp

This change follows up on a FIXME submitted with D105974. This change simply let's the reference case fall through to return a concrete 'true'
instead of a nonloc pointer of appropriate length set to NULL.

Reviewed By: NoQ

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

show more ...

[analyzer] Handle std::make_unique

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

[analyzer] Do not assume that all pointers have the same bitwidth as void*

This change addresses this assertion that occurs in a downstream
compiler with a custom target.

```APInt.h:1151: bool llvm

[analyzer] Do not assume that all pointers have the same bitwidth as void*

This change addresses this assertion that occurs in a downstream
compiler with a custom target.

```APInt.h:1151: bool llvm::APInt::operator==(const llvm::APInt &) const: Assertion `BitWidth == RHS.BitWidth && "Comparison requires equal bit widths"'```

No covering test case is susbmitted with this change since this crash
cannot be reproduced using any upstream supported target. The test case
that exposes this issue is as simple as:

```lang=c++
void test(int * p) {
int * q = p-1;
if (q) {}
if (q) {} // crash
(void)q;
}
```

The custom target that exposes this problem supports two address spaces,
16-bit `char`s, and a `_Bool` type that maps to 16-bits. There are no upstream
supported targets with similar attributes.

The assertion appears to be happening as a result of evaluating the
`SymIntExpr` `(reg_$0<int * p>) != 0U` in `VisitSymIntExpr` located in
`SimpleSValBuilder.cpp`. The `LHS` is evaluated to `32b` and the `RHS` is
evaluated to `16b`. This eventually leads to the assertion in `APInt.h`.

While this change addresses the crash and passes LITs, two follow-ups
are required:
1) The remainder of `getZeroWithPtrWidth()` and `getIntWithPtrWidth()`
should be cleaned up following this model to prevent future
confusion.
2) We're not sure why references are found along with the modified
code path, that should not be the case. A more principled
fix may be found after some further comprehension of why this
is the case.

Acks: Thanks to @steakhal and @martong for the discussions leading to this
fix.

Reviewed By: NoQ

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

show more ...

[analyzer] [NFC] Implement a wrapper SValBuilder::getCastedMemRegionVal for similar functionality on region cast

Summary: Replaced code on region cast with a function-wrapper SValBuilder::getCasted

[analyzer] [NFC] Implement a wrapper SValBuilder::getCastedMemRegionVal for similar functionality on region cast

Summary: Replaced code on region cast with a function-wrapper SValBuilder::getCastedMemRegionVal. This is a next step of code refining due to suggestions in D103319.

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

show more ...

[analyzer] Use Optional as a return type of StoreManager::castRegion

Summary: Make StoreManager::castRegion function usage safier. Replace `const MemRegion *` with `Optional<const MemRegion *>`. Si

[analyzer] Use Optional as a return type of StoreManager::castRegion

Summary: Make StoreManager::castRegion function usage safier. Replace `const MemRegion *` with `Optional<const MemRegion *>`. Simplified one of related test cases due to suggestions in D101635.

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

show more ...

[analyzer] Fix assertion in SVals.h

Fix assertion in SVals.h apparently caused by
https://reviews.llvm.org/D89055.

clang:clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:596:
clang::en

[analyzer] Fix assertion in SVals.h

Fix assertion in SVals.h apparently caused by
https://reviews.llvm.org/D89055.

clang:clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:596:
clang::ento::loc::MemRegionVal::MemRegionVal(const clang::ento::MemRegion *):
Assertion `r' failed.

Backtrace:
...
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:597:3
clang::QualType, clang::QualType)
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:773:18
clang::QualType, clang::QualType)
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:612:12
clang::QualType) clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:587:12
namespace)::RegionBindingsRef const&, clang::ento::Loc, clang::QualType)
clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1510:24
...

Reviewed By: ASDenysPetrov

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

show more ...

[analyzer] Wrong type cast occurs during pointer dereferencing after type punning

Summary: During pointer dereferencing CastRetrievedVal uses wrong type from the Store after type punning. Namely, th

[analyzer] Wrong type cast occurs during pointer dereferencing after type punning

Summary: During pointer dereferencing CastRetrievedVal uses wrong type from the Store after type punning. Namely, the pointer casts to another type and then assigns with a value of one more another type. It produces NonLoc value when Loc is expected.

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

Fixes:
https://bugs.llvm.org/show_bug.cgi?id=37503
https://bugs.llvm.org/show_bug.cgi?id=49007

show more ...

[analyzer] [NFC] Eliminate dispatchCast, evalCastFromNonLoc and evalCastFromLoc functions from SValBuilder

Summary: Remove dispatchCast, evalCastFromNonLoc and evalCastFromLoc functions since their

[analyzer] [NFC] Eliminate dispatchCast, evalCastFromNonLoc and evalCastFromLoc functions from SValBuilder

Summary: Remove dispatchCast, evalCastFromNonLoc and evalCastFromLoc functions since their functionality has been moved to common evalCast function. Use evalCast instead.

Post-clean up patch for https://reviews.llvm.org/D96090 patch. The patch shall not change any behavior.

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

show more ...

[analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

Summary: Move logic from CastRetrievedVal to evalCast and replace CastRetrievedVal with evalCast. Also move guts from Sim

[analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

Summary: Move logic from CastRetrievedVal to evalCast and replace CastRetrievedVal with evalCast. Also move guts from SimpleSValBuilder::dispatchCast inside evalCast.
evalCast intends to substitute dispatchCast, evalCastFromNonLoc and evalCastFromLoc in the future. OriginalTy provides additional information for casting, which is useful for some cases and useless for others. If `OriginalTy.isNull()` is true, then cast performs based on CastTy only. Now evalCast operates in two ways. It retains all previous behavior and take over dispatchCast behavior. dispatchCast, evalCastFromNonLoc and evalCastFromLoc is considered as buggy since it doesn't take into account OriginalTy of the SVal and should be improved.

From this patch use evalCast instead of dispatchCast, evalCastFromNonLoc and evalCastFromLoc functions. dispatchCast redirects to evalCast.

This patch shall not change any behavior.

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

show more ...