[5.9][move-only] Do not attempt to lazily deserialize the moveonly deinit table in IRGen. #66525
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
…table in IRGen. SIL Functions are serialized in canonical SIL before they have their final ABI adjusted for large function arguments. Large function argument ABI is adjusted to be indirect as part of the transition from canonical SIL to lowered SIL. This means that if we deserialize a function from another module in canonical SIL and attempt to call it in IRGen we will call it with the wrong ABI implying if we reference any fields of the type in the deinit we will most likely crash (among other potential issues). This patch fixes the issue by changing IRGen to not lazily deserialize the moveonly deinit table and its associated functions. Instead if we do not have our table already deserialized, we just call the function's deinit via the destroy value deinit table. rdar://110496872 (cherry picked from commit b2a52ff)
|
@swift-ci test |
jckarter
approved these changes
Jun 12, 2023
gottesmm
changed the title
tbkka
approved these changes
Jun 13, 2023
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: [move-only] Do not attempt to lazily deserialize the moveonly deinit table in IRGen
• Risk: This changes IRGen to no longer attempt to lazily deserialize the moveonly deinit table. The reason that we need to do this is that SIL functions are serialized in canonical SIL before they have their final ABI adjusted for large function arguments. If we lazily deserialize in IRGen, then we may pull in a deinit for a large type without its proper final ABI resulting in us miscompiling when attempting to access fields. Specifically, the deinit (the callee) will expect to be passed a pointer but the caller will pass the arguments ins register.
• Original PR: #66505
• Reviewed By: @jckarter
• Testing: Added regression tests and execution tests that exercised this code path.
• Resolves: rdar://110496872
SIL Functions are serialized in canonical SIL before they have their final ABI adjusted for large function arguments. Large function argument ABI is adjusted to be indirect as part of the transition from canonical SIL to lowered SIL. This means that if we deserialize a function from another module in canonical SIL and attempt to call it in IRGen we will call it with the wrong ABI implying if we reference any fields of the type in the deinit we will most likely crash (among other potential issues).
This patch fixes the issue by changing IRGen to not lazily deserialize the moveonly deinit table and its associated functions. Instead if we do not have our table already deserialized, we just call the function's deinit via the destroy value deinit table.
rdar://110496872
(cherry picked from commit b2a52ff)
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves #NNNNN, fix apple/llvm-project#MMMMM.