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

Support raw access without lifetimes to UntypedMut #10039

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

udoprog
Copy link

@udoprog udoprog commented Oct 7, 2023

Objective

Raw access through a pointer requires us to shed the lifetime associated with UntypedMut to later reconstruct it using a pointer cast, and the only valid option is 'static. I am genuinely not sure whether this is sound. My hunch is that it isn't because Rust is open to decide that Foo<'a> is laid out differently to Foo<'static>, all though in practice I don't think it is. To be super sure though it's better to have a dedicated set of raw APIs which are guaranteed to be sound.

Solution

  • Switch Ticks and TicksMut to both utilize a layout equivalent interior struct called TicksRaw containing pointers instead of references, and ensures that their lifetype parameters are constrained through PhantomData.
  • UntypedMut gets a new sibling type called UntypedMutRaw which you can construct through UntypedMut::into_raw.
  • UntypedMutRaw gets an unsafe function called as_mut which translates it into an UntypedMut with an unbounded lifetime.

If this looks good, I'd be happy to build out raw Ref access as well since that is also something I'd need. Thank you!

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2023

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@james7132 james7132 self-requested a review October 7, 2023 23:32
@james7132
Copy link
Member

. I am genuinely not sure whether this is sound. My hunch is that it isn't because Rust is open to decide that Foo<'a> is laid out differently to Foo<'static>

Lifetime transmutation is one of the only acceptable uses of transmute, so this is probably sound. However, arbitrary unscoped lifetime extension is 100% unsound. We do this in TaskPool::scope, but we strictly control the exposed API to make sure that the 'static lifetime is never leaked outside of the scope itself, whereas this openly exposes unbounded lifetimes publicly.

@udoprog
Copy link
Author

udoprog commented Oct 8, 2023

[..] However, arbitrary unscoped lifetime extension is 100% unsound. We do this in TaskPool::scope, but we strictly control the exposed API to make sure that the 'static lifetime is never leaked outside of the scope itself, whereas this openly exposes unbounded lifetimes publicly.

This is also why as_mut is unsafe. The caller is responsible for ensuring that the pointed to object is valid and alive, similarly to ptr::NonNull::as_mut. The one aspect I'm worried about is the layout of the struct.

I would also appreciate if there's a rigorous source for the soundness of lifetime transmutes to point to. There is the transmute example but it doesn't fill me with a great deal of confidence, as in:

Extending a lifetime, or shortening an invariant lifetime. This is advanced, very unsafe Rust!

struct R<'a>(&'a i32);
unsafe fn extend_lifetime<'b>(r: R<'b>) -> R<'static> {
    std::mem::transmute::<R<'b>, R<'static>>(r)
}

Reading this, do we know for sure that now and in the future the correctness of the transmute doesn't depend on the layout of R?

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! A-Pointers Relating to Bevy pointer abstractions labels Oct 9, 2023
@TheRawMeatball
Copy link
Member

I'm not sure this is the best way to expose the necessary functionality - the data side of the UntypedMut can already be accessed as raw pointers through the conversions to PtrMut. With that in mind, it seems the only part that's missing is access to the raw pointers to the ticks. I think it could be preferable to simply expose a getter for TicksRaw, instead of having an UntypedMutRaw.

Furthermore, I think it'd be better if neither of these raw options were actually used in bevy, and instead kept only for interfacing with external libraries that can't or won't pipe around the lifetimes, to make as much use of lifetime checking in the bevy codebase as possible and minimize spots where it could get forgotten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Pointers Relating to Bevy pointer abstractions C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants