Skip to content

Rewrite static section bounds discovery in Swift. #1012

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

Merged
merged 10 commits into from
Mar 19, 2025

Conversation

grynspan
Copy link
Contributor

@grynspan grynspan commented Mar 11, 2025

This PR rewrites the static section bounds discovery code in Swift (replacing the current C++ implementation.) We are making use of @_silgen_name here to name the section bounds symbols that are defined by the linker, so we'll want to get the core team's approval before merging.

Why do we need to rewrite this code in Swift?

In Embedded Swift, there is only one (statically linked) image in the process containing Swift code, and if the target is a microcontroller or similar, it probably doesn't even have a dynamic linker/loader in the first place for us to query. So we need, on Embedded Swift targets, to perform static discovery of exactly one test content section. (This is potentially something we can make configurable via the target's toolset in a future PR.)

The bounds of the test content section are currently defined in C++ using __asm__ to reference linker-defined symbols. However, our C++ target can't reliably tell if it is being built for Embedded Swift. That knowledge is treated by the Swift toolchain as a Swift language feature and can be checked in Swift with hasFeature(Embedded), but there's no C++ equivalent. If the implementation is written in Swift, we can know at compile time whether or not we're dynamically or statically linking to a test target (and therefore whether we need to do dynamic or static lookup of images, customized toolset aside.)

Rewriting this logic in Swift reduces our C++ code size (and our direct dependencies on the C++ STL) which is also a nice bonus.

Resolves rdar://147430270.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

@grynspan grynspan added enhancement New feature or request less-c++ Work to reduce the size of our C++ codebase and/or dependencies labels Mar 11, 2025
@grynspan grynspan added this to the Swift 6.2 milestone Mar 11, 2025
@grynspan grynspan self-assigned this Mar 11, 2025
@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan grynspan force-pushed the jgrynspan/move-static-section-bounds-to-swift branch from 09723a9 to 3c9eeff Compare March 11, 2025 14:56
@grynspan grynspan added darwin 🍎 macOS, iOS, watchOS, tvOS, and visionOS support embedded-swift 📟 Embedded Swift issues wasi/wasm 🧭 WebAssembly support labels Mar 11, 2025
Base automatically changed from jgrynspan/custom-metadata-section to main March 11, 2025 19:10
@grynspan grynspan force-pushed the jgrynspan/move-static-section-bounds-to-swift branch 2 times, most recently from eae08c4 to 8e967cf Compare March 11, 2025 19:16
@grynspan grynspan changed the title Rewrite static section bounds discovery in Swift. [DO NOT MERGE] Rewrite static section bounds discovery in Swift. Mar 11, 2025
@grynspan grynspan force-pushed the jgrynspan/move-static-section-bounds-to-swift branch from 8e967cf to e107fa4 Compare March 12, 2025 12:36
@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan grynspan added the discovery 🔎 test content discovery label Mar 12, 2025
This PR rewrites the static section bounds discovery code in Swift (replacing
the current C++ implementation.) We are making use of `@_silgen_name` here to
name the section bounds symbols that are defined by the linker, so we'll want to
get the core team's approval before merging.
@grynspan grynspan force-pushed the jgrynspan/move-static-section-bounds-to-swift branch from e107fa4 to 7a4f5dd Compare March 17, 2025 14:12
@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan grynspan requested review from Azoy, kubamracek and atrick March 17, 2025 14:31
@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan
Copy link
Contributor Author

@swift-ci test Windows

@grynspan grynspan changed the title [DO NOT MERGE] Rewrite static section bounds discovery in Swift. Rewrite static section bounds discovery in Swift. Mar 18, 2025
@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan grynspan requested a review from Azoy March 18, 2025 18:16
@grynspan
Copy link
Contributor Author

@swift-ci test

Copy link

@Azoy Azoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful 👍

@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan grynspan closed this Mar 18, 2025
@grynspan grynspan reopened this Mar 18, 2025
@grynspan
Copy link
Contributor Author

Honestly, that "Close with comment" button is too easy to click.

@grynspan grynspan marked this pull request as ready for review March 18, 2025 18:22
@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan
Copy link
Contributor Author

@swift-ci test

@grynspan
Copy link
Contributor Author

Waiting for a toolchain build to complete before merging.

Copy link

@jckarter jckarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems unlikely that this would break.

@grynspan
Copy link
Contributor Author

@jckarter Famous last words.

@grynspan grynspan merged commit cf94bdc into main Mar 19, 2025
3 checks passed
@grynspan grynspan deleted the jgrynspan/move-static-section-bounds-to-swift branch March 19, 2025 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
darwin 🍎 macOS, iOS, watchOS, tvOS, and visionOS support discovery 🔎 test content discovery embedded-swift 📟 Embedded Swift issues enhancement New feature or request less-c++ Work to reduce the size of our C++ codebase and/or dependencies wasi/wasm 🧭 WebAssembly support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants