Remove object lifetime cast #1

BoxyUwU · 2025-03-13T17:18:00Z

Hi there o/

I'm a member of the Rust Language Types Team; as part of stabilizing the arbitrary_self_types and derive_coerce_pointee features we may need to change what raw pointer casts are legal (rust-lang/rust#136702). Specifically, casting *const dyn Trait + 'a to *const dyn Trait + 'b where it is not able to be proven that 'a outlives 'b may become an error.

Without going into too much detail, the justification for this change be that casting trait object's lifetime bound to a lifetime that lives for a longer time could invalidate the VTable of the trait object, allowing for dispatching to methods that should not be callable. See this example from the linked issue:

#![forbid(unsafe_code)]
#![feature(arbitrary_self_types, derive_coerce_pointee)]

use std::any::TypeId;
use std::marker::{CoercePointee, PhantomData};

#[derive(CoercePointee)]
#[repr(transparent)]
struct SelfPtr<T: ?Sized>(*const T);

impl<T: ?Sized> std::ops::Deref for SelfPtr<T> {
    type Target = T;
    fn deref(&self) -> &T {
        panic!("please don't call me, I just want the `Receiver` impl!");
    }
}

trait GetTypeId {
    fn get_type_id(self: SelfPtr<Self>) -> TypeId
    where
        Self: 'static;
}

impl<T: ?Sized> GetTypeId for PhantomData<T> {
    fn get_type_id(self: SelfPtr<Self>) -> TypeId
    where
        Self: 'static,
    {
        TypeId::of::<T>()
    }
}

// no `T: 'static` bound necessary
fn type_id_of<T: ?Sized>() -> TypeId {
    let ptr = SelfPtr(
        &PhantomData::<T> as *const (dyn GetTypeId + '_) as *const (dyn GetTypeId + 'static),
    );
    ptr.get_type_id()
}

Unfortunately, going through the usual "future compatibility warning" process may turn out to not be possible as checking lifetime constraints without emitting an error is prohibitively difficult to do in the implementation of the borrow checker. This means that you may wind up never receiving a FCW and its associated grace period to update your code to be compatible with the new rules.

I have attempted to update your codebase to the new rules for you so that it will still compile if we make this change. I hope this potential breakage won't cause you too much trouble and that you might even find the post-migration code to be better than how it was previously :-)

Finally, it has not been decided yet whether we are to go through with this breaking change. If you feel that your code should continue to work as-is then please let me know and we'll try to take that into account when evaluating whether to go through with this breakage. Additionally if you have any questions about why this change is required please let me know and I'll do my best to further explain the justification.

WorldSEnder · 2025-03-13T17:59:53Z

This seems reasonable. Are there any plans for change the ownership assumptions I make over the pointer metadata? Let me explain:

The current code basically wants to cast Arc<dyn T + 'a> where 'a is some local lifetime to Arc<dyn T + 'static> and assumes that the latter will have access to the vtable for the however long it wants to. While I don't think the linked issue changes this, I want to make sure this is still the case.

Further, the linked issue definitely shows behaviour that shouldn't be possible in safe Rust, but since my code already wraps it in unsafe, are there plans to allow this sort of cast in unsafe as is? Using transmute always feels like bringing a chainsaw just to cut some cheese on the kitchen table.

Thanks for the heads up in any case!

BoxyUwU · 2025-03-21T22:13:56Z

I don't believe anything is changing here in a way that affects the underlying concept of what you're trying to do here (i.e, if it was correct before it will continue to be correct). In general you just have to be careful when unsafely extending the lifetime of the trait object that you not expose it to safe code that expects the lifetime bound to be "truthful".

The VTable in some sense is "valid" at any point regardless of the lifetime bound, or in otherwords, VTables don't contain potentially dangling references and it effectively lives for 'static. This is true of both current stable and also in the future world where casting object lifetimes requires unsafe. The only sense in which it becomes "invalid" is that some lifetime bounds on functions may have become satisfiable after your lifetime cast where previously they would not have been (which may lead to functions being callable that should not be).

I don't think its been brought up yet in discussion as to whether to retain the current behaviour inside of a unsafe contexts. I'll make sure that's considered before any breakage makes its way to stable, though I can't guarantee anything here of course :)

I do agree that transmute feels very heavy handed though and it's unfortunate that this is the only migration strategy currently (though once again I can't guarantee there being a different path forwards).

Forbid freely casting lifetime bounds of dyn-types Fixes #136702 Reference PR: - rust-lang/reference#1951 Background reading about VTable calls/dyn compatibility: https://hackmd.io/zUp-sgZ0RFuFgsNfD4JqYw This PR causes us to start enforcing that lifetimes of dyn types are constrained through pointer casts. Currently on stable casting `*mut dyn Trait + 'a` to `*mut dyn Trait + 'b` passes with no requirements on `'a` or `'b`. Under this PR we now require `'a` to outlive `'b`. Even though the pointee of `*mut` pointers is considered to be invariant, we still use subtyping rather than equality. This mirrors how we support coercing `&mut dyn Trait + 'a` to `&mut dyn Trait + 'b` while requiring only `'a: 'b`. I believe this coercion is sound as there is no way for safe code to `mem::swap` two `dyn Trait`'s, and the same is definitely true of raw pointers. See the changes to this test: https://github.com/rust-lang/rust/pull/136776/files#diff-5523f20a800287a89c9f3e92646c887f3f7599be006b29dd9315f734a2137764 We also do not enforce any constraints on the lifetime of the dyn types if there are multiple pointer indirections. For example `*mut *mut dyn Trait + 'a` is allowed to be casted to `*mut *mut dyn Trait + 'b` with no requirements on `'a` or 'b`. This case is just a normal thin pointer cast where we do not care about the pointee type as there is no VTable in play. Test: https://github.com/rust-lang/rust/pull/136776/files#diff-3b6c8da342bb6530524158d686455a545bb8fd6f59cf5ff50d1d991ce74c9649 Finally, this is about *any* cast where the pointee is *unsized* with dyn-type metadata, not just *literally* the pointee type being a dyn-type. E.g. casting `*mut Wrapper<dyn Trait + 'a>` to `*mut Wrapper<dyn Trait + 'b>` requires `'a: 'b` under this PR. Test: https://github.com/rust-lang/rust/pull/136776/files#diff-ca0c44df62ae1ad1be70f892f01a59714336c7baf78602a5887ac1cf81145c96 ### Breakage This is a breaking change. Crater Report Comment: #136776 (comment) Generated Report: https://crater-reports.s3.amazonaws.com/pr-136776-2/index.html The majority of the breakage is caused by the `metrics` crate with 142 of the regressions, and the `may` crate with 14 of the regressions. The `metrics` crate has been fixed and has backported the fix to previous versions of the crate that were also affected. The`may` crate has also been fixed. PRs against affected crates have been opened and can be seen here: - belalang-project/belalang#6 - tyilo/multi-vec#1 - luksan/lox#1 - pfzetto/bring-your-own-memory-demo#1 - vitorhnn/bfr#1 - gipsyh/PPSMC#1 - orengine/orengine#33 - maroider/async_scoped_task#1 - WorldSEnder/scoped_worker_thread#1 - Wind-Corporation/trapiron#5 - Thombrom/snek#1 - Xudong-Huang/may#113 - metrics-rs/metrics#564 - DouglasDwyer/micropool#1 - Magicolo/phylactery#8 - HellButcher/pulz#29 - UxuginPython/rrtk#1 - wvwwvwwv/scalable-delayed-dealloc#4 - ultimaweapon/tsuki#32 There were six regressions I've not filed PRs against: - https://github.com/weiznich/diesel_benches depends on a ~6year old version of diesel (where the regression is) - https://crates.io/crates/cogo/0.1.36 is an old version of cogo, since that release cogo has already been updated to not depend on pattern this PR breaks - https://github.com/cruise-automation/webviz-rust-framework is an archived read only repo so 🤷‍♀️ - makepad_render, doesn't seem to have source available and is 6 years old 🤷‍♀️ - outsource-heap - not on github - zaplib - I couldn't get it to compile locally as it failed to compile a dependency r? `@ghost`

Forbid freely casting lifetime bounds of dyn-types Fixes rust-lang/rust#136702 Reference PR: - rust-lang/reference#1951 Background reading about VTable calls/dyn compatibility: https://hackmd.io/zUp-sgZ0RFuFgsNfD4JqYw This PR causes us to start enforcing that lifetimes of dyn types are constrained through pointer casts. Currently on stable casting `*mut dyn Trait + 'a` to `*mut dyn Trait + 'b` passes with no requirements on `'a` or `'b`. Under this PR we now require `'a` to outlive `'b`. Even though the pointee of `*mut` pointers is considered to be invariant, we still use subtyping rather than equality. This mirrors how we support coercing `&mut dyn Trait + 'a` to `&mut dyn Trait + 'b` while requiring only `'a: 'b`. I believe this coercion is sound as there is no way for safe code to `mem::swap` two `dyn Trait`'s, and the same is definitely true of raw pointers. See the changes to this test: https://github.com/rust-lang/rust/pull/136776/files#diff-5523f20a800287a89c9f3e92646c887f3f7599be006b29dd9315f734a2137764 We also do not enforce any constraints on the lifetime of the dyn types if there are multiple pointer indirections. For example `*mut *mut dyn Trait + 'a` is allowed to be casted to `*mut *mut dyn Trait + 'b` with no requirements on `'a` or 'b`. This case is just a normal thin pointer cast where we do not care about the pointee type as there is no VTable in play. Test: https://github.com/rust-lang/rust/pull/136776/files#diff-3b6c8da342bb6530524158d686455a545bb8fd6f59cf5ff50d1d991ce74c9649 Finally, this is about *any* cast where the pointee is *unsized* with dyn-type metadata, not just *literally* the pointee type being a dyn-type. E.g. casting `*mut Wrapper<dyn Trait + 'a>` to `*mut Wrapper<dyn Trait + 'b>` requires `'a: 'b` under this PR. Test: https://github.com/rust-lang/rust/pull/136776/files#diff-ca0c44df62ae1ad1be70f892f01a59714336c7baf78602a5887ac1cf81145c96 ### Breakage This is a breaking change. Crater Report Comment: rust-lang/rust#136776 (comment) Generated Report: https://crater-reports.s3.amazonaws.com/pr-136776-2/index.html The majority of the breakage is caused by the `metrics` crate with 142 of the regressions, and the `may` crate with 14 of the regressions. The `metrics` crate has been fixed and has backported the fix to previous versions of the crate that were also affected. The`may` crate has also been fixed. PRs against affected crates have been opened and can be seen here: - belalang-project/belalang#6 - tyilo/multi-vec#1 - luksan/lox#1 - pfzetto/bring-your-own-memory-demo#1 - vitorhnn/bfr#1 - gipsyh/PPSMC#1 - orengine/orengine#33 - maroider/async_scoped_task#1 - WorldSEnder/scoped_worker_thread#1 - Wind-Corporation/trapiron#5 - Thombrom/snek#1 - Xudong-Huang/may#113 - metrics-rs/metrics#564 - DouglasDwyer/micropool#1 - Magicolo/phylactery#8 - HellButcher/pulz#29 - UxuginPython/rrtk#1 - wvwwvwwv/scalable-delayed-dealloc#4 - ultimaweapon/tsuki#32 There were six regressions I've not filed PRs against: - https://github.com/weiznich/diesel_benches depends on a ~6year old version of diesel (where the regression is) - https://crates.io/crates/cogo/0.1.36 is an old version of cogo, since that release cogo has already been updated to not depend on pattern this PR breaks - https://github.com/cruise-automation/webviz-rust-framework is an archived read only repo so 🤷‍♀️ - makepad_render, doesn't seem to have source available and is 6 years old 🤷‍♀️ - outsource-heap - not on github - zaplib - I couldn't get it to compile locally as it failed to compile a dependency r? `@ghost`

Remove object lifetime cast

16b2b52

BoxyUwU mentioned this pull request Mar 22, 2025

Forbid freely casting lifetime bounds of dyn-types rust-lang/rust#136776

Merged

WorldSEnder merged commit 363497b into WorldSEnder:master May 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove object lifetime cast #1

Remove object lifetime cast #1

Uh oh!

BoxyUwU commented Mar 13, 2025

Uh oh!

WorldSEnder commented Mar 13, 2025 •

edited

Loading

Uh oh!

BoxyUwU commented Mar 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Remove object lifetime cast #1

Remove object lifetime cast #1

Uh oh!

Conversation

BoxyUwU commented Mar 13, 2025

Uh oh!

WorldSEnder commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BoxyUwU commented Mar 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

WorldSEnder commented Mar 13, 2025 •

edited

Loading