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

Refactor Crystal::System.retry_with_buffer calling #to_slice #14614

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented May 23, 2024

This is a trivial refactor, moving the call to StaticArray#to_slice out of the loop. On repeat iterations buf is a Slice so #to_slice is actually a no-op.

However, this change breaks some specs in the interpreter where it produces only gibberish. Looks like stack corruption.
I don't think this change is responsible, it just makes the problem visible.

Related to #14613
Depends on #14615

@BlobCodes
Copy link
Contributor

I don't think this change is responsible, it just makes the problem visible.

The relevant change is probably that before this commit, the whole initial_buf buffer was copied to buf:

buf = initial_buf

After this, initial_buf was never used again - which means there's 1024 bytes of padding in front of the actually relevant buffer.

It seems like there always has been a stack corruption, but only inside the irrelevant padding region.
Now, the stack corruption inside initial_buf actually matters.

@straight-shoota straight-shoota marked this pull request as ready for review May 29, 2024 13:40
@straight-shoota straight-shoota added this to the 1.13.0 milestone Jun 10, 2024
@straight-shoota straight-shoota merged commit 5fd4595 into crystal-lang:master Jun 11, 2024
61 checks passed
@straight-shoota straight-shoota deleted the refactor/retry_with_buffer-to_slice branch June 11, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants