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

Use MaybeUninit to avoid aliasing Vec and String while they can be moved. #30

Merged
merged 3 commits into from
Dec 13, 2024

Conversation

de-vri-es
Copy link
Contributor

@de-vri-es de-vri-es commented Dec 13, 2024

The idea is: by storing the wrapped Template held by TemplateBuf in MaybeUninit, we're not really storing a reference to the source data. Or at-least not as far as the compiler is aware.

Instead, we "recreate" the Template with reference to the source data when we use MaybeUninit::assume_init_ref().

I'm not 100% sure this is totally sound, but miri accepts it. The PR also adds cargo miri test to the CI workflow.

The PR also switches to Pin<String> and Pin<Vec<u8>>, although that currently doesn't actually do anything about aliasing rules. Pin may have different aliasing rules in the future, which will likely be more appropriate.

@de-vri-es de-vri-es force-pushed the no-aliasing branch 2 times, most recently from bb6b5d3 to e71d8f8 Compare December 13, 2024 20:13
@de-vri-es de-vri-es changed the title Use Pin and MaybeUninit to avoid aliasing Vec and String while they are being moved. Use MaybeUninit to avoid aliasing Vec and String while they are being moved. Dec 13, 2024
@de-vri-es de-vri-es marked this pull request as ready for review December 13, 2024 20:15
@de-vri-es de-vri-es changed the title Use MaybeUninit to avoid aliasing Vec and String while they are being moved. Use MaybeUninit to avoid aliasing Vec and String while they can be moved. Dec 13, 2024
This avoids having a real reference to the source data,
which should avoid UB from aliasing the `String`/`Vec`.
@de-vri-es de-vri-es merged commit 2ba38d8 into main Dec 13, 2024
2 checks passed
@de-vri-es de-vri-es deleted the no-aliasing branch December 13, 2024 20:42
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.

1 participant