-
Notifications
You must be signed in to change notification settings - Fork 10.5k
SourceKit: explicitly link against BlocksRuntime as well #19668
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
Conversation
Please test with following pull request: @swift-ci please test |
Build failed |
Build failed |
@compnerd I am not sure I understand the comment about building BlocksRuntime with hidden visibility. As I mentioned in Also the description contains placeholder text, can you update it or delete it? |
@kevints, I agree that in the end we need a single copy of BlocksRuntime in the runtime. The hidden visibility is masking over the problem that the BlocksRuntime is not currently built shared. Building it shared is the only way to really guarantee that it is truly unique. The thing is, in order to get it to build shared, we need to fix a staging issue (the BlocksRuntime is not currently copied to a location that swift-pm is expecting. Once we fix that, we should have a shared BlocksRuntime which will give us the uniqueness on all the targets. It also will mirror what currently is the case on Darwin - libSystem.dylib uses a LC_REEXPORT_DYLIB for libSystem_blocks.dylib to provide a shared BlocksRuntime. |
I am still reading the PR. But this seems like something that @gparker42 may be interested in. |
Temporarily using a private BlocksRuntime should be safe for now. In the event that you get two runtimes in the same process, they'll probably have the same contents: the blocks runtime is a slow-moving target. Also I can't think of any places that perform comparisons of any symbol addresses (e.g. nothing checks for isa == _NSConcreteMallocBlock so nothing should be confused by two copies of _NSConcreteMallocBlock itself). |
@gparker42 - yeah, thats my take as well. Plus, the short term that I'm talking about here is days (I have changes for libdispatch to build both BlocksRuntime -- shared and static -- and then a subsequent change to use the shared BlocksRuntime to build SourceKit). |
5929e00
to
da22801
Compare
@swift-ci please test |
Build failed |
Build failed |
On Darwin platforms, libdispatch and libBlocksRuntime are re-exported from libSystem (via LC_REEXPORT_DYLIB). Other platforms do not have libdispatch and libBlocksRuntime in their C runtime, so we need to explicitly link against them. Now that we are building BlocksRuntime with hidden visibility, we do not accidentally get the symbols from libdispatch.
da22801
to
e7bc37b
Compare
@swift-ci please test |
Build failed |
Build failed |
On Darwin platforms, libdispatch and libBlocksRuntime are re-exported from
libSystem (via LC_REEXPORT_DYLIB). Other platforms do not have libdispatch and
libBlocksRuntime in their C runtime, so we need to explicitly link against them.
Now that we are building BlocksRuntime with hidden visibility, we do not
accidentally get the symbols from libdispatch.