So far, the `DW_AT_linkage_name` of the coroutine `resume`, `destroy`,
`cleanup` and `noalloc` function clones were incorrectly set to the
original function name instead of the updated function names.
With this commit, we now update the `DW_AT_linkage_name` to the correct
name. This has multiple benefits:
1. it's easier for me (and other toolchain developers) to understand the
output of `llvm-dwarf-dump` when coroutines are involved.
2. When hitting a breakpoint, both LLDB and GDB now tell you which clone
of the function you are in. E.g., GDB now prints "Breakpoint 1.2,
coro_func(int) [clone .resume] (v=43) at ..." instead of "Breakpoint
1.2, coro_func(int) (v=43) at ...".
3. GDB's `info line coro_func` command now allows you to distinguish the
multiple different clones of the function.
In Swift, the linkage names of the clones were already updated. The
comment right above the relevant code in `CoroSplit.cpp` already hinted
that the linkage name should probably also be updated in C++. This
comment was added in commit 6ce76ff7eb7640, and back then the
corresponding `DW_AT_specification` (i.e., `SP->getDeclaration()`) was
not updated, yet, which led to problems for C++. In the meantime, commit
ca1a5b37c7236d added code to also update `SP->getDeclaration`, as such
there is no reason anymore to not update the linkage name for C++.
Note that most test cases used inconsistent function names for the LLVM
function vs. the DISubprogram linkage name. clang would never emit such
LLVM IR. This confused me initially, and hence I fixed it while updating
the test case.
Drive-by fix: The change in `CGVTables.cpp` is purely stylistic, NFC.
When looking for other usages of `replaceWithDistinct`, I got initially
confused because `CGVTables.cpp` was calling a static function via an
object instance.
Finding operator delete[] is still problematic, without it the extension
is a security hazard, so reverting until the problem with operator
delete[] is figured out.
This reverts the following PRs:
Reland [MS][clang] Add support for vector deleting destructors (llvm#133451)
[MS][clang] Make sure vector deleting dtor calls correct operator delete (llvm#133950)
[MS][clang] Fix crash on deletion of array of pointers (llvm#134088)
[clang] Do not diagnose unused deleted operator delete[] (llvm#134357)
[MS][clang] Error about ambiguous operator delete[] only when required (llvm#135041)
This is an expensive header, only include it where needed. Move some
functions out of line to achieve that.
This reduces time to build clang by ~0.5% in terms of instructions
retired.
Whereas it is UB in terms of the standard to delete an array of objects
via pointer whose static type doesn't match its dynamic type, MSVC
supports an extension allowing to do it.
Aside from array deletion not working correctly in the mentioned case,
currently not having this extension implemented causes clang to generate
code that is not compatible with the code generated by MSVC, because
clang always puts scalar deleting destructor to the vftable. This PR
aims to resolve these problems.
It was reverted due to link time errors in chromium with sanitizer
coverage enabled,
which is fixed by https://github.com/llvm/llvm-project/pull/131929 .
The second commit of this PR also contains a fix for a runtime failure
in chromium reported
in
https://github.com/llvm/llvm-project/pull/126240#issuecomment-2730216384
.
Fixes https://github.com/llvm/llvm-project/issues/19772
Original PR: #130537
Originally reverted due to revert of dependent commit. Relanding with no
changes.
This changes the MemberPointerType representation to use a
NestedNameSpecifier instead of a Type to represent the base class.
Since the qualifiers are always parsed as nested names, there was an
impedance mismatch when converting these back and forth into types, and
this led to issues in preserving sugar.
The nested names are indeed a better match for these, as the differences
which a QualType can represent cannot be expressed syntatically, and
they represent the use case more exactly, being either dependent or
referring to a CXXRecord, unqualified.
This patch also makes the MemberPointerType able to represent sugar for
a {up/downcast}cast conversion of the base class, although for now the
underlying type is canonical, as preserving the sugar up to that point
requires further work.
As usual, includes a few drive-by fixes in order to make use of the
improvements.
Original PR: #130537
Reland after updating lldb too.
This changes the MemberPointerType representation to use a
NestedNameSpecifier instead of a Type to represent the base class.
Since the qualifiers are always parsed as nested names, there was an
impedance mismatch when converting these back and forth into types, and
this led to issues in preserving sugar.
The nested names are indeed a better match for these, as the differences
which a QualType can represent cannot be expressed syntatically, and
they represent the use case more exactly, being either dependent or
referring to a CXXRecord, unqualified.
This patch also makes the MemberPointerType able to represent sugar for
a {up/downcast}cast conversion of the base class, although for now the
underlying type is canonical, as preserving the sugar up to that point
requires further work.
As usual, includes a few drive-by fixes in order to make use of the
improvements.
This changes the MemberPointerType representation to use a
NestedNameSpecifier instead of a Type to represent the class.
Since the qualifiers are always parsed as nested names, there was an
impedance mismatch when converting these back and forth into types, and
this led to issues in preserving sugar.
The nested names are indeed a better match for these, as the differences
which a QualType can represent cannot be expressed syntactically, and it
also represents the use case more exactly, being either dependent or
referring to a CXXRecord, unqualified.
This patch also makes the MemberPointerType able to represent sugar for
a {up/downcast}cast conversion of the base class, although for now the
underlying type is canonical, as preserving the sugar up to that point
requires further work.
As usual, includes a few drive-by fixes in order to make use of the
improvements, and removing some duplications, for example
CheckBaseClassAccess is deduplicated from across SemaAccess and
SemaCast.
This caused link errors when building with sancov. See comment on the PR.
> Whereas it is UB in terms of the standard to delete an array of objects
> via pointer whose static type doesn't match its dynamic type, MSVC
> supports an extension allowing to do it.
> Aside from array deletion not working correctly in the mentioned case,
> currently not having this extension implemented causes clang to generate
> code that is not compatible with the code generated by MSVC, because
> clang always puts scalar deleting destructor to the vftable. This PR
> aims to resolve these problems.
>
> Fixes https://github.com/llvm/llvm-project/issues/19772
This reverts commit d6942d54f677000cf713d2b0eba57b641452beb4.
Whereas it is UB in terms of the standard to delete an array of objects
via pointer whose static type doesn't match its dynamic type, MSVC
supports an extension allowing to do it.
Aside from array deletion not working correctly in the mentioned case,
currently not having this extension implemented causes clang to generate
code that is not compatible with the code generated by MSVC, because
clang always puts scalar deleting destructor to the vftable. This PR
aims to resolve these problems.
Fixes https://github.com/llvm/llvm-project/issues/19772
Currently, the clang frontend incorrectly emits the callee instead of
the thunk for the callee in the VTable. This is the case because the
thunk index is not incremented when their callees cannot be emitted.
This patch fixes the bug.
Reland https://github.com/llvm/llvm-project/pull/75912
The differences of this PR between
https://github.com/llvm/llvm-project/pull/75912 are:
- Fixed a regression in `Decl::isInAnotherModuleUnit()` in DeclBase.cpp
pointed by @mizvekov and add the corresponding test.
- Fixed the regression in windows
https://github.com/llvm/llvm-project/issues/97447. The changes are in
`CodeGenModule::getVTableLinkage` from
`clang/lib/CodeGen/CGVTables.cpp`. According to the feedbacks from MSVC
devs, the linkage of vtables won't affected by modules. So I simply
skipped the case for MSVC.
Given this is more or less fundamental to the use of modules. I hope we
can backport this to 19.x.
When using the relative vtable ABI, if a vtable is not dso_local, it's
given private linkage (if not COMDAT) or hidden visibility (if COMDAT)
to make it dso_local (to place it in rodata instead of data.rel.ro), and
an alias generated with the original linkage and visibility. This alias
could later be removed from the symbol table, e.g. if using a version
script, at which point we lose all symbol information about the vtable.
Use internal linkage instead of private linkage to avoid this.
While I'm here, clarify the comment about why COMDAT vtables can't use
internal (or private) linkage, and associate it with the else block
where hidden visibility is applied instead of internal linkage.
This reverts commit 18f3bcbb13ca83d33223b00761d8cddf463e9ffb, 15bb02650e26875c48889053d6a9697444583721 and
99873b35da7ecb905143c8a6b8deca4d4416f1a9.
See the post commit message in
https://github.com/llvm/llvm-project/pull/75912 to see the reasons.
named modules
Close https://github.com/llvm/llvm-project/issues/97313
In the previous patch (https://github.com/llvm/llvm-project/pull/75912),
I made an oversight that I ignored the templates in named module when
calculating the linkage for the vtables. In this patch, I tried to
correct the behavior by merging the logics to calculate the linkage with
key functions with named modules.
Virtual function pointer entries in v-tables are signed with address
discrimination in addition to declaration-based discrimination, where an
integer discriminator the string hash (see
`ptrauth_string_discriminator`) of the mangled name of the overridden
method. This notably provides diversity based on the full signature of
the overridden method, including the method name and parameter types.
This patch introduces ItaniumVTableContext logic to find the original
declaration of the overridden method.
On AArch64, these pointers are signed using the `IA` key (the
process-independent code key.)
V-table pointers can be signed with either no discrimination, or a
similar scheme using address and decl-based discrimination. In this
case, the integer discriminator is the string hash of the mangled
v-table identifier of the class that originally introduced the vtable
pointer.
On AArch64, these pointers are signed using the `DA` key (the
process-independent data key.)
Not using discrimination allows attackers to simply copy valid v-table
pointers from one object to another. However, using a uniform
discriminator of 0 does have positive performance and code-size
implications on AArch64, and diversity for the most important v-table
access pattern (virtual dispatch) is already better assured by the
signing schemas used on the virtual functions. It is also known that
some code in practice copies objects containing v-tables with `memcpy`,
and while this is not permitted formally, it is something that may be
invasive to eliminate.
This is controlled by:
```
-fptrauth-vtable-pointer-type-discrimination
-fptrauth-vtable-pointer-address-discrimination
```
In addition, this provides fine-grained controls in the
ptrauth_vtable_pointer attribute, which allows overriding the default
ptrauth schema for vtable pointers on a given class hierarchy, e.g.:
```
[[clang::ptrauth_vtable_pointer(no_authentication, no_address_discrimination,
no_extra_discrimination)]]
[[clang::ptrauth_vtable_pointer(default_key, default_address_discrimination,
custom_discrimination, 0xf00d)]]
```
The override is then mangled as a parametrized vendor extension:
```
"__vtptrauth" I
<key>
<addressDiscriminated>
<extraDiscriminator>
E
```
To support this attribute, this patch adds a small extension to the
attribute-emitter tablegen backend.
Note that there are known areas where signing is either missing
altogether or can be strengthened. Some will be addressed in later
changes (e.g., member function pointers, some RTTI).
`dynamic_cast` in particular is handled by emitting an artificial
v-table pointer load (in a way that always authenticates it) before the
runtime call itself, as the runtime doesn't have enough information
today to properly authenticate it. Instead, the runtime is currently
expected to strip the v-table pointer.
---------
Co-authored-by: John McCall <rjmccall@apple.com>
Co-authored-by: Ahmed Bougacha <ahmed@bougacha.org>
This patch fixes debug records in clang, by adding support for debug
records to the only remaining place that refers to DbgVariableIntrinsics
directly and does not handle DbgVariableRecords.
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).
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.
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 way, it the rtti_proxies can be candidates for being replaced
altogether with GOTPCREL relocations because they are discardable.
Functionally, this shouldn't change the final ELF linkage of the
proxies.
Hashing the sugared type instead of the canonical type meant that
a simple example like this would always fail under MSVC:
```
static auto l() {}
int main() {
auto a = l;
a();
}
```
`clang --target=x86_64-pc-windows-msvc -fno-exceptions
-fsanitize=function -g -O0 -fuse-ld=lld -o test.exe test.cc`
produces:
```
test.cc:4:3: runtime error: call to function l through pointer to incorrect function type 'void (*)()'
```
The goal of this change is to clean up some of the code surrounding
HLSL using CXXThisExpr as a non-pointer l-value. This change cleans up
a bunch of assumptions and inconsistencies around how the type of
`this` is handled through the AST and code generation.
This change is be mostly NFC for HLSL, and completely NFC for other
language modes.
This change introduces a new member to query for the this object's type
and seeks to clarify the normal usages of the this type.
With the introudction of HLSL to clang, CXXThisExpr may now be an
l-value and behave like a reference type rather than C++'s normal
method of it being an r-value of pointer type.
With this change there are now three ways in which a caller might need
to query the type of `this`:
* The type of the `CXXThisExpr`
* The type of the object `this` referrs to
* The type of the implicit (or explicit) `this` argument
This change codifies those three ways you may need to query
respectively as:
* CXXMethodDecl::getThisType()
* CXXMethodDecl::getThisObjectType()
* CXXMethodDecl::getThisArgType()
This change then revisits all uses of `getThisType()`, and in cases
where the only use was to resolve the pointee type, it replaces the
call with `getThisObjectType()`. In other cases it evaluates whether
the desired returned type is the type of the `this` expr, or the type
of the `this` function argument. The `this` expr type is used for
creating additional expr AST nodes and for member lookup, while the
argument type is used mostly for code generation.
Additionally some cases that used `getThisType` in simple queries could
be substituted for `getThisObjectType`. Since `getThisType` is
implemented in terms of `getThisObjectType` calling the later should be
more efficient if the former isn't needed.
Reviewed By: aaron.ballman, bogner
Differential Revision: https://reviews.llvm.org/D159247
All data structures and values associated with handling virtual functions / inheritance, as well as RTTI, are globals and thus can only reside in the global address space. This was not taken fully taken into account because for most targets, global & generic appear to coincide. However, on targets where global & generic ASes differ (e.g. AMDGPU), this was problematic, since it led to the generation of invalid bitcasts (which would trigger asserts in Debug) and less than optimal code. This patch does two things:
ensures that vtables, vptrs, vtts, typeinfo are generated in the right AS, and populated accordingly;
removes a bunch of bitcasts which look like left-overs from the typed ptr era.
Reviewed By: yxsamliu
Differential Revision: https://reviews.llvm.org/D153092
This patch renames the `OpenMPIRBuilderConfig` flags to reduce confusion over
their meaning. `IsTargetCodegen` becomes `IsGPU`, whereas `IsEmbedded` becomes
`IsTargetDevice`. The `-fopenmp-is-device` compiler option is also renamed to
`-fopenmp-is-target-device` and the `omp.is_device` MLIR attribute is renamed
to `omp.is_target_device`. Getters and setters of all these renamed properties
are also updated accordingly. Many unit tests have been updated to use the new
names, but an alias for the `-fopenmp-is-device` option is created so that
external programs do not stop working after the name change.
`IsGPU` is set when the target triple is AMDGCN or NVIDIA PTX, and it is only
valid if `IsTargetDevice` is specified as well. `IsTargetDevice` is set by the
`-fopenmp-is-target-device` compiler frontend option, which is only added to
the OpenMP device invocation for offloading-enabled programs.
Differential Revision: https://reviews.llvm.org/D154591
Close https://github.com/llvm/llvm-project/issues/61940.
The root cause is that clang will generate vtable as strong symbol now
even if the corresponding class is defined in other module units. After
I check the wording in Itanium ABI, I find this is not inconsistent.
Itanium ABI 5.2.3
(https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable) says:
> The virtual table for a class is emitted in the same object containing
> the definition of its key function, i.e. the first non-pure virtual
> function that is not inline at the point of class definition.
So the current behavior is incorrect. This patch tries to address this.
Also I think we need to do a similar change for MSVC ABI. But I don't
find the formal wording. So I don't address this in this patch.
Reviewed By: rjmccall, iains, dblaikie
Differential Revision: https://reviews.llvm.org/D150023
Breaks check-clang on win and mac, see comments on https://reviews.llvm.org/D150023
This reverts commit d8a36b00d198fdc2ea866ea5da449628db07070f.
Also revert follow-up "[NFC] skip the test modules-vtable.cppm on windows"
This reverts commit baf0b12ca6c624b2a59aa6f2fd0310c72d35ac56.
Close https://github.com/llvm/llvm-project/issues/61940.
The root cause is that clang will generate vtable as strong symbol now
even if the corresponding class is defined in other module units. After
I check the wording in Itanium ABI, I find this is not inconsistent.
Itanium ABI 5.2.3
(https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable) says:
> The virtual table for a class is emitted in the same object containing
> the definition of its key function, i.e. the first non-pure virtual
> function that is not inline at the point of class definition.
So the current behavior is incorrect. This patch tries to address this.
Also I think we need to do a similar change for MSVC ABI. But I don't
find the formal wording. So I don't address this in this patch.
Reviewed By: rjmccall, iains, dblaikie
Differential Revision: https://reviews.llvm.org/D150023
Reported by Coverity:
AUTO_CAUSES_COPY
Unnecessary object copies can affect performance.
1. Inside "SemaDeclCXX.cpp" file, in <unnamed>::DiagnoseUninitializedFields(clang::Sema &, clang::CXXConstructorDecl const *): Using the auto keyword without an & causes the copy of an object of type CXXBaseSpecifier.
2. Inside "ClangAttrEmitter.cpp" file, in clang::EmitClangAttrParsedAttrImpl(llvm::RecordKeeper &, llvm::raw_ostream &): Using the auto keyword without an & causes the copy of an object of type pair.
3. Inside "Marshallers.h" file, in clang::ast_matchers::dynamic::internal::MapAnyOfBuilderDescriptor::buildMatcherCtor(clang::ast_matchers::dynamic::SourceRange, llvm::ArrayRef<clang::ast_matchers::dynamic::ParserValue>, clang::ast_matchers::dynamic::Diagnostics *): Using the auto keyword without an & causes the copy of an object of type ParserValue.
4. Inside "CGVTables.cpp" file, in clang::CodeGen::CodeGenModule::GetVCallVisibilityLevel(clang::CXXRecordDecl const *, llvm::DenseSet<clang::CXXRecordDecl const *, llvm::DenseMapInfo<clang::CXXRecordDecl const *, void>> &): Using the auto keyword without an & causes the copy of an object of type CXXBaseSpecifier.
5. Inside "ASTContext.cpp" file, in hasTemplateSpecializationInEncodedString(clang::Type const *, bool): Using the auto keyword without an & causes the copy of an object of type CXXBaseSpecifier.
6. Inside "ComputeDependence.cpp" file, in clang::computeDependence(clang::DependentScopeDeclRefExpr *): Using the auto keyword without an & causes the copy of an object of type TemplateArgumentLoc.
Reviewed By: tahonermann, erichkeane
Differential Revision: https://reviews.llvm.org/D148812
This patch moves the Debug Options to llvm/Frontend so that it can be shared by Flang as well.
Reviewed By: kiranchandramohan, awarzynski
Differential Revision: https://reviews.llvm.org/D142347
Prior to this, metadata pertaining to the size or address point offsets
into a relative vtable were twice the value they should be (treating
component widths as pointer width rather than 4 bytes). This prevented
some vtables from being devirtualized with D134320. This ensures the
correct metadata is written so whole program devirtualization can catch
these remaining devirt targets.
Differential Revision: https://reviews.llvm.org/D134687
Mixing LLVM and Clang address spaces can result in subtle bugs, and there
is no need for this hook to use the LLVM IR level address spaces.
Most of this change is just replacing zero with LangAS::Default,
but it also allows us to remove a few calls to getTargetAddressSpace().
This also removes a stale comment+workaround in
CGDebugInfo::CreatePointerLikeType(): ASTContext::getTypeSize() does
return the expected size for ReferenceType (and handles address spaces).
Differential Revision: https://reviews.llvm.org/D138295
We run into a duplicate symbol error when instrumenting the rtti_proxies
generated as part of the relative vtables ABI with hwasan:
```
ld.lld: error: duplicate symbol: typeinfo for icu_71::UObject
(.rtti_proxy)
>>> defined at brkiter.cpp
>>>
arm64-hwasan-shared/obj/third_party/icu/source/common/libicuuc.brkiter.cpp.o:(typeinfo
for icu_71::UObject (.rtti_proxy))
>>> defined at locavailable.cpp
>>>
arm64-hwasan-shared/obj/third_party/icu/source/common/libicuuc.locavailable.cpp.o:(.data.rel.ro..L_ZTIN6icu_717UObjectE.rtti_proxy.hwasan+0xE00000000000000)
```
The issue here is that the hwasan alias carries over the visibility and
linkage of the original proxy, so we have duplicate external symbols
that participate in linking. Similar to D132425 we can just disable
hwasan for the proxies for now.
Differential Revision: https://reviews.llvm.org/D132691
