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

fix for Miri and add Miri to CI #27

Merged
merged 2 commits into from
Aug 2, 2019
Merged

fix for Miri and add Miri to CI #27

merged 2 commits into from
Aug 2, 2019

Conversation

RalfJung
Copy link
Contributor

@RalfJung RalfJung commented Aug 2, 2019

This fixes running the fast path allocation function in Miri. The issue with the old function is that it implicitly calls Vec::deref_mut, which creates a mutable slice covering all elements of the vector -- so if there are other pointers into that vector, they are now invalid, because the slice must be a unique pointer. The fix is to avoid Vec::deref_mut.

This is an instance of rust-lang/unsafe-code-guidelines#133: the mutable reference (for the slice) is created but never used, and still Stacked Borrows requires it to be unique. See that issue for further details.

This also sets up CI to run Miri. I hope I got that right.

Copy link
Collaborator

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Thanks for setting up CI integration!

I'll need to steal this for bumpalo too. I tried once to run miri on that and didn't really get very far.

Thanks @RalfJung :)

@fitzgen fitzgen merged commit 0cf4323 into thomcc:master Aug 2, 2019
@RalfJung
Copy link
Contributor Author

RalfJung commented Aug 2, 2019

I'll need to steal this for bumpalo too. I tried once to run miri on that and didn't really get very far.

Feel free to ping me if you are still running into problems. :)

@RalfJung RalfJung deleted the miri branch August 2, 2019 18:34
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.

2 participants