Skip to content

[move-only] Do not attempt to lazily deserialize the moveonly deinit table in IRGen. #66505

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jun 9, 2023

The reason to avoid this is if we attempt to lazily deserialize the moveonly deinit table, we may also attempt to deserialize a deinit function. This would result in us breaking the invariant that we no longer deserialize SILFunctions once we are in LoweredSIL.

If we fail to lazily deserialize, we instead just call the deinit via the destroy value witness of the type.

This only affects builds built with asserts enabled since llvm_unreachable is compiled out when NDEBUG is set.

rdar://110496872

@gottesmm gottesmm requested a review from jckarter June 9, 2023 17:59
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 9, 2023

@swift-ci test

@glessard
Copy link
Contributor

glessard commented Jun 9, 2023

@swift-ci please Build Toolchain macOS Platform

…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
@gottesmm gottesmm force-pushed the pr-13b73601ca744f17bd595d547bdfa2b9bb4f0b1e branch from 4dc593d to b2a52ff Compare June 9, 2023 19:42
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 9, 2023

@swift-ci test

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 9, 2023

@swift-ci please Build Toolchain macOS Platform

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