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

Make Vec from a pointer that is valid for mutation #614

Merged
merged 1 commit into from
Feb 7, 2022

Conversation

saethlin
Copy link
Contributor

@saethlin saethlin commented Feb 7, 2022

slice::as_ptr documents that the returned pointer or any pointer derived from it may not be used for mutation. It is therefore unsound to construct a Vec using this pointer, the docs advise as_raw_ptr instead: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.as_ptr

I also specified the type in the cast, because as _ can produce very surprising behavior in unsafe code due to the reference and pointer coercions.

This issue can be detected with MIRIFLAGS="-Zmiri-tag-raw-pointers" cargo miri test

This change would be rendered unnecessary by #603 but I can't tell what state that is in, the CI logs seem to have expired.

slice::as_ptr documents that the returned pointer or any pointer derived
from it may not be used for mutation. It is therefore unsound to
construct a Vec using this pointer, we must use as_raw_ptr instead.
Copy link
Collaborator

@philipc philipc left a comment

Choose a reason for hiding this comment

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

Thanks!

@philipc philipc merged commit 4fd7391 into gimli-rs:master Feb 7, 2022
@philipc
Copy link
Collaborator

philipc commented Feb 7, 2022

I also specified the type in the cast, because as _ can produce very surprising behavior in unsafe code due to the reference and pointer coercions.

Just for my understanding, can you point to an example of how that can go wrong?

@saethlin
Copy link
Contributor Author

saethlin commented Feb 7, 2022

In general casting of pointers is fraught with danger, because const vs mut is just a lint. Stacked Borrows (which is still just a prototype) makes this even more dangerous, because as _ hides what you're casting to. For example, you may do &T as *const T as _, which may convert const to mut, may change the type of the pointee, or both. If the cast is adding mut, this code is probably buggy.

Things can get even more strange in contrived examples. This code compiles, but it is UB per Stacked Borrows and probably by whatever finished pointer model Rust adopts:

unsafe{core::ptr::write(&mut 0i32 as *const i32 as *mut i32,1i32)}

The reason is that there is no as cast from &mut T to *const T. There's an implicit conversion from &mut T to &T inserted to make this compile. It's not necessarily UB to take a *const T and convert it to a *mut T to do writes through it, but in this case it is because raw pointer is derived from a shared reference. But you'd be hard-pressed indeed to figure that out without knowing what the input and output type is for the first as cast.

@philipc
Copy link
Collaborator

philipc commented Feb 8, 2022

If I understand you correctly, the problem is that as itself is surprising, but as _ doesn't add any additional surprises. So the benefit of avoiding as _ is simply that it makes it easier to audit the as conversions.

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