[Linux][Runtime][IRGen] Mark metadata sections as retained and support section GC. #73371
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We changed to `llvm.compiler.used` because of the behaviour of `gold`, which refuses to coalesce sections that have different `SHF_GNU_RETAIN` flags, which causes problems with metadata. Originally I thought we were going to have to generate two sections with distinct names and have the runtime look for both of them, but it turns out that the runtime only wants to see sections that have `SHF_GNU_RETAIN` in any case. It's really the reflection code that is interested in being able to see non-retained sections. The upshot is that we don't need to use `llvm.compiler.used`; it's just fine if we have duplicate sections, as long as the reflection code looks for them when it's inspecting an ELF image. This also means we no longer need to pass `-z nostart-stop-gc` to the linker if we're using `lld`. rdar://123504095
This also calls out the reflection sections, even though we don't actually need to do anything special for them (as it turns out). rdar://123504095
…tions. When linking with `gold`, we can end up with two separate sections for each of the reflection sections; this happens because `swiftrt.o` declares them with `SHF_GNU_RETAIN` set (to stop them from vanishing when section GC is enabled), but if the compiler is set to allow the reflection information to be stripped then *it* will generate them *without* `SHF_GNU_RETAIN`. The other Linux linkers will coalesce the two sections and leave the `SHF_GNU_RETAIN` flag set, but `gold` does not, and adds both of them separately to the output. The upshot is that we want `ReflectionContext` to look for both the retained and non-retained sections and read the data from both of them. rdar://123504095
Without doing this, `__Swift_AST` gets stripped from the output. We also need to call `IGM.finalize()` before `::performLLVM()` in the `createSwiftModuleObjectFile()` function, so that we update the section to mark it as retained. rdar://123504095
IRGen sets up the `llvm::TargetOptions` by itself, rather than copying them from the Clang instance (it has to do this, at present, because Clang doesn't provide a way to get it to initialise one). Unfortunately, it didn't set `UseInitArray`, which when linking with GNU `ld` or `gold` is *fine* because those linkers automatically convert `.ctors` and `.dtors` sections into `.init_array` and `.fini_array` respectively. *However*, `lld` does *not* do this conversion, so when using `lld`, if the compiler generates a constructor or destructor function, it won't be called(!) The fix is to set `UseInitArray` properly; I chose to copy the setting from Clang, just in case Clang knows of a reason why it shouldn't be `true`. rdar://123504095
`gold` and `lld` produce map files in different formats; this test can and should work for both of them. rdar://123504095
When testing different linkers, it's sometimes useful to run the tests with `SWIFT_DRIVER_TEST_OPTIONS=" -use-ld=<linker>"`. If we do this, it will break a handful of tests because they expect the compiler driver to choose an appropriate linker automatically. To avoid having these fail, detect when someone has done this, and set a new feature, `linker_overridden`, then mark the tests in question with `UNSUPPORTED: linker_overridden`. rdar://123504095
Since this is adjacent to a `!=` operator, explicitly using `bool` seems clearer. Co-authored-by: Evan Wilde <etceterawilde@gmail.com>
On ELF platforms, the output will be slightly different to non-ELF platforms. The test should ideally run everywhere, however, so we need to be able to distinguish these platforms by changing the lit.cfg to add some extra variables. rdar://123504095
Add a comment to ReflectionContext.h explaining the situation regarding `SHF_GNU_RETAIN` and the reflection metadata sections. rdar://123504095
Without `retain` here, we might remove the reference that pulls in the backtracing support code. rdar://123504095
We were linking with the newly built `swiftrt.o` when in hosttools mode, which is wrong because the newly built `swiftrt.o` does not match the compiler we were using for the `SwiftCompilerSources`. This manifests as a failure in `SwiftCompilerSources/Sources/Optimizer/Utilities/Verifier.swift` because `self is ForwardingInstruction` fails as we can't find the protocol conformance records. rdar://123504095
This doesn't change any code, just makes things look slightly neater. rdar://123504095
There are a number of instances in `ReflectionContext.h` where we
are doing `return false` with an `std::optional<...>` where it seems
we really mean to return an empty optional instead.
(The way to do this is either `return {}` or `return std::nullopt`.)
rdar://123504095
If we're on a system that has ld.gold 2.35 or earlier, we want to use lld instead because otherwise we end up with duplicate sections in the output. rdar://123504095
al45tair
added
Linux
|
This is a cherry pick of #72061. |
|
Explanation: This fix is required both for the static Linux SDK work and also to allow @etcwilde's changes (to get Swift to use Clang for linking); in the latter case it's necessary because without it there's no way to tell whether to pass the |
|
@swift-ci please test |
compnerd
approved these changes
May 1, 2024
You mean lld, we already use clang as the linker driver. |
tbkka
approved these changes
May 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want to support section GC, and the
__startand__stopsymbols don't count as roots for that (at least, not by default). Additionally, while GNUldandlldwill coalesce retained and non-retained sections,goldrefuses to do so (see https://sourceware.org/bugzilla/show_bug.cgi?id=31415), which trips up the reflection code because that only expects to find a single section.Turn on the
SHF_GNU_RETAINflag inswiftrt.o, and remove the existing workarounds for both of the above.Also fix generation of module objects to be section GC compatible by marking the
__Swift_ASTsymbol as used and finalising the module in IRGen.Additionally, it turns out that we weren't initialising the
llvm::TargetOptionsproperly and in particular theUseInitArraysetting was wrong; this is masked by code ingoldthat converts.ctorsand.dtorssections into.init_arrayand.fini_arrayrespectively, butllddoes not do this, and since Glibc no longer calls functions in these sections, that means that when linked withlld, any constructors or destructors generated by the Swift compiler don't run. Fix by settingUseInitArray.rdar://123504095