This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Stop explictly ad-hoc signing compiler-rt dylibs in apple builds if ld is new enough
ClosedPublic

Authored by thakis on Apr 10 2022, 6:31 PM.

Details

Summary

ld64 implicitly ad-hoc code-signs as of Xcode 12, and strip and friends know
how keep this special ad-hoc signature valid.

So this should have no effective behavior change, except that you can now strip
libclang_rt.asan_osx_dynamic.dylib and it'll still have a valid ad-hoc
signature, instead of strip printing "warning: changes being made to the file
will invalidate the code signature in:" and making the ad-hoc code signature
invalid.

Diff Detail

Event Timeline

thakis created this revision.Apr 10 2022, 6:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 10 2022, 6:31 PM
thakis requested review of this revision.Apr 10 2022, 6:31 PM

+ more Apple folks

Makes sense, the only thing I can think of is if someone wants to build compiler-rt with older Xcode/SDKs. Not sure if we care about that.

Makes sense, the only thing I can think of is if someone wants to build compiler-rt with older Xcode/SDKs. Not sure if we care about that.

wants to build compiler-rt *for iOS* with older Xcode/SDKs, right? Mac/Intel doesn't care about the signature, and Mac/Arm requires Xcode 12+ to be able to target it at all.

I don't know if that's something people do.

I guess we can land this and put it back behind an option if it turns out someone does need it?

+ more Apple folks

Makes sense, the only thing I can think of is if someone wants to build compiler-rt with older Xcode/SDKs. Not sure if we care about that.

For our own internal CI I don't think this change should cause problems. I'm not sure about the public CI (http://lab.llvm.org:8080/green/). The page says:

Builders are running macOS Catalina 10.15.5 (19F101) with Xcode 11.5 (11E608c). System tools: clang-1103.0.32.62, ld64-556.6

If this information is accurate it might cause problems but I'm not sure if this up-to-date.

thakis updated this revision to Diff 422080.Apr 11 2022, 5:42 PM
thakis retitled this revision from [compiler-rt] Stop explictly ad-hoc signing compiler-rt dylibs in apple builds to [compiler-rt] Stop explictly ad-hoc signing compiler-rt dylibs in apple builds if ld is new enough.

check ld version

Looks like those bots do build for iossim, thanks for pointing this out.

I tweaked this so that it omits the codesign invocation only if ld64 is new enough. That makes it so that the dylib can be stripped as long as LLVM is built with a new-enough Xcode, and one day we can remove the whole block. How does that look?

yln accepted this revision.Apr 12 2022, 10:12 AM

LGTM to me, thanks!
@delcypher, what do you think?

This revision is now accepted and ready to land.Apr 12 2022, 10:12 AM
This revision was landed with ongoing or failed builds.Apr 13 2022, 5:39 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 5:39 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

Looking through https://green.lab.llvm.org/green/view/Sanitizer%20CI/job/clang-san-iossim/ (the only bot I could find building compiler-rt for iOS), all its builds seem to run on https://green.lab.llvm.org/green/computer/green-dragon-08/, which does have Xcode 13 according to the web interface.

So maybe just removing this is fine after all?

But happy to keep it in as-is for a while too, let me know.