| Age | Commit message (Collapse) | Author |
|---|
|
Reverts llvm/llvm-project#93612 due to the issues with ppc64le platform.
|
|
Without this patch compiler-rt ubsan library has a bug displaying
incorrect values for variables of the _BitInt (previously called
_ExtInt) type. This patch affects affects both: generation of metadata
inside code generator and runtime part. The runtime part provided only
for i386 and x86_64 runtimes. Other runtimes should be updated to take
full benefit of this patch.
The patch is constructed the way to be backward compatible and int and
float type runtime diagnostics should be unaffected for not yet updated
runtimes.
This patch fixes issue:
https://github.com/llvm/llvm-project/issues/64100.
Co-authored-by: Vladislav Aranov <vladislav.aranov@ericsson.com>
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
|
|
Instruction-creation (#94226)
This patch simplifies instruction creation by replacing all overloads of
instruction constructors/Create methods that are identical other than
the Instruction *InsertBefore/BasicBlock *InsertAtEnd/BasicBlock::iterator
InsertBefore argument with a single version that takes an InsertPosition
argument. The InsertPosition class can be implicitly constructed from
any of the above, internally converting them to the appropriate
BasicBlock::iterator value which can then be used to insert the
instruction (or to not insert it if an invalid iterator is passed).
The upshot of this is that code will be deduplicated, and all callsites
will switch to calling the new unified version without any changes
needed to make the compiler happy. There is at least one exception to
this; the construction of InsertPosition is a user-defined conversion,
so any caller that was already relying on a different user-defined
conversion won't work. In all of LLVM and Clang this happens exactly
once: at clang/lib/CodeGen/CGExpr.cpp:123 we try to construct an alloca
with an AssertingVH<Instruction> argument, which must now be cast to an
Instruction* by using `&*`. If this is more common elsewhere, it could
be fixed by adding an appropriate constructor to InsertPosition.
|
|
llvm::hash_value is not guaranteed to be deterministic. Use the
deterministic xxh3_64bits. A strong bit mixer isn't necessary. Use a
simpler one that works well with pointers.
|
|
This should simplify handling of resulting value by the callers.
|
|
It does not look like particular value is inportant.
Howere, there is a comment., but the current implementation
of `create{Unlikely,Likely}BranchWeights` use the same value.
Follow up to #89464
|
|
This issue was found in https://github.com/llvm/llvm-project/pull/86960.
But I'd like to avoid mixing together a bunch of cleanups with actual
changes.
Signed-off-by: yronglin <yronglin777@gmail.com>
|
|
The parameter of `getAddress()` was not being used, as I recall, and
seems to have been removed in the meantime. Merging this w/o review
since this is breaking builds.
|
|
lambda (#89828)
Consider this code:
```c++
template <typename... Ts>
struct Overloaded : Ts... { using Ts::operator()...; };
template <typename... Ts>
Overloaded(Ts...) -> Overloaded<Ts...>;
void f() {
int x;
Overloaded o {
[&](this auto& self) {
return &x;
}
};
o();
}
```
To access `x` in the lambda, we need to perform derived-to-base
conversion on `self` (since the type of `self` is not the lambda type,
but rather `Overloaded<(lambda type)>`). We were previously missing this
step, causing us to attempt to load the entire lambda (as the base
class, it would end up being the ‘field’ with index `0` here), which
would then assert later on in codegen.
Moreover, this is only valid in the first place if there is a unique and
publicly accessible cast path from the derived class to the lambda’s
type, so this also adds a check in Sema to diagnose problematic
cases.
This fixes #87210 and fixes #89541.
|
|
operator (#91965)
The patch at https://reviews.llvm.org/D122732 introduced using the array
subscript operator for SVE vectors, however it also causes an ICE when
the subscripting expression is used as an lvalue.
This patches fixes the error. Lvalue subscripting expressions are
emitted as LLVM IR `insertelement`.
|
|
This is in effect a revert of f139ae3d93797, as we have since gained a
more sophisticated way of doing extra IRGen with the addition of
RawAddress in #86923.
|
|
Latest diff:
https://github.com/llvm/llvm-project/pull/89154/files/f1ab4c2677394bbfc985d9680d5eecd7b2e6a882..adf9bc902baddb156c83ce0f8ec03c142e806d45
We address two additional bugs here:
### Problem 1: Deactivated normal cleanup still runs, leading to
double-free
Consider the following:
```cpp
struct A { };
struct B { B(const A&); };
struct S {
A a;
B b;
};
int AcceptS(S s);
void Accept2(int x, int y);
void Test() {
Accept2(AcceptS({.a = A{}, .b = A{}}), ({ return; 0; }));
}
```
We add cleanups as follows:
1. push dtor for field `S::a`
2. push dtor for temp `A{}` (used by ` B(const A&)` in `.b = A{}`)
3. push dtor for field `S::b`
4. Deactivate 3 `S::b`-> This pops the cleanup.
5. Deactivate 1 `S::a` -> Does not pop the cleanup as *2* is top. Should
create _active flag_!!
6. push dtor for `~S()`.
7. ...
It is important to deactivate **5** using active flags. Without the
active flags, the `return` will fallthrough it and would run both `~S()`
and dtor `S::a` leading to **double free** of `~A()`.
In this patch, we unconditionally emit active flags while deactivating
normal cleanups. These flags are deleted later by the `AllocaTracker` if
the cleanup is not emitted.
### Problem 2: Missing cleanup for conditional lifetime extension
We push 2 cleanups for lifetime-extended cleanup. The first cleanup is
useful if we exit from the middle of the expression (stmt-expr/coro
suspensions). This is deactivated after full-expr, and a new cleanup is
pushed, extending the lifetime of the temporaries (to the scope of the
reference being initialized).
If this lifetime extension happens to be conditional, then we use active
flags to remember whether the branch was taken and if the object was
initialized.
Previously, we used a **single** active flag, which was used by both
cleanups. This is wrong because the first cleanup will be forced to
deactivate after the full-expr and therefore this **active** flag will
always be **inactive**. The dtor for the lifetime extended entity would
not run as it always sees an **inactive** flag.
In this patch, we solve this using two separate active flags for both
cleanups. Both of them are activated if the conditional branch is taken,
but only one of them is deactivated after the full-expr.
---
Fixes https://github.com/llvm/llvm-project/issues/63818
Fixes https://github.com/llvm/llvm-project/issues/88478
---
Previous PR logs:
1. https://github.com/llvm/llvm-project/pull/85398
2. https://github.com/llvm/llvm-project/pull/88670
3. https://github.com/llvm/llvm-project/pull/88751
4. https://github.com/llvm/llvm-project/pull/88884
|
|
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.
|
|
We always capitalize bitfield as "BitField".
|
|
and related commits (#88884)
The original change caused widespread breakages in msan/ubsan tests and
causes `use-after-free`. Most likely we are adding more cleanups than
necessary.
|
|
(#85398)
Fixes https://github.com/llvm/llvm-project/issues/63818 for control flow
out of an expressions.
#### Background
A control flow could happen in the middle of an expression due to
stmt-expr and coroutine suspensions.
Due to branch-in-expr, we missed running cleanups for the temporaries
constructed in the expression before the branch.
Previously, these cleanups were only added as `EHCleanup` during the
expression and as normal expression after the full expression.
Examples of such deferred cleanups include:
`ParenList/InitList`: Cleanups for fields are performed by the
destructor of the object being constructed.
`Array init`: Cleanup for elements of an array is included in the array
cleanup.
`Lifetime-extended temporaries`: reference-binding temporaries in
braced-init are lifetime extended to the parent scope.
`Lambda capture init`: init in the lambda capture list is destroyed by
the lambda object.
---
#### In this PR
In this PR, we change some of the `EHCleanups` cleanups to
`NormalAndEHCleanups` to make sure these are emitted when we see a
branch inside an expression (through statement expressions or coroutine
suspensions).
These are supposed to be deactivated after full expression and destroyed
later as part of the destructor of the aggregate or array being
constructed. To simplify deactivating cleanups, we add two utilities as
well:
* `DeferredDeactivationCleanupStack`: A stack to remember cleanups with
deferred deactivation.
* `CleanupDeactivationScope`: RAII for deactivating cleanups added to
the above stack.
---
#### Deactivating normal cleanups
These were previously `EHCleanups` and not `Normal` and **deactivation**
of **required** `Normal` cleanups had some bugs. These specifically
include deactivating `Normal` cleanups which are not the top of
`EHStack`
[source1](https://github.com/llvm/llvm-project/blob/92b56011e6b61e7dc1628c0431ece432f282b3cb/clang/lib/CodeGen/CGCleanup.cpp#L1319),
[2](https://github.com/llvm/llvm-project/blob/92b56011e6b61e7dc1628c0431ece432f282b3cb/clang/lib/CodeGen/CGCleanup.cpp#L722-L746).
This has not been part of our test suite (maybe it was never required
before statement expressions). In this PR, we also fix the emission of
required-deactivated-normal cleanups.
|
|
Fix since #75481 got reverted.
- Explicitly set BitfieldBits to 0 to avoid uninitialized field member
for the integer checks:
```diff
- llvm::ConstantInt::get(Builder.getInt8Ty(), Check.first)};
+ llvm::ConstantInt::get(Builder.getInt8Ty(), Check.first),
+ llvm::ConstantInt::get(Builder.getInt32Ty(), 0)};
```
- `Value **Previous` was erroneously `Value *Previous` in
`CodeGenFunction::EmitWithOriginalRHSBitfieldAssignment`, fixed now.
- Update following:
```diff
- if (Kind == CK_IntegralCast) {
+ if (Kind == CK_IntegralCast || Kind == CK_LValueToRValue) {
```
CK_LValueToRValue when going from, e.g., char to char, and
CK_IntegralCast otherwise.
- Make sure that `Value *Previous = nullptr;` is initialized (see
https://github.com/llvm/llvm-project/commit/1189e87951e59a81ee097eae847c06008276fef1)
- Add another extensive testcase
`ubsan/TestCases/ImplicitConversion/bitfield-conversion.c`
---------
Co-authored-by: Vitaly Buka <vitalybuka@gmail.com>
|
|
Intrinsic inserted into CodeGenFunction::EmitCheck, which
is not mostly used by CFI.
CFI is not the goal, and fixing inconsistencies with CFI is outside of
the cope of the patch.
RFC:
https://discourse.llvm.org/t/rfc-add-llvm-experimental-hot-intrinsic-or-llvm-hot/77641
|
|
bitfields""" (#87562)
Reverts llvm/llvm-project#87529
Reverts #87518
https://lab.llvm.org/buildbot/#/builders/37/builds/33262 is still broken
|
|
(#87529)
Reverts llvm/llvm-project#87518
Revert is not needed as the regression was fixed with
1189e87951e59a81ee097eae847c06008276fef1.
I assumed the crash and warning are different issues, but according to
https://lab.llvm.org/buildbot/#/builders/240/builds/26629
fixing warning resolves the crash.
|
|
Reverts llvm/llvm-project#75481
Breaks multiple bots, see #75481
|
|
This patch fixes:
clang/lib/CodeGen/CGExpr.cpp:5607:11: error: variable 'Result' is
used uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
|
|
This patch implements the implicit truncation and implicit sign change
checks for bitfields using UBSan. E.g.,
`-fsanitize=implicit-bitfield-truncation` and
`-fsanitize=implicit-bitfield-sign-change`.
|
|
Extracted from #84858
|
|
HLSL constant sized array function parameters do not decay to pointers.
Instead constant sized array types are preserved as unique types for
overload resolution, template instantiation and name mangling.
This implements the change by adding a new `ArrayParameterType` which
represents a non-decaying `ConstantArrayType`. The new type behaves the
same as `ConstantArrayType` except that it does not decay to a pointer.
Values of `ConstantArrayType` in HLSL decay during overload resolution
via a new `HLSLArrayRValue` cast to `ArrayParameterType`.
`ArrayParamterType` values are passed indirectly by-value to functions
in IR generation resulting in callee generated memcpy instructions.
The behavior of HLSL function calls is documented in the [draft language
specification](https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf)
under the Expr.Post.Call heading.
Additionally the design of this implementation approach is documented in
[Clang's
documentation](https://clang.llvm.org/docs/HLSL/FunctionCalls.html)
Resolves #70123
|
|
to authenticate signed pointers (#86923)
To authenticate pointers, CodeGen needs access to the key and
discriminators that were used to sign the pointer. That information is
sometimes known from the context, but not always, which is why `Address`
needs to hold that information.
This patch adds methods and data members to `Address`, which will be
needed in subsequent patches to authenticate signed pointers, and uses
the newly added methods throughout CodeGen. Although this patch isn't
strictly NFC as it causes CodeGen to use different code paths in some
cases (e.g., `mergeAddressesInConditionalExpr`), it doesn't cause any
changes in functionality as it doesn't add any information needed for
authentication.
In addition to the changes mentioned above, this patch introduces class
`RawAddress`, which contains a pointer that we know is unsigned, and
adds several new functions for creating `Address` and `LValue` objects.
This reapplies d9a685a9dd589486e882b722e513ee7b8c84870c, which was
reverted because it broke ubsan bots. There seems to be a bug in
coroutine code-gen, which is causing EmitTypeCheck to use the wrong
alignment. For now, pass alignment zero to EmitTypeCheck so that it can
compute the correct alignment based on the passed type (see function
EmitCXXMemberOrOperatorMemberCallExpr).
|
|
needed to authenticate signed pointers (#86721)" (#86898)
This reverts commit d9a685a9dd589486e882b722e513ee7b8c84870c.
The commit broke ubsan bots.
|
|
to authenticate signed pointers (#86721)
To authenticate pointers, CodeGen needs access to the key and
discriminators that were used to sign the pointer. That information is
sometimes known from the context, but not always, which is why `Address`
needs to hold that information.
This patch adds methods and data members to `Address`, which will be
needed in subsequent patches to authenticate signed pointers, and uses
the newly added methods throughout CodeGen. Although this patch isn't
strictly NFC as it causes CodeGen to use different code paths in some
cases (e.g., `mergeAddressesInConditionalExpr`), it doesn't cause any
changes in functionality as it doesn't add any information needed for
authentication.
In addition to the changes mentioned above, this patch introduces class
`RawAddress`, which contains a pointer that we know is unsigned, and
adds several new functions for creating `Address` and `LValue` objects.
This reapplies 8bd1f9116aab879183f34707e6d21c7051d083b6. The commit
broke msan bots because LValue::IsKnownNonNull was uninitialized.
|
|
(#78253)
…i_check
This causes __cfi_check, just as __cfi_check_fail, to get the proper
target-specific attributes, in particular uwtable for unwind table
generation. Previously, nounwind attribute could be inferred for
__cfi_check, which caused it to lose its unwind table even with
-funwind-table option.
~~
Huawei RRI, OS Lab
Co-authored-by: Nikolai Kholiavin <kholiavin.nikolai@huawei-partners.com>
|
|
needed to authenticate signed pointers (#67454)" (#86674)
This reverts commit 8bd1f9116aab879183f34707e6d21c7051d083b6.
It appears that the commit broke msan bots.
|
|
to authenticate signed pointers (#67454)
To authenticate pointers, CodeGen needs access to the key and
discriminators that were used to sign the pointer. That information is
sometimes known from the context, but not always, which is why `Address`
needs to hold that information.
This patch adds methods and data members to `Address`, which will be
needed in subsequent patches to authenticate signed pointers, and uses
the newly added methods throughout CodeGen. Although this patch isn't
strictly NFC as it causes CodeGen to use different code paths in some
cases (e.g., `mergeAddressesInConditionalExpr`), it doesn't cause any
changes in functionality as it doesn't add any information needed for
authentication.
In addition to the changes mentioned above, this patch introduces class
`RawAddress`, which contains a pointer that we know is unsigned, and
adds several new functions for creating `Address` and `LValue` objects.
|
|
'CountAttributedType' (#78000)
In `-fbounds-safety`, bounds annotations are considered type attributes
rather than declaration attributes. Constructing them as type attributes
allows us to extend the attribute to apply nested pointers, which is
essential to annotate functions that involve out parameters: `void
foo(int *__counted_by(*out_count) *out_buf, int *out_count)`.
We introduce a new sugar type to support bounds annotated types,
`CountAttributedType`. In order to maintain extra data (the bounds
expression and the dependent declaration information) that is not
trackable in `AttributedType` we create a new type dedicate to this
functionality.
This patch also extends the parsing logic to parse the `counted_by`
argument as an expression, which will allow us to extend the model to
support arguments beyond an identifier, e.g., `__counted_by(n + m)` in
the future as specified by `-fbounds-safety`.
This also adjusts `__bdos` and array-bounds sanitizer code that already
uses `CountedByAttr` to check `CountAttributedType` instead to get the
field referred to by the attribute.
|
|
HLSL supports vector truncation and element conversions as part of
standard conversion sequences. The vector truncation conversion is a C++
second conversion in the conversion sequence. If a vector truncation is
in a conversion sequence an element conversion may occur after it before
the standard C++ third conversion.
Vector element conversions can be boolean conversions, floating point or
integral conversions or promotions.
[HLSL Draft
Specification](https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf)
---------
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
|
|
This re-applies 30155fc0 with a fix for clangd.
### Description
clang don't evaluate the object argument of `static operator()` and
`static operator[]` currently, for example:
```cpp
#include <iostream>
struct Foo {
static int operator()(int x, int y) {
std::cout << "Foo::operator()" << std::endl;
return x + y;
}
static int operator[](int x, int y) {
std::cout << "Foo::operator[]" << std::endl;
return x + y;
}
};
Foo getFoo() {
std::cout << "getFoo()" << std::endl;
return {};
}
int main() {
std::cout << getFoo()(1, 2) << std::endl;
std::cout << getFoo()[1, 2] << std::endl;
}
```
`getFoo()` is expected to be called, but clang don't call it currently
(17.0.6). This PR fixes this issue.
Fixes #67976, reland #68485.
### Walkthrough
- **clang/lib/Sema/SemaOverload.cpp**
- **`Sema::CreateOverloadedArraySubscriptExpr` &
`Sema::BuildCallToObjectOfClassType`**
Previously clang generate `CallExpr` for static operators, ignoring the
object argument. In this PR `CXXOperatorCallExpr` is generated for
static operators instead, with the object argument as the first
argument.
- **`TryObjectArgumentInitialization`**
`const` / `volatile` objects are allowed for static methods, so that we
can call static operators on them.
- **clang/lib/CodeGen/CGExpr.cpp**
- **`CodeGenFunction::EmitCall`**
CodeGen changes for `CXXOperatorCallExpr` with static operators: emit
and ignore the object argument first, then emit the operator call.
- **clang/lib/AST/ExprConstant.cpp**
- **`ExprEvaluatorBase::handleCallExpr`**
Evaluation of static operators in constexpr also need some small changes
to work, so that the arguments won't be out of position.
- **clang/lib/Sema/SemaChecking.cpp**
- **`Sema::CheckFunctionCall`**
Code for argument checking also need to be modify, or it will fail the
test `clang/test/SemaCXX/overloaded-operator-decl.cpp`.
- **clang-tools-extra/clangd/InlayHints.cpp**
- **`InlayHintVisitor::VisitCallExpr`**
Now that the `CXXOperatorCallExpr` for static operators also have object
argument, we should also take care of this situation in clangd.
### Tests
- **Added:**
- **clang/test/AST/ast-dump-static-operators.cpp**
Verify the AST generated for static operators.
- **clang/test/SemaCXX/cxx2b-static-operator.cpp**
Static operators should be able to be called on const / volatile
objects.
- **Modified:**
- **clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp**
- **clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp**
Matching the new CodeGen.
### Documentation
- **clang/docs/ReleaseNotes.rst**
Update release notes.
---------
Co-authored-by: Shafik Yaghmour <shafik@users.noreply.github.com>
Co-authored-by: cor3ntin <corentinjabot@gmail.com>
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
|
|
This reverts commit 30155fc0ef4fbdce2d79434aaae8d58b2fabb20a.
It seems to have broken some tests in clangd:
http://45.33.8.238/linux/129484/step_9.txt
|
|
### Description
clang don't evaluate the object argument of `static operator()` and
`static operator[]` currently, for example:
```cpp
#include <iostream>
struct Foo {
static int operator()(int x, int y) {
std::cout << "Foo::operator()" << std::endl;
return x + y;
}
static int operator[](int x, int y) {
std::cout << "Foo::operator[]" << std::endl;
return x + y;
}
};
Foo getFoo() {
std::cout << "getFoo()" << std::endl;
return {};
}
int main() {
std::cout << getFoo()(1, 2) << std::endl;
std::cout << getFoo()[1, 2] << std::endl;
}
```
`getFoo()` is expected to be called, but clang don't call it currently
(17.0.2). This PR fixes this issue.
Fixes #67976.
### Walkthrough
- **clang/lib/Sema/SemaOverload.cpp**
- **`Sema::CreateOverloadedArraySubscriptExpr` &
`Sema::BuildCallToObjectOfClassType`**
Previously clang generate `CallExpr` for static operators, ignoring the
object argument. In this PR `CXXOperatorCallExpr` is generated for
static operators instead, with the object argument as the first
argument.
- **`TryObjectArgumentInitialization`**
`const` / `volatile` objects are allowed for static methods, so that we
can call static operators on them.
- **clang/lib/CodeGen/CGExpr.cpp**
- **`CodeGenFunction::EmitCall`**
CodeGen changes for `CXXOperatorCallExpr` with static operators: emit
and ignore the object argument first, then emit the operator call.
- **clang/lib/AST/ExprConstant.cpp**
- **`ExprEvaluatorBase::handleCallExpr`**
Evaluation of static operators in constexpr also need some small changes
to work, so that the arguments won't be out of position.
- **clang/lib/Sema/SemaChecking.cpp**
- **`Sema::CheckFunctionCall`**
Code for argument checking also need to be modify, or it will fail the
test `clang/test/SemaCXX/overloaded-operator-decl.cpp`.
### Tests
- **Added:**
- **clang/test/AST/ast-dump-static-operators.cpp**
Verify the AST generated for static operators.
- **clang/test/SemaCXX/cxx2b-static-operator.cpp**
Static operators should be able to be called on const / volatile
objects.
- **Modified:**
- **clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp**
- **clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp**
Matching the new CodeGen.
### Documentation
- **clang/docs/ReleaseNotes.rst**
Update release notes.
---------
Co-authored-by: Shafik Yaghmour <shafik@users.noreply.github.com>
Co-authored-by: cor3ntin <corentinjabot@gmail.com>
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
|
|
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.
|
|
This fixes:
```
[3963/6996] Building CXX object tools\clang\lib\CodeGen\CMakeFiles\obj.clangCodeGen.dir\CGExpr.cpp.obj
C:\git\llvm-project\clang\lib\CodeGen\CGExpr.cpp(3808): warning C4018: '<=': signed/unsigned mismatch
```
|
|
scalar type. (#78041)
Previously committed as 9e08e51a20d0d2b1c5724bb17e969d036fced4cd, and
reverted because a dependency commit was reverted, then committed again
as 4b574008aef5a7235c1f894ab065fe300d26e786 and reverted again because
"dependency commit" 5a391d38ac6c561ba908334d427f26124ed9132e was
reverted. But it doesn't seem that 5a391d38ac6c was a real dependency
for this.
This commit incorporates 4b574008aef5a7235c1f894ab065fe300d26e786 and
18e093faf726d15f210ab4917142beec51848258 by Richard Smith (@zygoloid),
with some minor fixes, most notably:
- `UncommonValue` renamed to `StructuralValue`
- `VK_PRValue` instead of `VK_RValue` as default kind in lvalue and
member pointer handling branch in
`BuildExpressionFromNonTypeTemplateArgumentValue`;
- handling of `StructuralValue` in `IsTypeDeclaredInsideVisitor`;
- filling in `SugaredConverted` along with `CanonicalConverted`
parameter in `Sema::CheckTemplateArgument`;
- minor cleanup in
`TemplateInstantiator::transformNonTypeTemplateParmRef`;
- `TemplateArgument` constructors refactored;
- `ODRHash` calculation for `UncommonValue`;
- USR generation for `UncommonValue`;
- more correct MS compatibility mangling algorithm (tested on MSVC ver.
19.35; toolset ver. 143);
- IR emitting fixed on using a subobject as a template argument when the
corresponding template parameter is used in an lvalue context;
- `noundef` attribute and opaque pointers in `template-arguments` test;
- analysis for C++17 mode is turned off for templates in
`warn-bool-conversion` test; in C++17 and C++20 mode, array reference
used as a template argument of pointer type produces template argument
of UncommonValue type, and
`BuildExpressionFromNonTypeTemplateArgumentValue` makes
`OpaqueValueExpr` for it, and `DiagnoseAlwaysNonNullPointer` cannot see
through it; despite of "These cases should not warn" comment, I'm not
sure about correct behavior; I'd expect a suggestion to replace `if` by
`if constexpr`;
- `temp.arg.nontype/p1.cpp` and `dr18xx.cpp` tests fixed.
|
|
The 'counted_by' attribute is used on flexible array members. The
argument for the attribute is the name of the field member holding the
count of elements in the flexible array. This information is used to
improve the results of the array bound sanitizer and the
'__builtin_dynamic_object_size' builtin. The 'count' field member must
be within the same non-anonymous, enclosing struct as the flexible array
member. For example:
```
struct bar;
struct foo {
int count;
struct inner {
struct {
int count; /* The 'count' referenced by 'counted_by' */
};
struct {
/* ... */
struct bar *array[] __attribute__((counted_by(count)));
};
} baz;
};
```
This example specifies that the flexible array member 'array' has the
number of elements allocated for it in 'count':
```
struct bar;
struct foo {
size_t count;
/* ... */
struct bar *array[] __attribute__((counted_by(count)));
};
```
This establishes a relationship between 'array' and 'count';
specifically that 'p->array' must have *at least* 'p->count' number of
elements available. It's the user's responsibility to ensure that this
relationship is maintained throughout changes to the structure.
In the following, the allocated array erroneously has fewer elements
than what's specified by 'p->count'. This would result in an
out-of-bounds access not not being detected:
```
struct foo *p;
void foo_alloc(size_t count) {
p = malloc(MAX(sizeof(struct foo),
offsetof(struct foo, array[0]) + count *
sizeof(struct bar *)));
p->count = count + 42;
}
```
The next example updates 'p->count', breaking the relationship
requirement that 'p->array' must have at least 'p->count' number of
elements available:
```
void use_foo(int index, int val) {
p->count += 42;
p->array[index] = val; /* The sanitizer can't properly check this access */
}
```
In this example, an update to 'p->count' maintains the relationship
requirement:
```
void use_foo(int index, int val) {
if (p->count == 0)
return;
--p->count;
p->array[index] = val;
}
```
|
|
This reverts commit 164f85db876e61cf4a3c34493ed11e8f5820f968.
|
|
The 'counted_by' attribute is used on flexible array members. The
argument for the attribute is the name of the field member holding the
count of elements in the flexible array. This information is used to
improve the results of the array bound sanitizer and the
'__builtin_dynamic_object_size' builtin. The 'count' field member must
be within the same non-anonymous, enclosing struct as the flexible array
member. For example:
```
struct bar;
struct foo {
int count;
struct inner {
struct {
int count; /* The 'count' referenced by 'counted_by' */
};
struct {
/* ... */
struct bar *array[] __attribute__((counted_by(count)));
};
} baz;
};
```
This example specifies that the flexible array member 'array' has the
number of elements allocated for it in 'count':
```
struct bar;
struct foo {
size_t count;
/* ... */
struct bar *array[] __attribute__((counted_by(count)));
};
```
This establishes a relationship between 'array' and 'count';
specifically that 'p->array' must have *at least* 'p->count' number of
elements available. It's the user's responsibility to ensure that this
relationship is maintained throughout changes to the structure.
In the following, the allocated array erroneously has fewer elements
than what's specified by 'p->count'. This would result in an
out-of-bounds access not not being detected:
```
struct foo *p;
void foo_alloc(size_t count) {
p = malloc(MAX(sizeof(struct foo),
offsetof(struct foo, array[0]) + count *
sizeof(struct bar *)));
p->count = count + 42;
}
```
The next example updates 'p->count', breaking the relationship
requirement that 'p->array' must have at least 'p->count' number of
elements available:
```
void use_foo(int index, int val) {
p->count += 42;
p->array[index] = val; /* The sanitizer can't properly check this access */
}
```
In this example, an update to 'p->count' maintains the relationship
requirement:
```
void use_foo(int index, int val) {
if (p->count == 0)
return;
--p->count;
p->array[index] = val;
}
```
|
|
This reverts commit fefdef808c230c79dca2eb504490ad0f17a765a5.
Breaks check-clang, see
https://github.com/llvm/llvm-project/pull/76348#issuecomment-1886029515
Also revert follow-on "[Clang] Update 'counted_by' documentation"
This reverts commit 4a3fb9ce27dda17e97341f28005a28836c909cfc.
|
|
The 'counted_by' attribute is used on flexible array members. The
argument for the attribute is the name of the field member holding the
count of elements in the flexible array. This information is used to
improve the results of the array bound sanitizer and the
'__builtin_dynamic_object_size' builtin. The 'count' field member must
be within the same non-anonymous, enclosing struct as the flexible array
member. For example:
```
struct bar;
struct foo {
int count;
struct inner {
struct {
int count; /* The 'count' referenced by 'counted_by' */
};
struct {
/* ... */
struct bar *array[] __attribute__((counted_by(count)));
};
} baz;
};
```
This example specifies that the flexible array member 'array' has the
number of elements allocated for it in 'count':
```
struct bar;
struct foo {
size_t count;
/* ... */
struct bar *array[] __attribute__((counted_by(count)));
};
```
This establishes a relationship between 'array' and 'count';
specifically that 'p->array' must have *at least* 'p->count' number of
elements available. It's the user's responsibility to ensure that this
relationship is maintained throughout changes to the structure.
In the following, the allocated array erroneously has fewer elements
than what's specified by 'p->count'. This would result in an
out-of-bounds access not not being detected:
```
struct foo *p;
void foo_alloc(size_t count) {
p = malloc(MAX(sizeof(struct foo),
offsetof(struct foo, array[0]) + count *
sizeof(struct bar *)));
p->count = count + 42;
}
```
The next example updates 'p->count', breaking the relationship
requirement that 'p->array' must have at least 'p->count' number of
elements available:
```
void use_foo(int index, int val) {
p->count += 42;
p->array[index] = val; /* The sanitizer can't properly check this access */
}
```
In this example, an update to 'p->count' maintains the relationship
requirement:
```
void use_foo(int index, int val) {
if (p->count == 0)
return;
--p->count;
p->array[index] = val;
}
```
|
|
There are many issues that popped up with the counted_by feature. The
patch #73730 has grown too large and approval is blocking Linux testing.
Includes reverts of:
commit 769bc11f684d ("[Clang] Implement the 'counted_by' attribute
(#68750)")
commit bc09ec696209 ("[CodeGen] Revamp counted_by calculations
(#70606)")
commit 1a09cfb2f35d ("[Clang] counted_by attr can apply only to C99
flexible array members (#72347)")
commit a76adfb992c6 ("[NFC][Clang] Refactor code to calculate flexible
array member size (#72790)")
commit d8447c78ab16 ("[Clang] Correct handling of negative and
out-of-bounds indices (#71877)")
Partial commit b31cd07de5b7 ("[Clang] Regenerate test checks (NFC)")
Closes #73168
Closes #75173
|
|
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.
|
|
This commit adds a new BPF specific structure attribte
`__attribute__((preserve_static_offset))` and a pass to deal with it.
This attribute may be attached to a struct or union declaration, where
it notifies the compiler that this structure is a "context" structure.
The following limitations apply to context structures:
- runtime environment might patch access to the fields of this type by
updating the field offset;
BPF verifier limits access patterns allowed for certain data
types. E.g. `struct __sk_buff` and `struct bpf_sock_ops`. For these
types only `LD/ST <reg> <static-offset>` memory loads and stores are
allowed.
This is so because offsets of the fields of these structures do not
match real offsets in the running kernel. During BPF program
load/verification loads and stores to the fields of these types are
rewritten so that offsets match real offsets. For this rewrite to
happen static offsets have to be encoded in the instructions.
See `kernel/bpf/verifier.c:convert_ctx_access` function in the Linux
kernel source tree for details.
- runtime environment might disallow access to the field of the type
through modified pointers.
During BPF program verification a tag `PTR_TO_CTX` is tracked for
register values. In case if register with such tag is modified BPF
programs are not allowed to read or write memory using register. See
kernel/bpf/verifier.c:check_mem_access function in the Linux kernel
source tree for details.
Access to the structure fields is translated to IR as a sequence:
- `(load (getelementptr %ptr %offset))` or
- `(store (getelementptr %ptr %offset))`
During instruction selection phase such sequences are translated as a
single load instruction with embedded offset, e.g. `LDW %ptr, %offset`,
which matches access pattern necessary for the restricted
set of types described above (when `%offset` is static).
Multiple optimizer passes might separate these instructions, this
includes:
- SimplifyCFGPass (sinking)
- InstCombine (sinking)
- GVN (hoisting)
The `preserve_static_offset` attribute marks structures for which the
following transformations happen:
- at the early IR processing stage:
- `(load (getelementptr ...))` replaced by call to intrinsic
`llvm.bpf.getelementptr.and.load`;
- `(store (getelementptr ...))` replaced by call to intrinsic
`llvm.bpf.getelementptr.and.store`;
- at the late IR processing stage this modification is undone.
Such handling prevents various optimizer passes from generating
sequences of instructions that would be rejected by BPF verifier.
The __attribute__((preserve_static_offset)) has a priority over
__attribute__((preserve_access_index)). When preserve_access_index
attribute is present preserve access index transformations are not
applied.
This addresses the issue reported by the following thread:
https://lore.kernel.org/bpf/CAA-VZPmxh8o8EBcJ=m-DH4ytcxDFmo0JKsm1p1gf40kS0CE3NQ@mail.gmail.com/T/#m4b9ce2ce73b34f34172328f975235fc6f19841b6
This is a second attempt to commit this change, previous reverted
commit is: cb13e9286b6d4e384b5d4203e853d44e2eff0f0f.
The following items had been fixed:
- test case bpf-preserve-static-offset-bitfield.c now uses
`-triple bpfel` to avoid different codegen for little/big endian
targets.
- BPFPreserveStaticOffset.cpp:removePAICalls() modified to avoid
use after free for `WorkList` elements `V`.
Differential Revision: https://reviews.llvm.org/D133361
|
|
This reverts commit cb13e9286b6d4e384b5d4203e853d44e2eff0f0f.
Buildbot reports MSAN failures in tests added in this commit:
https://lab.llvm.org/buildbot/#/builders/5/builds/38806
Failing tests:
LLVM :: CodeGen/BPF/preserve-static-offset/load-arr-pai.ll
LLVM :: CodeGen/BPF/preserve-static-offset/load-ptr-pai.ll
LLVM :: CodeGen/BPF/preserve-static-offset/load-struct-pai.ll
LLVM :: CodeGen/BPF/preserve-static-offset/load-union-pai.ll
LLVM :: CodeGen/BPF/preserve-static-offset/store-pai.ll
|
|
This commit adds a new BPF specific structure attribte
`__attribute__((preserve_static_offset))` and a pass to deal with it.
This attribute may be attached to a struct or union declaration, where
it notifies the compiler that this structure is a "context" structure.
The following limitations apply to context structures:
- runtime environment might patch access to the fields of this type by
updating the field offset;
BPF verifier limits access patterns allowed for certain data
types. E.g. `struct __sk_buff` and `struct bpf_sock_ops`. For these
types only `LD/ST <reg> <static-offset>` memory loads and stores are
allowed.
This is so because offsets of the fields of these structures do not
match real offsets in the running kernel. During BPF program
load/verification loads and stores to the fields of these types are
rewritten so that offsets match real offsets. For this rewrite to
happen static offsets have to be encoded in the instructions.
See `kernel/bpf/verifier.c:convert_ctx_access` function in the Linux
kernel source tree for details.
- runtime environment might disallow access to the field of the type
through modified pointers.
During BPF program verification a tag `PTR_TO_CTX` is tracked for
register values. In case if register with such tag is modified BPF
programs are not allowed to read or write memory using register. See
kernel/bpf/verifier.c:check_mem_access function in the Linux kernel
source tree for details.
Access to the structure fields is translated to IR as a sequence:
- `(load (getelementptr %ptr %offset))` or
- `(store (getelementptr %ptr %offset))`
During instruction selection phase such sequences are translated as a
single load instruction with embedded offset, e.g. `LDW %ptr, %offset`,
which matches access pattern necessary for the restricted
set of types described above (when `%offset` is static).
Multiple optimizer passes might separate these instructions, this
includes:
- SimplifyCFGPass (sinking)
- InstCombine (sinking)
- GVN (hoisting)
The `preserve_static_offset` attribute marks structures for which the
following transformations happen:
- at the early IR processing stage:
- `(load (getelementptr ...))` replaced by call to intrinsic
`llvm.bpf.getelementptr.and.load`;
- `(store (getelementptr ...))` replaced by call to intrinsic
`llvm.bpf.getelementptr.and.store`;
- at the late IR processing stage this modification is undone.
Such handling prevents various optimizer passes from generating
sequences of instructions that would be rejected by BPF verifier.
The __attribute__((preserve_static_offset)) has a priority over
__attribute__((preserve_access_index)). When preserve_access_index
attribute is present preserve access index transformations are not
applied.
This addresses the issue reported by the following thread:
https://lore.kernel.org/bpf/CAA-VZPmxh8o8EBcJ=m-DH4ytcxDFmo0JKsm1p1gf40kS0CE3NQ@mail.gmail.com/T/#m4b9ce2ce73b34f34172328f975235fc6f19841b6
Differential Revision: https://reviews.llvm.org/D133361
|
|
HLSL supports vector swizzles on scalars by implicitly converting the
scalar to a single-element vector. This syntax is a convienent way to
initialize vectors based on filling a scalar value.
There are two parts of this change. The first part in the Lexer splits
numeric constant tokens when a `.x` or `.r` suffix is encountered. This
splitting is a bit hacky but allows the numeric constant to be parsed
separately from the vector element expression. There is an ambiguity
here with the `r` suffix used by fixed point types, however fixed point
types aren't supported in HLSL so this should not cause any exposable
problems (a separate issue has been filed to track validating language
options for HLSL: #67689).
The second part of this change is in Sema::LookupMemberExpr. For HLSL,
if the base type is a scalar, we implicit cast the scalar to a
one-element vector then call back to perform the vector lookup.
Fixes #56658 and #67511
|
