Fix bad interaction between SIL deserialization and weak linking [5.1 08/28] #27104
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
First, remove the AvailabilityContext parameter; it was confusing because we actually always want to use the deployment target here. Then, split this method up into three methods: - isAlwaysWeakImported(): simply checks for a @_weakLinked attribute, either on the declaration itself or one of its parent contexts. - getAvailabilityForLinkage(): returns the OS version availability when this declaration was introduced, or if the declaration does not have explicit availability, check it's storage (if its an accessor), or its parent contexts. - isWeakImported(ModuleDecl *fromModule): combines these two checks to determine if the declaration should be weak linked when referenced from the given module, or if it might be weak referenced from some module (if the module parameter is null).
…ported flag The weak imported flag is now only set if the attribute is unconditionally weak linked, which is the case when it or one of its parent contexts has a @_weakLinked attribute. To correctly handle weak linking based availability with serialized SIL functions, we need to serialize the actual version tuple when the SIL function was introduced. This is because the deployment target of the client app can be older than the deployment target that the original module was built with. Fixes <rdar://problem/52783668>.
|
@swift-ci Please test |
|
Build failed |
|
@dcci @shahmishal Is this a known failure on this branch? Looks unrelated to my PR: |
|
@swift-ci Please test Linux |
tkremenek
approved these changes
Sep 10, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: When using conditionally-available framework declarations from a client app, we're supposed to weak link those declarations. However if the reference came from deserialized SIL code, eg if the client calls a function in the framework that has a default argument, then that reference was not weak linked. This is because SIL functions only stored a single 'weak linked' bit that was computed from the deployment target of the framework together with the declaration's availability. But instead we need to store the version range in the SIL function and make the determination based on the deployment target of the client into which the SIL function is deserialized.
Scope of the issue: This affects anyone that's writing an app that deploys to OS version N, but takes advantage of a Swift-only framework that's available in version N+1.
Origination: The problem has been there ever since weak linking was introduced in Swift 5.0.
Risk: It's a big change, so medium, but mostly it's a straightforward change.
Reviewed by: @jrose-apple
Radar: rdar://problem/52783668