IRGen: Always call metadata accessor for resilient types even when resilience bypass is enabled [4.2] #17736
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
…silience bypass is enabled Otherwise, we might directly use uninitialized metadata, because we think the type has a fixed size, even if it was defined in a different module built with resilience. Fixes <rdar://problem/40034143>, for real this time.
|
@swift-ci Please test |
|
@rjmccall This is another hackaround for LLDB's lack of resiience support. PTAL? |
slavapestov
changed the title
rjmccall
reviewed
Jul 5, 2018
| // However, when -enable-resilience-bypass is on, we might be using a value | ||
| // type from another module built with resilience enabled. In that case, the | ||
| // type looks like it has fixed size to us, since we're bypassing resilience, | ||
| // but the metadata still requires runtime initialization, so its incorrect |
|
I don't really understand the resilience-bypass logic very well, but this looks fine inasmuch as it's something we ought to be able to remove later. |
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: The previous fix for rdar://problem/40034143 was incomplete, because I got the reduced test case working but the original test case was still broken. The problem is when you have a module dependency graph like
A --> B --> Cand A is built with-enable-resilience-bypass(ie, the LLDB REPL), B uses a type from C, and C is built with resilience enabled, then sometimes A would directly access type metadata for types defined in B, instead of going through metadata accessors. This was because to A, everything has a known size, so it thinks the metadata does not require runtime initialization; however when B was built, the size of types in C was not known, so B's types do require runtime initialization.Risk: Low, the fix only changes behavior when generating code for the LLDB REPL.
Scope of the issue: Impacts CreateML and possibly other frameworks that use Foundation value types, which are resilient.
Origination: New in 4.2 when we enabled resilience for stdlib and overlays.
Bug: rdar://40034143 (re-opened)
Reviewed by: @rjmccall