[compiler-rt] Don't rely on automatic codesigning with Apple's linker #91681
Conversation
In llvm#88323, I changed the logic within `add_compiler_rt_runtime` to only explicitly code sign the resulting library if an older version of Apple's ld64 was in use. This was based on the assumption that newer versions of ld64 and the new Apple linker always ad-hoc sign their output binaries. This is true in most cases, but not when using Apple's new linker with the '-darwin-target-variant' flag to build Mac binaries that are compatible with Catalyst. Rather than adding increasingly complicated logic to detect the exact scenarios that require explicit code signing, I've opted to always explicitly code sign when using any Apple linker. We instead detect and use the 'linker-signed' codesigning option when possible to match the signatures that the linker would otherwise create. This avoids having non-'linker-signed' ad-hoc signatures which was the underlying problem that llvm#88323 intended to address.
|
I can confirm that this fixes the problem I reported. I am now able to build and run Much appreciated @bdash |
| COMMAND sh -c "${CMAKE_LINKER} -v 2>&1 | head -1" | ||
| RESULT_VARIABLE HAD_ERROR | ||
| OUTPUT_VARIABLE LD_V_OUTPUT | ||
| COMMAND sh -c "codesign -f -s - -o linker-signed this-does-not-exist 2>&1 | grep -q linker-signed" |
There was a problem hiding this comment.
This is clever! Hopefully not too brittle? Only other thing I can think of is man codesign | grep linker-signed, but I like that less because it uses a completely other command (man).
There was a problem hiding this comment.
I wish there were a more direct way to test for support, but this was the best I could come up with. I've verified the format of errors for unknown -o values is the same from codesign on macOS 10.15 through macOS 14.4, so hopefully it'll stay consistent going forwards too.
| # most situations, except: | ||
| # 1. Versions of ld64 prior to ld64-609 in Xcode 12 predate this behavior. | ||
| # 2. Apple's new linker does not when building with `-darwin-target-variant` | ||
| # to support macOS Catalyst. |
There was a problem hiding this comment.
According to this (and assuming we can drop support for the old linker) would be to ensure the linker behaves consistently and always codesigns, right?
(Not asking for this in this PR, just wanted to make sure I understand.)
There was a problem hiding this comment.
I think this would all need to remain even if support is dropped for the old linker, purely for Catalyst scenario. The detection of -o linker-signed will be able to go away at that point since all versions of the developer tools that support the new linker also support that option.
|
LGTM, thanks @bdash! |
|
@yln What needs to be done to merge this? Does someone with commit access need to hit the merge button? |
|
Ah, I didn't realize to you don't have PR merge privileges / commit access. I will merge this for you on Tue morning (Mo is holiday in US) to avoid landing this just before the weekend reduce friction. Feel free to ping me if I haven't done this by Tue evening. If something goes wrong, people will comment here (and are allowed to revert our change). Please respond in a timely matter. Adding reviewers for visibility: @thetruestblue @wrotki |
|
Sounds good, thank you! |
|
@yln Any chance you'll be able to get it in this week? |
In #88323, I changed the logic within
add_compiler_rt_runtimeto only explicitly code sign the resulting library if an older version of Apple's ld64 was in use. This was based on the assumption that newer versions of ld64 and the new Apple linker always ad-hoc sign their output binaries. This is true in most cases, but not when using Apple's new linker with the-darwin-target-variantflag to build Mac binaries that are compatible with Catalyst.Rather than adding increasingly complicated logic to detect the exact scenarios that require explicit code signing, I've opted to always explicitly code sign when using any Apple linker. We instead detect and use the 'linker-signed' codesigning option when possible to match the signatures that the linker would otherwise create. This avoids having non-'linker-signed' ad-hoc signatures which was the underlying problem that #88323 was intended to address.