[Linux][Runtime][IRGen] Mark metadata sections as retained and support section GC. #72061
Conversation
al45tair requested review from MaxDesiatov, artemcm, etcwilde, ktoso and mikeash as code owners 
| @swift-ci Please test |
| Glad that you're trying to fix this properly, this closes #60406? Any plans to get this into 5.10 or will that have to rely on my old workaround of extra lld flags instead? |
| @swift-ci test WebAssembly |
Looks like it will, yes. |
|
|
| Hmmmm… not sure what this is all about: Will take a look at that tomorrow. |
It won't be in 5.10.0. I think it'd be worth getting it into a point release though. |
| The SourceKit test failures are all of the form which seems to relate to something @eeckstein added on the 14th of February. They don't seem to be related to the changes in this PR. |
| @swift-ci Please test |
| @swift-ci test WebAssembly |
| OK. Apparently the problem with the SourceKit tests is that the |
al45tair requested review from ahoppen, bnbarham and hamishknight as code owners 
…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 force-pushed the eng/PR-123504095 branch from d20c02d to a014bd2 Compare | (For those following this PR, I'm still working on the LLDB issues. One of them is fixed here: llvm/llvm-project#90099, but there are some more things going wrong for some reason.) |
| OK, so the "more things going wrong" are actually the same problem as the one fixed in llvm/llvm-project#90099. a014bd2 only works around it for What was happening there, FWIW, was that a number of tests set a breakpoint on |
| @swift-ci Please test |
…g -z nostart-stop-gc" This reverts c771555, now that it was properly fixed in swiftlang/swift#72061.
| // RUN: %target-build-swift -Xfrontend -function-sections -emit-module -emit-library -static -parse-stdlib %S/Inputs/FunctionSections.swift | ||
| // RUN: %target-build-swift -Xlinker --gc-sections -Xlinker -Map=%t/../../FunctionSections.map -I%t/../.. -L%t/../.. -lFunctionSections %S/Inputs/FunctionSectionsUse.swift | ||
| // RUN: %FileCheck %s < %t/../../FunctionSections.map | ||
| // RUN: %target-build-swift -Xlinker -v -Xlinker --gc-sections -Xlinker -Map=%t/../../FunctionSections.map -I%t/../.. -L%t/../.. -lFunctionSections %S/Inputs/FunctionSectionsUse.swift 2>&1 | sed 's/.*\(gold\|LLD\).*/\1/g' | tr "[:lower:]" "[:upper:]" > %t/../../Linker.txt |
There was a problem hiding this comment.
This change takes the stdout and stderr of %target-build-stdlib pass each line through sed that replaces lines that say either gold or LLD with just those word, and convert everything to upper case. Then the result of all that is passed as --check-prefix?
What happens when -v prints more than one line? All the lines will be printed and passed to as extra arguments to %FileCheck?
$ echo -e 'foo\nbar\nthis will say LLD\nbaz' | sed 's/.*\(gold\|LLD\).*/\1/g' foo bar LLD baz Am I missing something?
In swiftlang#72061 the test was modified because LLD and Gold linker map formats are different, so they need different CHECKs. The original method in PR swiftlang#72061 presents problems when the output is not exactly only one line. These changes use the first line of the linker map file to decide if the Gold or the LLD linker map checks should be used, since the format differs in both for the first line. This way of checking works in my Linux machine when using the default linker, when forcing Gold with `-use-ld=gold` and when forcing LLD with `-use-ld=lld`.
In #72061 the test was modified because LLD and Gold linker map formats are different, so they need different CHECKs. The original method in PR #72061 presents problems when the output is not exactly only one line. These changes use the first line of the linker map file to decide if the Gold or the LLD linker map checks should be used, since the format differs in both for the first line. This way of checking works in my Linux machine when using the default linker, when forcing Gold with `-use-ld=gold` and when forcing LLD with `-use-ld=lld`.
7 participants
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