[5.7] Remove --gc-sections for all targets for now #5708
Conversation
Similar to the comment here about wasm-ld, lld recently switched a default that broke this for all elf targets. This is a low risk fix since we weren't passing this flag before either. People who really need this behavior can still pass something like `-Xlinker --gc-sections -Xlinker -z -Xlinker nostart-stop-gc`
| @@ -1290,17 +1290,12 @@ public final class ProductBuildDescription { | |||
| return ["-Xlinker", "-dead_strip"] | |||
| } else if buildParameters.triple.isWindows() { | |||
| return ["-Xlinker", "/OPT:REF"] | |||
| } else if buildParameters.triple.arch == .wasm32 { | |||
| // FIXME: wasm-ld strips data segments referenced through __start/__stop symbols | |||
| } else { | |||
There was a problem hiding this comment.
Should we really be resolving this by making this the unconditional else branch? Is there no solution here where we check a second condition to get this behavior? (Sorry about my ignorance in this area, I’m mostly just coming to this conclusion in a this context-free way)
There was a problem hiding this comment.
I think we'd have to check that the linker was not lld, which might be fine to do just based on the use-ld arg value, but also that supports paths like -use-ld=/path/to/lld, which just made it feel potentially more error prone.
There was a problem hiding this comment.
Other than this concern, I’d be fine taking it onto the release branch.
There was a problem hiding this comment.
I think this is fine? its basically adding additional [dead stripping] linker arguments for certain platforms, and none in the default case
There was a problem hiding this comment.
It is too bad that folks not using lld can't get the dead stripping benefit, but theoretically at least they can opt in manually 😕
There was a problem hiding this comment.
I agree that checking for lld is too error-prone, as linker selection is not a direct SPM flag, so you're stuck parsing the various -Xswiftc and -Xlinker flags and still wouldn't catch situations where people don't pass in SPM flags but symbolically link their system linker to lld, because they use lld by default with all toolchains.
|
@swift-ci please test |
|
Something wrong with the CI? |
|
@swift-ci please smoke test |
Similar to the comment here about wasm-ld, lld recently switched a
default that broke this for all elf targets. This is a low risk fix
since we weren't passing this flag before either. People who really need
this behavior can still pass something like
-Xlinker --gc-sections -Xlinker -z -Xlinker nostart-stop-gcFixes #5698