Skip to content

Package CMO: add deserialization checks to ensure correct memory layout #78258

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 1 commit into from
Jan 11, 2025

Conversation

elsh
Copy link
Contributor

@elsh elsh commented Dec 17, 2024

Package optimization allows bypassing resilience, but that assumes the memory layout of the decl being accessed is correct. When this assumption fails due to a deserialization error of its members, the use site accesses the layout of the decl with a wrong field offset, resulting in UB or a crash. The deserialization error is currently not caught at compile time due to LangOpts.EnableDeserializationRecovery being enabled by default (to allow for recovery of some of the deserialization errors at a later time). In case of member deserialization, however, it's not necessarily recovered later on and is silently dropped.

This PR tracks errors in member deserialization by recursively loading members of a type and checking for deserialization errors. and fails and emits a diagnostic. It prevents resilience bypassing when the deserialized decl's layout is incorrect. It also provides an opt-out flag for deserialization checks for temporary migration purposes.

Resolves rdar://132411524

@elsh
Copy link
Contributor Author

elsh commented Dec 17, 2024

@swift-ci test

@elsh elsh force-pushed the elsh/disallow-bypass-deser-check branch from 2a5a1e6 to 9589546 Compare December 18, 2024 03:10
@elsh
Copy link
Contributor Author

elsh commented Dec 18, 2024

@swift-ci test

@elsh elsh requested a review from nkcsgexi December 18, 2024 03:12
@elsh elsh force-pushed the elsh/disallow-bypass-deser-check branch from c24f01a to be6e16a Compare December 21, 2024 09:36
@elsh elsh requested a review from DougGregor as a code owner December 21, 2024 09:36
@elsh elsh force-pushed the elsh/disallow-bypass-deser-check branch from be6e16a to 0633894 Compare January 7, 2025 04:12
@elsh
Copy link
Contributor Author

elsh commented Jan 7, 2025

@swift-ci test

@elsh
Copy link
Contributor Author

elsh commented Jan 8, 2025

@swift-ci test linux

@elsh elsh force-pushed the elsh/disallow-bypass-deser-check branch from c50fd24 to 12c2e3d Compare January 8, 2025 05:35
@elsh
Copy link
Contributor Author

elsh commented Jan 8, 2025

@swift-ci smoke test

@elsh elsh changed the title Package optimization: Check for deserialization error when bypassing resilience Package CMO: add deserialization checks to ensure correct memory layout Jan 8, 2025
…e memory layout of the

decl being accessed is correct. When this assumption fails due to a deserialization error
of its members, the use site accesses the layout with a wrong field offset, resulting in
UB or a crash. The deserialization error is currently not caught at compile time due to
LangOpts.EnableDeserializationRecovery being enabled by default to allow for recovery of some
of the deserialization errors at a later time. In case of member deserialization, however,
it's not necessarily recovered later on.

This PR tracks whether member deserialization had an error by recursively loading members and
checking for deserialization error, and fails and emits a diagnostic. It provides a way to
prevent resilience bypassing when the deserialized decl's layout is incorrect.

Resolves rdar://132411524
@elsh elsh force-pushed the elsh/disallow-bypass-deser-check branch from 12c2e3d to c03abed Compare January 8, 2025 05:52
@elsh
Copy link
Contributor Author

elsh commented Jan 8, 2025

@swift-ci smoke test

@nkcsgexi
Copy link
Contributor

nkcsgexi commented Jan 8, 2025

This PR is probably worth an eye from @xymus .

@elsh
Copy link
Contributor Author

elsh commented Jan 8, 2025

@swift-ci smoke test

Copy link
Contributor

@aschwaighofer aschwaighofer left a comment

Choose a reason for hiding this comment

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

This looks good to me in principle.

I'll leave it to AST/TypeChecker code base experts to comment on whether the implementation of IterableDeclContext::checkDeserializeMemberErrorInPackage is the best way to do this.

@elsh
Copy link
Contributor Author

elsh commented Jan 11, 2025

Calling the IterableDeclContext::checkDeserializeMemberErrorInPackage method can be opted out with a flag. I’ll merge this PR as is and address any issues that come up.

@elsh elsh merged commit 727fb8c into main Jan 11, 2025
3 checks passed
@elsh elsh deleted the elsh/disallow-bypass-deser-check branch January 11, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants