This serves two purposes:
- Because, today, we only copy the `StructValue`, modifying the destination of
the copy also modifies the source. This is demonstrated by the new checks
added to `CopyConstructor` and `MoveConstructor`, which fail without the
deep copy.
- It lays the groundwork for eliminating the redundancy between
`AggregateStorageLocation` and `StructValue`, which will happen as part of the
ongoing migration to strict handling of value categories (seeo
https://discourse.llvm.org/t/70086 for details). This will involve turning
`StructValue` into essentially just a wrapper for `AggregateStorageLocation`;
under this scheme, the current "shallow" copy (copying a `StructValue` from
one `AggregateStorageLocation` to another) will no longer be possible.
Because we now perform deep copies, tests need to perform a deep equality
comparison instead of just comparing for equal identity of the `StructValue`s.
The new function `recordsEqual()` provides such a deep equality comparison.
Reviewed By: xazax.hun
Differential Revision: https://reviews.llvm.org/D153006
This patch includes a test that fails without the fix.
I discovered that we weren't creating `Value`s for integer literals when, in a
different patch, I tried to overwrite the value of a struct field with a literal
for the purposes of a test and was surprised to find that the struct compared
the same before and after the assignment.
This functionality therefore seems useful at least for tests, but is probably
also useful for actual analysis of code.
Reviewed By: ymandel, xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D152813
This patch changes the way `Environment::ReturnLoc` is set: Whereas previously it was set by the caller, it is now set by the callee (obviously, as we otherwise would not be able to return references).
The patch also introduces `Environment::ReturnVal`, which is used for non-reference-type return values. This allows these to be handled with the correct value category semantics; see also https://discourse.llvm.org/t/70086, which describes the ongoing migration to strict value category semantics.
Depends On D150776
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D151194
Attempting to analyze templated code doesn't have a good cost-benefit ratio. We
have so far done a best-effort attempt at this, but maintaining this support has
an ongoing high maintenance cost because the AST for templates can violate a lot
of the invariants that otherwise hold for the AST of concrete code. As just one
example, in concrete code the operand of a UnaryOperator '*' is always a prvalue
(https://godbolt.org/z/s3e5xxMd1), but in templates this isn't true
(https://godbolt.org/z/6W9xxGvoM).
Further rationale for not analyzing templates:
* The semantics of a template itself are weakly defined; semantics can depend
strongly on the concrete template arguments. Analyzing the template itself (as
opposed to an instantiation) therefore has limited value.
* Analyzing templates requires a lot of special-case code that isn't necessary
for concrete code because dependent types are hard to deal with and the AST
violates invariants that otherwise hold for concrete code (see above).
* There's precedent in that neither Clang Static Analyzer nor the flow-sensitive
warnings in Clang (such as uninitialized variables) support analyzing
templates.
Reviewed By: gribozavr2, xazax.hun
Differential Revision: https://reviews.llvm.org/D150352
This parameter was already a no-op, so removing it doesn't change behavior.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D150137
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
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
Keeping this false could end up with extra iterations on a lot of loops
that aren't real ones (e.g. they could be a do-while-false for macros),
and makes the analyses very slow.
This patch changes the default for
CFG::BuildOptions.PruneTriviallyFalseEdges to true to avoid it.
Reviewed By: ymandel, xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D149640
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
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
The crash happened because the transfer fucntion for `&&` and `||`
unconditionally tried to retrieve the value of the RHS. However, if the RHS
is unreachable, there is no environment for it, and trying to retrieve the
operand's value causes an assertion failure.
See also the comments in the code for further details.
Reviewed By: xazax.hun, ymandel, sgatev, gribozavr2
Differential Revision: https://reviews.llvm.org/D146514
The invariants around `ReferenceValues` are subtle (arguably, too much so). That
includes that we need to take care not to double wrap them -- in cases where we
wrap a loc in an `ReferenceValue` we need to be sure that the pointee isn't
already a `ReferenceValue`. `BindingDecl` introduces another situation in which
this can arise. Previously, the code did not properly handle `BindingDecl`,
resulting in double-wrapped values, which broke other invariants (at least, that
struct values have an `AggregateStorageLocation`).
This patch adjusts the interpretation of `DeclRefExpr` to take `BindingDecl`'s
peculiarities into account. It also fixes the two tests which should have caught
this issue but were themselves (subtly) buggy.
Differential Revision: https://reviews.llvm.org/D140897
This patch fixes a subtle bug in how we create lvalues to reference-typed
fields. In the rare case that the field is umodeled because of the depth limit
on field modeling, the lvalue created can be malformed. This patch prevents that
and adds some related assertions to other code dealing with lvalues for
references.
Differential Revision: https://reviews.llvm.org/D142468
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
Merges `TransferOptions` into the newly-introduced
`DataflowAnalysisContext::Options` and removes explicit parameter for
`TransferOptions`, relying instead on the common options carried by the analysis
context. Given that there was no intent to allow different options between calls
to `transfer`, a common value for the options is preferable.
Differential Revision: https://reviews.llvm.org/D140703
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.
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
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
The handling of return statements, added in support of context-sensitive
analysis, has a bug relating to functions that return reference
types. Specifically, interpretation of such functions can result in a crash from
a bad cast. This patch fixes the bug and guards all of that code with the
context-sensitive option, since there's no reason to execute at all when
context-sensitive analysis is off.
Differential Revision: https://reviews.llvm.org/D140430
This patch adds interpretation of binding declarations resulting from a
structured binding (`DecompositionDecl`) to a tuple-like type. Currently, the
framework only supports binding to a struct.
Fixes issue #57252.
Differential Revision: https://reviews.llvm.org/D139544
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
* Adds API support for widening of lattice elements and environments,
* Updates the algorithm to apply widening where appropriate,
* Implements widening for boolean values. In the process, moves the unsoundness
of comparison from the default implementation of
`Environment::ValueModel::compare` to model-specific handling inside
`DataflowEnvironment::equivalentTo`. This change is intended to clarify
the source and location of unsoundess.
This patch is a replacement for, and was based substantially on, https://reviews.llvm.org/D131645.
Differential Revision: https://reviews.llvm.org/D137948
Extend the context-sensitive analysis to handle a call to a method (of the same
class) from within a method. That, is a member-call expression through `this`.
Differential Revision: https://reviews.llvm.org/D134432
This patch adds a `Depth` field (default value 2) to `ContextSensitiveOptions`, allowing context-sensitive analysis of functions that call other functions. This also requires replacing the `DeclCtx` field on `Environment` with a `CallString` field that contains a vector of decl contexts, to ensure that the analysis doesn't try to analyze recursive or mutually recursive calls (which would result in a crash, due to the way we handle `StorageLocation`s).
Reviewed By: xazax.hun
Differential Revision: https://reviews.llvm.org/D131809
This patch restructures `DataflowAnalysisOptions` and `TransferOptions` to use `llvm::Optional`, in preparation for adding more sub-options to the `ContextSensitiveOptions` struct introduced here.
Reviewed By: sgatev, xazax.hun
Differential Revision: https://reviews.llvm.org/D131779
This patch modifies `Environment`'s `pushCall` method to pass over arguments that are missing storage locations, instead of crashing.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D131600
This patch adds the ability to context-sensitively analyze constructor bodies, by changing `pushCall` to allow both `CallExpr` and `CXXConstructExpr`, and extracting the main context-sensitive logic out of `VisitCallExpr` into a new `transferInlineCall` method which is now also called at the end of `VisitCXXConstructExpr`.
Reviewed By: ymandel, sgatev, xazax.hun
Differential Revision: https://reviews.llvm.org/D131438
This patch modifies `Environment`'s `pushCall` method to pass over arguments that are missing storage locations, instead of crashing.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D131600
This patch adds the ability to context-sensitively analyze constructor bodies, by changing `pushCall` to allow both `CallExpr` and `CXXConstructExpr`, and extracting the main context-sensitive logic out of `VisitCallExpr` into a new `transferInlineCall` method which is now also called at the end of `VisitCXXConstructExpr`.
Reviewed By: ymandel, sgatev, xazax.hun
Differential Revision: https://reviews.llvm.org/D131438
This patch adds the ability to context-sensitively analyze method bodies, by moving `ThisPointeeLoc` from `DataflowAnalysisContext` to `Environment`, and adding code in `pushCall` to set it.
Reviewed By: ymandel, sgatev, xazax.hun
Differential Revision: https://reviews.llvm.org/D131170
This patch adds a `ReturnLoc` field to the `Environment`, serving a similar to the `ThisPointeeLoc` field in the `DataflowAnalysisContext`. It then uses that (along with a new `VisitReturnStmt` method in `TransferVisitor`) to handle non-`void`-returning functions in context-sensitive analysis.
Reviewed By: ymandel, sgatev
Differential Revision: https://reviews.llvm.org/D130600
This patch enables context-sensitive analysis of multiple different calls to the same function (see the `ContextSensitiveSetBothTrueAndFalse` example in the `TransferTest` suite) by replacing the `Environment` copy-assignment with a call to the new `popCall` method, which `std::move`s some fields but specifically does not move `DeclToLoc` and `ExprToLoc` from the callee back to the caller.
To enable this, the `StorageLocation` for a given parameter needs to be stable across different calls to the same function, so this patch also improves the modeling of parameter initialization, using `ReferenceValue` when necessary (for arguments passed by reference).
This approach explicitly does not work for recursive calls, because we currently only plan to use this context-sensitive machinery to support specialized analysis models we write, not analysis of arbitrary callees.
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D130726
This patch adds initial support for context-sensitive analysis of simple functions whose definition is available in the translation unit, guarded by the `ContextSensitive` flag in the new `TransferOptions` struct. When this option is true, the `VisitCallExpr` case in the builtin transfer function has a fallthrough case which checks for a direct callee with a body. In that case, it constructs a CFG from that callee body, uses the new `pushCall` method on the `Environment` to make an environment to analyze the callee, and then calls `runDataflowAnalysis` with a `NoopAnalysis` (disabling context-sensitive analysis on that sub-analysis, to avoid problems with recursion). After the sub-analysis completes, the `Environment` from its exit block is simply assigned back to the environment at the callsite.
The `pushCall` method (which currently only supports non-method functions with some restrictions) maps the `SourceLocation`s for all the parameters to the existing source locations for the corresponding arguments from the callsite.
This patch adds a few tests to check that this context-sensitive analysis works on simple functions. More sophisticated functionality will be added later; the most important next step is to explicitly model context in some fields of the `DataflowAnalysisContext` class, as mentioned in a `FIXME` comment in the `pushCall` implementation.
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D130306
Depends On D130305
This patch adds initial support for context-sensitive analysis of simple functions whose definition is available in the translation unit, guarded by the `ContextSensitive` flag in the new `TransferOptions` struct. When this option is true, the `VisitCallExpr` case in the builtin transfer function has a fallthrough case which checks for a direct callee with a body. In that case, it constructs a CFG from that callee body, uses the new `pushCall` method on the `Environment` to make an environment to analyze the callee, and then calls `runDataflowAnalysis` with a `NoopAnalysis` (disabling context-sensitive analysis on that sub-analysis, to avoid problems with recursion). After the sub-analysis completes, the `Environment` from its exit block is simply assigned back to the environment at the callsite.
The `pushCall` method (which currently only supports non-method functions with some restrictions) first calls `initGlobalVars`, then maps the `SourceLocation`s for all the parameters to the existing source locations for the corresponding arguments from the callsite.
This patch adds a few tests to check that this context-sensitive analysis works on simple functions. More sophisticated functionality will be added later; the most important next step is to explicitly model context in some fields of the `DataflowAnalysisContext` class, as mentioned in a `TODO` comment in the `pushCall` implementation.
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D130306
This patch moves `Analysis/FlowSensitive/NoopAnalysis.h` from `clang/unittests/` to `clang/include/clang/`, so that we can use it for doing context-sensitive analysis.
Reviewed By: ymandel, gribozavr2, sgatev
Differential Revision: https://reviews.llvm.org/D130304
