Skip to content
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

Disable inlining for effect functions that manipulate stacks #3231

Merged
merged 2 commits into from
Nov 5, 2024

Conversation

TheNumbat
Copy link
Contributor

@TheNumbat TheNumbat commented Nov 5, 2024

All of the effect continue functions extract a stack pointer from the given continuation and hide it in an immediate.
If we enter the GC between extracing the stack and calling resume, the stack will not be marked, leading to memory corruption.
This situation can only occur due to re-ordering allocations after inlining, so we can work around this issue by disabling inlining.

Additionally, Deep.try/match_with and Shallow.fiber must not enter the GC between alloc_stack and runstack for the same reason.

A better fix would be to change the fiber layout to not rely on storing the stack/last_fiber pointers as immediates, or at least use a single primitive to implement atomic continue and alloc_and_runstack operations.

@TheNumbat TheNumbat added bug Something isn't working runtime labels Nov 5, 2024
@TheNumbat TheNumbat requested a review from stedolan November 5, 2024 17:22
@TheNumbat TheNumbat merged commit 50b8f0a into main Nov 5, 2024
20 checks passed
@TheNumbat TheNumbat deleted the noinline-continue branch November 5, 2024 18:02
@mshinwell
Copy link
Collaborator

I think this probably should have had comments to explain the reason for those attributes in the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants