Skip to content

[Build][Backtracing] Make sure the target for OS X is at least 10.14.4. #75710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Aug 6, 2024

We need to make sure that we build swift-backtrace with a deployment target newer than 10.14.4 in order that we get linked against /usr/lib/swift/libswiftCore.dylib rather than using an @rpath-based path.

If we fail to do that, dyld becomes confused and we end up crashing with weird errors about missing method implementations on Swift.__StringStorage.

To make this work, add support for DEPLOYMENT_VERSION_* in the add_swift_target_executable() CMake function. And since I spotted a bug in it, fix the existing support in add_swift_target_library() while I'm there.

rdar://132710670

… 10.14.4.

We need to make sure that we build swift-backtrace with a deployment target
newer than 10.14.4 in order that we get linked against
`/usr/lib/swift/libswiftCore.dylib` rather than using an `@rpath`-based
path.

If we fail to do that, dyld becomes confused and we end up crashing with
weird errors about missing method implementations on `Swift.__StringStorage`.

To make this work, add support for `DEPLOYMENT_VERSION_*` in the
`add_swift_target_executable()` CMake function.  And since I spotted a bug
in it, fix the existing support in `add_swift_target_library()` while I'm
there.

rdar://132710670
@al45tair al45tair requested a review from a team as a code owner August 6, 2024 14:10
@al45tair
Copy link
Contributor Author

al45tair commented Aug 6, 2024

@swift-ci Please test

@al45tair
Copy link
Contributor Author

al45tair commented Aug 6, 2024

Note: this doesn't affect the overall correctness of the final build. It just fixes problems with the tests.

@al45tair
Copy link
Contributor Author

al45tair commented Aug 6, 2024

Explanation: If the deployment target is set to anything prior to 10.14.4, the $ld$previous feature is used to replace the path to the Swift libraries with an @rpath-based path. This is done to permit Swift programs to be back-deployed to systems where Swift was not part of the OS. Unfortunately, doing this breaks the use of DYLD_LIBRARY_PATH in the tests to point things at the newly built libswiftCore.dylib, because dyld looks at the original load path to determine whether or not to override the system copy of the library, and if it sees @rpath on the front, it assumes that it doesn't need to do that, which means that other system libraries still have pointers to the system copy of libswiftCore.dylib, which then results in sadness.
Risk: Only affects tests. The final binaries are built with a different deployment target rather than the default 10.13.
Original PR: #75709
Reviewed by: @edymtt
Resolves: rdar://132710670
Tests: Backtracing/EarlyMessage.swift and Backtracing/Timing.swift will fail for some builds without this change. With it, they work.

@al45tair al45tair added swift 6.0 🍒 release cherry pick Flag: Release branch cherry picks labels Aug 9, 2024
@al45tair
Copy link
Contributor Author

al45tair commented Sep 3, 2024

Sounds like we aren't going to fix this for release 6.0. It only affects the tests anyway (the actual build will be done with an appropriate target).

@al45tair al45tair closed this Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant