The arguments to this are the same as for the 'wait' clause, so this
reuses all of that infrastructure. So all this has to do is support a
pair of clauses that are already implemented (if and async), plus create
an AST node. This patch does so, and adds proper testing.
These constructs are all very similar and closely related, so this patch
creates the AST nodes for them, serialization, printing/etc.
Additionally the restrictions are all added as tests/todos in the tests,
as those will have to be implemented once we get those clauses implemented.
Previously `BranchNodeBuilder` had a machinery to mark the two possible
branches (true, false) as infeasible, but this was completely useless in
practice, because the `BranchNodeBuilder` objects where short-lived
local variables so the `markInfeasible()` calls did not affect any later
calls.
The only theoretical exception was that in `ExprEngine::processBranch`
the methods of `BranchNodeBuilder` were called within a `for` loop that
iterates over the nodes created by the `check::BranchCondition`
callbacks.
However, currently only two checkers listen to `check::BranchCondition`
and neither of them produces multiple out nodes. This is fortunate,
because if the `for` loop had multiple iterations, then the lingering
effects of `markInfeasible()` would have caused wildly incorrect
behavior.
_For example, let's assume that a hypothetical `check::BranchCondition`
callback transitions to two different states, and the condition
expression happens to be true in the first and false in the second. In
this situation the first iteration of the loop would mark the false
branch as 'infeasible' and then in the second iteration the analyzer
would skip creating the transition to the false branch (from the state
where that is the 'right' path forward)._
After removing `markInfeasible()`, it became clear that the
`isFeasible()` calls in `ExprEngine::processBranch` are redundant
because they only guarded a `generateNode` call -- which immediately
calls `isFeasible()` and does nothing on an infeasible branch.
At this point in the refactoring the only code writing the feasibility
data members is their initialization in the constructors of
`BranchNodeBuilder` and the only code reading them is the check at the
beginning of `BranchNodeBuilder::generateNode`, so it was
straightforward to remove them completely and simplify the logic of
`generateNode()` to let it directly check the nullness of the
`CFGBlock*` pointer that it wants to use.
Finally, after these changes it became clear that in
`ExprEngine::processBranch` the `else` branch
(corresponding to the case when `assumeCondition` fails) is equivalent
to the "normal" case, so I eliminated it as well.
I also update the capitalization of a few variables that are already
affected by this change.
Like in the test case:
```c++
struct String {
String(const String &) {}
};
struct MatchComponent {
unsigned numbers[2];
String prerelease;
MatchComponent(MatchComponent const &) = default;
};
MatchComponent get();
void consume(MatchComponent const &);
MatchComponent parseMatchComponent() {
MatchComponent component = get();
component.numbers[0] = 10;
component.numbers[1] = 20;
return component; // We should have no stack addr escape warning here.
}
void top() {
consume(parseMatchComponent());
}
```
When calling `consume(parseMatchComponent())` the
`parseMatchComponent()` would return a copy of a temporary of
`component`. That copy would invoke the
`MatchComponent::MatchComponent(const MatchComponent &)` ctor.
That ctor would have a (reference typed) ParamVarRegion, holding the
location (lvalue) of the object we are about to copy (&component). So
far so good, but just before evaluating the binding operation for
initializing the `numbers` field of the temporary, we evaluate the
ArrayInitLoopExpr representing the by-value elementwise copy of the
array `component.numbers`. This is represented by a LazyCompoundVal,
because we (usually) don't just copy large arrays and bind many
individual direct bindings. Rather, we take a snapshot by using a LCV.
However, notice that the LCV representing this copy would look like
this:
lazyCompoundVal{ParamVarRegion{"reference param of cctor"}.numbers}
Notice that it refers to the numbers field of a reference. It would be
much better to desugar the reference to the actual object, thus it
should be: `lazyCompoundVal{component.numbers}`
Actually, when binding the result of the ArrayInitLoopExpr to the
`temp_object.numbers` in the compiler-generated member initializer of
the cctor, we should have two individual direct bindings because this is
a "small array":
```
binding &Element{temp_object.numbers, 0} <- loadFrom(&Element{component.numbers, 0})
binding &Element{temp_object.numbers, 1} <- loadFrom(&Element{component.numbers, 1})
```
Where `loadFrom(...)` would be:
```
loadFrom(&Element{component.numbers, 0}): 10 U32b
loadFrom(&Element{component.numbers, 1}): 20 U32b
```
So the store should look like this, after PostInitializer of
`temp_object.numbers`:
```
temp_object at offset 0: 10 U32b
temp_object at offset 32: 20 U32b
```
The lesson is that it's okay to have TypedValueRegions of references as
long as we don't form subregions of those. If we ever want to refer to a
subregion of a "reference" we actually meant to "desugar" the reference
and slice a subregion of the pointee of the reference instead.
Once this canonicalization is in place, we can also drop the special
handling of references in `ProcessInitializer`, because now reference
TypedValueRegions are eagerly desugared into their referee region when
forming a subregion of it.
There should be no practical differences, but there are of course bugs
that this patch may surface.
Combined constructs (OpenACC 3.3 section 2.11) are a short-cut for
writing a `loop` construct immediately inside of a `compute` construct.
However, this interaction requires we do additional work to ensure that
we get the semantics between the two correct, as well as diagnostics.
This patch adds the semantic analysis for the constructs (but no
clauses), as well as the AST nodes.
This commit is a collection of several very minor code quality
improvements. The main goal is removing the misleading "Bin" substring
from the names of several methods and variables (like
`evalEagerlyAssumedBinOpBifurcation`) that are also applied for the
unary logical not operator.
In addition to this, I clarified the doc-comment of the method
`evalEagerlyAssumedBinOpBifurcation` and refactored the body of this
method to fix the capitalization of variable names and replace an
obsolete use of `std::tie` with a structured binding.
Finally, the data member `eagerlyAssumeBinOpBifurcation` of the class
`AnalyzerOptions` was completely removed (including a line in clang-tidy
that sets it to true), because it was never read by any code.
Note that the eagerly-assume mode of the analyzer is controlled by a
different boolean member of `AnalyzerOptions` which is called
`ShouldEagerlyAssume` and is defined via the macro magic from
`AnalyzerOptions.def`.
The 'tile' clause shares quite a bit of the rules with 'collapse', so a
followup patch will add those tests/behaviors. This patch deals with
adding the AST node.
The 'tile' clause takes a series of integer constant expressions, or *.
The asterisk is now represented by a new OpenACCAsteriskSizeExpr node,
else this clause is very similar to others.
Bind the array member to the compound region associated with the
initializer list, e.g.:
class C {
int arr[2];
C() : arr{1, 2} {}
};
C c;
This change enables correct values in `c.arr[0]` and `c.arr[1]`
CPP-5647
HLSL output parameters are denoted with the `inout` and `out` keywords
in the function declaration. When an argument to an output parameter is
constructed a temporary value is constructed for the argument.
For `inout` pamameters the argument is initialized via copy-initialization
from the argument lvalue expression to the parameter type. For `out`
parameters the argument is not initialized before the call.
In both cases on return of the function the temporary value is written
back to the argument lvalue expression through an implicit assignment
binary operator with casting as required.
This change introduces a new HLSLOutArgExpr ast node which represents
the output argument behavior. The OutArgExpr has three defined children:
- An OpaqueValueExpr of the argument lvalue expression.
- An OpaqueValueExpr of the copy-initialized parameter.
- A BinaryOpExpr assigning the first with the value of the second.
Fixes#87526
---------
Co-authored-by: Damyan Pepper <damyanp@microsoft.com>
Co-authored-by: John McCall <rjmccall@gmail.com>
Fix some false negatives of StackAddrEscapeChecker:
- Output parameters
```
void top(int **out) {
int local = 42;
*out = &local; // Noncompliant
}
```
- Indirect global pointers
```
int **global;
void top() {
int local = 42;
*global = &local; // Noncompliant
}
```
Note that now StackAddrEscapeChecker produces a diagnostic if a function
with an output parameter is analyzed as top-level or as a callee. I took
special care to make sure the reports point to the same primary location
and, in many cases, feature the same primary message. That is the
motivation to modify Core/BugReporter.cpp and Core/ExplodedGraph.cpp
To avoid false positive reports when a global indirect pointer is
assigned a local address, invalidated, and then reset, I rely on the
fact that the invalidation symbol will be a DerivedSymbol of a
ConjuredSymbol that refers to the same memory region.
The checker still has a false negative for non-trivial escaping via a
returned value. It requires a more sophisticated traversal akin to
scanReachableSymbols, which out of the scope of this change.
CPP-4734
---------
This is the last of the 3 stacked PRs, it must not be merged before
https://github.com/llvm/llvm-project/pull/105652 and
https://github.com/llvm/llvm-project/pull/105653
If pointer is passed as input operand for inline assembly, it's possible
that asm block will change memory behind this pointer. So if pointer is
passed inside inline asm block, it's better to not guess and assume
memory has unknown state.
Without such change, we observed a lot of FP with hand-written `memcpy`
and friends.
Since `raw_string_ostream` doesn't own the string buffer, it is
desirable (in terms of memory safety) for users to directly reference
the string buffer rather than use `raw_string_ostream::str()`.
Work towards TODO comment to remove `raw_string_ostream::str()`.
This commit implements the entirety of the now-accepted [N3017
-Preprocessor
Embed](https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3017.htm) and
its sister C++ paper [p1967](https://wg21.link/p1967). It implements
everything in the specification, and includes an implementation that
drastically improves the time it takes to embed data in specific
scenarios (the initialization of character type arrays). The mechanisms
used to do this are used under the "as-if" rule, and in general when the
system cannot detect it is initializing an array object in a variable
declaration, will generate EmbedExpr AST node which will be expanded by
AST consumers (CodeGen or constant expression evaluators) or expand
embed directive as a comma expression.
This reverts commit
682d461d5a.
---------
Co-authored-by: The Phantom Derpstorm <phdofthehouse@gmail.com>
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
Co-authored-by: cor3ntin <corentinjabot@gmail.com>
Co-authored-by: H. Vetinari <h.vetinari@gmx.com>
This commit implements the entirety of the now-accepted [N3017 -
Preprocessor
Embed](https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3017.htm) and
its sister C++ paper [p1967](https://wg21.link/p1967). It implements
everything in the specification, and includes an implementation that
drastically improves the time it takes to embed data in specific
scenarios (the initialization of character type arrays). The mechanisms
used to do this are used under the "as-if" rule, and in general when the
system cannot detect it is initializing an array object in a variable
declaration, will generate EmbedExpr AST node which will be expanded
by AST consumers (CodeGen or constant expression evaluators) or
expand embed directive as a comma expression.
---------
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
Co-authored-by: cor3ntin <corentinjabot@gmail.com>
Co-authored-by: H. Vetinari <h.vetinari@gmx.com>
Co-authored-by: Podchishchaeva, Mariya <mariya.podchishchaeva@intel.com>
This patch implements the 'loop' construct AST, as well as the basic
appertainment rule. Additionally, it sets up the 'parent' compute
construct, which is necessary for codegen/other diagnostics.
A 'loop' can apply to a for or range-for loop, otherwise it has no other
restrictions (though some of its clauses do).
Depends on https://github.com/llvm/llvm-project/pull/92527
Clang now support the following:
- Extending lifetime of object bound to reference members of aggregates,
that are created from default member initializer.
- Rebuild `CXXDefaultArgExpr` and `CXXDefaultInitExpr` as needed where
called or constructed.
But CFG and ExprEngine need to be updated to address this change.
This PR add `CXXDefaultArgExpr` and `CXXDefaultInitExpr` into CFG, and
correct handle these expressions in ExprEngine
---------
Signed-off-by: yronglin <yronglin777@gmail.com>
OpenACC is going to need an array sections implementation that is a
simpler version/more restrictive version of the OpenMP version.
This patch moves `OMPArraySectionExpr` to `Expr.h` and renames it `ArraySectionExpr`,
then adds an enum to choose between the two.
This also fixes a couple of 'drive-by' issues that I discovered on the way,
but leaves the OpenACC Sema parts reasonably unimplemented (no semantic
analysis implementation), as that will be a followup patch.
'serial', 'parallel', and 'kernel' constructs are all considered
'Compute' constructs. This patch creates the AST type, plus the required
infrastructure for such a type, plus some base types that will be useful
in the future for breaking this up.
The only difference between the three is the 'kind'( plus some minor
clause legalization rules, but those can be differentiated easily
enough), so rather than representing them as separate AST nodes, it
seems
to make sense to make them the same.
Additionally, no clause AST functionality is being implemented yet, as
that fits better in a separate patch, and this is enough to get the
'naked' constructs implemented.
This is otherwise an 'NFC' patch, as it doesn't alter execution at all,
so there aren't any tests. I did this to break up the review workload
and to get feedback on the layout.
Implements https://isocpp.org/files/papers/P2662R3.pdf
The feature is exposed as an extension in older language modes.
Mangling is not yet supported and that is something we will have to do before release.
Fixes#70464
When ctor is not declared in the base class, initializing the base class
with the initializer list will not trigger a proper assignment of the
base region, as a CXXConstructExpr doing that is not available in the
AST.
This patch checks whether the init expr is an InitListExpr under a base
initializer, and adds a binding if so.
Static analyze can't report diagnose when statement after a
CXXForRangeStmt and enable widen, because
`ExprEngine::processCFGBlockEntrance` lacks of CXXForRangeStmt and when
`AMgr.options.maxBlockVisitOnPath - 1` equals to `blockCount`, it can't
widen. After next iteration, `BlockCount >=
AMgr.options.maxBlockVisitOnPath` holds and generate a sink node. Add
`CXXForRangeStmt` makes it work.
Co-authored-by: huqizhi <836744285@qq.com>
structured-block
where clause is one of the following:
private(list)
reduction([reduction-modifier ,] reduction-identifier : list)
nowait
Differential Revision: https://reviews.llvm.org/D157933
I'm involved with the Static Analyzer for the most part.
I think we should embrace newer language standard features and gradually
move forward.
Differential Revision: https://reviews.llvm.org/D154325
This patch introduces a new `CXXLifetimeExtendedObjectRegion` as a representation
of the memory for the temporary object that is lifetime extended by the reference
to which they are bound.
This separation provides an ability to detect the use of dangling pointers
(either binding or dereference) in a robust manner.
For example, the `ref` is conditionally dangling in the following example:
```
template<typename T>
T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
int const& le = Composite{}.x;
auto&& ref = select(cond, le, 10);
```
Before the change, regardless of the value of `cond`, the `select()` call would
have returned a `temp_object` region.
With the proposed change we would produce a (non-dangling) `lifetime_extended_object`
region with lifetime bound to `le` or a `temp_object` region for the dangling case.
We believe that such separation is desired, as such lifetime extended temporaries
are closer to the variables. For example, they may have a static storage duration
(this patch removes a static temporary region, which was an abomination).
We also think that alternative approaches are not viable.
While for some cases it may be possible to determine if the region is lifetime
extended by searching the parents of the initializer expr, this quickly becomes
complex in the presence of the conditions operators like this one:
```
Composite cc;
// Ternary produces prvalue 'int' which is extended, as branches differ in value category
auto&& x = cond ? Composite{}.x : cc.x;
// Ternary produces xvalue, and extends the Composite object
auto&& y = cond ? Composite{}.x : std::move(cc).x;
```
Finally, the lifetime of the `CXXLifetimeExtendedObjectRegion` is tied to the lifetime of
the corresponding variables, however, the "liveness" (or reachability) of the extending
variable does not imply the reachability of all symbols in the region.
In conclusion `CXXLifetimeExtendedObjectRegion`, in contrast to `VarRegions`, does not
need any special handling in `SymReaper`.
RFC: https://discourse.llvm.org/t/rfc-detecting-uses-of-dangling-references/70731
Reviewed By: xazax.hun
Differential Revision: https://reviews.llvm.org/D151325
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
This commit relands the patches for implementing P0960R3 and P1975R0,
which describe initializing aggregates via a parenthesized list.
The relanded commits are:
* 40c52159d3ee - P0960R3 and P1975R0: Allow initializing aggregates from
a parenthesized list of values
* c77a91bb7ba7 - Remove overly restrictive aggregate paren init logic
* 32d7aae04fdb - Fix a clang crash on invalid code in C++20 mode
This patch also fixes a crash in the original implementation.
Previously, if the input tried to call an implicitly deleted copy or
move constructor of a union, we would then try to initialize the union
by initializing it's first element with a reference to a union. This
behavior is incorrect (we should fail to initialize) and if the type of
the first element has a constructor with a single template typename
parameter, then Clang will explode. This patch fixes that issue by
checking that constructor overload resolution did not result in a
deleted function before attempting parenthesized aggregate
initialization.
Additionally, this patch also includes D140159, which contains some
minor fixes made in response to code review comments in the original
implementation that were made after that patch was submitted.
Co-authored-by: Sheng <ox59616e@gmail.com>
Fixes#54040, Fixes#59675
Reviewed By: ilya-biryukov
Differential Revision: https://reviews.llvm.org/D141546
When the engine processes a store to a variable, it will eventually call
`ExprEngine::processPointerEscapedOnBind()`. This function is supposed to
invalidate (put the given locations to an escape list) the locations
which we cannot reason about.
Unfortunately, local static variables are also put into this list.
This patch relaxes the guard condition, so that beyond stack variables,
static local variables are also ignored.
Differential Revision: https://reviews.llvm.org/D139534
This feature causes clang to crash when compiling Chrome - see
https://crbug.com/1405031 and
https://github.com/llvm/llvm-project/issues/59675
Revert "[clang] Fix a clang crash on invalid code in C++20 mode."
This reverts commit 32d7aae04fdb58e65a952f281ff2f2c3f396d98f.
Revert "[clang] Remove overly restrictive aggregate paren init logic"
This reverts commit c77a91bb7ba793ec3a6a5da3743ed55056291658.
Revert "[clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values"
This reverts commit 40c52159d3ee337dbed14e4c73b5616ea354c337.
This avoids the continuous API churn when upgrading things to use
std::optional and makes trivial string replace upgrades possible.
I tested this with GCC 7.5, the oldest supported GCC I had around.
Differential Revision: https://reviews.llvm.org/D140332
This patch implements P0960R3, which allows initialization of aggregates
via parentheses.
As an example:
```
struct S { int i, j; };
S s1(1, 1);
int arr1[2](1, 2);
```
This patch also implements P1975R0, which fixes the wording of P0960R3
for single-argument parenthesized lists so that statements like the
following are allowed:
```
S s2(1);
S s3 = static_cast<S>(1);
S s4 = (S)1;
int (&&arr2)[] = static_cast<int[]>(1);
int (&&arr3)[2] = static_cast<int[2]>(1);
```
This patch was originally authored by @0x59616e and completed by
@ayzhao.
Fixes#54040, Fixes#54041
Co-authored-by: Sheng <ox59616e@gmail.com>
Full write up : https://discourse.llvm.org/t/c-20-rfc-suggestion-desired-regarding-the-implementation-of-p0960r3/63744
Reviewed By: ilya-biryukov
Differential Revision: https://reviews.llvm.org/D129531
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
We now skip the destruction of array elements for `delete[] p`,
if the value of `p` is UnknownVal and does not have corresponding region.
This eliminate the crash in `getDynamicElementCount` on that
region and matches the behavior for deleting the array of
non-constant range.
Reviewed By: isuckatcs
Differential Revision: https://reviews.llvm.org/D136671
This was done as a test for D137302 and it makes sense to push these changes
Reviewed By: shafik
Differential Revision: https://reviews.llvm.org/D137491
