I'm in need of the ability to package semi-arbitrary data that is either 'static
or 'a
within something with
the properties of Box<dyn Any + 'a>
. (Note that I aim to potentially use this technique with a slightly different container, like maybe bumpalo's Box
.)
I've written some unsafe code that does this by casting data with lifetime 'a
to a version of that type with lifetime 'static
. If the type is already 'static
, its identity is preserved; otherwise, if it is of form T<'a>
, I cast it to T<'static>
. This is restricted only to types that fit these categories. Then, I store the casted data in Box<dyn Any>
, and on downcast, if downcasting to the 'static
variant succeeds, I then cast to the 'a
variant.
My question is, is this code sound? Lifetimes and transmutes are both very subtle, so I'm concerned whether I might not be creating UB.
Here's my current implementation (that passes MIRI). Thanks!
I need to reread again later, but after my first two passes, I get the impression it can leak somehow. Or rather the owning resource can be freed or resized. You could also violate AXM. Again, not sure, need to read closely. I'm sure it Can work soundly, but the issue would be if it got into the wild and triggered my fear cases.
Your type DynBox<'a>
is currently defined to be covariant over 'a
. However, this is unsound: interior-mutability types cannot be soundly converted from longer lifetimes to shorter lifetimes. To demonstrate the issue (Rust Playground):
use std::{cell::Cell, rc::Rc};
pub fn lifetime_transmute<T: ?Sized + 'static>(src: &T) -> &'static T {
struct Helper<'a, T: ?Sized>(Rc<Cell<Option<&'a T>>>);
// SAFETY: Trivial.
unsafe impl<'a, T: ?Sized + 'static> AnyRef<'a> for Helper<'a, T> {
type Static = Helper<'static, T>;
}
let dest = Rc::new(Cell::new(None));
let helper = Helper(dest.clone());
let helper: Helper<T> = DynBox::new(helper).downcast().unwrap();
helper.0.set(Some(src));
dest.get().unwrap()
}
3 Likes
Hm ok definitely unsafe. Any suggestions on how to fix the problem?
Edited because I originally misunderstood the problem.
Either strengthen the safety requirements for implementing AnyRef
to require covariance in 'a
, or make DynBox<'a>
invariant in 'a
by using something like PhantomData<Cell<&'a ()>>
or PhantomData<fn(&'a ()) -> &'a ()>
in place of PhantomData<&'a ()>
.
The safety comment on AnyRef
probably needs some improvement in any case. E.g. it would (usually) be unsound for some type Foo<'a>
to have both Foo<'a>: AnyRef<'a>
and Foo<'static>: AnyRef<'a>
, yet one of the implementations could fall under the
// ... if the type is the result of parameterizing a type constructor
/// with the lifetime 'a, the result of that type constructor parameterized
/// with 'static.
clause while the other can fall under
// the type for which this is implemented
3 Likes
Also, you might want to note in AnyRef
's docs that the type cannot be parameterized over any lifetime other than 'a
. Otherwise, converting to a DynBox<'a>
and back could trivially transmute the other lifetimes. (That is, impl<'a, 'b> AnyRef<'a> for MyType<'a, 'b>
must be disallowed, but impl<'a> AnyRef<'a> for MyType<'a, 'a>
can be allowed.) Once you do that, I think your DynBox
is completely sound.
1 Like
That was really helpful, thanks!
It has some of the concerns as static mut
-- i.e. the mere existence of two aliasing &mut
is technically UB. If I follow your safety guidelines for &'_ mut i32
for example, your code has that situation for a brief period when transforming into/out-of DynBox
.
One may say, well, that's just some transient state that is never observed, so it's okay. And this may be true in practice today. But there is also a history of things like RFC 2582.
Perhaps there's a way to "rephrase" the implementation so that the output is a reborrow of the input. I think this might actually be easier with borrowed data, but don't have the time to sink into it at this moment.
1 Like
If there is a safer way to do the transmute I'd love to do it; I wish std::mem::transmute
were usable in a generic context even if it might cause monomorphization errors.
I suppose you're referring to the simultaneous ManuallyDrop<&mut i32>
and returned &mut i32
in transmute
. But as I understand it, creating the second reference only makes the reference in the ManuallyDrop
an invalid value. Since we destruct the ManuallyDrop
without touching it again, we never have to assert the first reference's validity; otherwise, methods like ManuallyDrop::drop()
would be wildly unsafe. I've depended on this property myself for dropping a ManuallyDrop<Box<_>>
within a Drop
impl, and Ralf Jung seemed to tentatively agree with the reasoning.
On the road thought - perhaps you could wrap the static type in MaybeUninit (at the cost of leaking if dropped instead of reconstructed).
I mean between transmutation and the unsizing plus pairing with the lifetime carrier, you hold a mut reference with static lifetime (which can't be a subborrow of the original borrow). It's not inside ManuallyDrop. Similarly when downcasting.
(Sorry for the lack of formatting or detail; I'll check out your links later.)
I don't quite understand what you mean, then. Are you referring to transmute
not preserving the tag for Stacked Borrows? Could you give an example?
1 Like
Actually, given that the point of SyncUnsafeCell
is to supply an alternative to static mut
wherein "you only have to prove data accesses correct instead of the existence of the references themselves" (to quote eddyb), I think using UnsafeCell<T::Static>
instead of T::Static
would address the (perhaps overcautious?) concern. (Or a SyncUnsafeCell
-like wrapper if you need Sync
too.)
I wasn't thinking in SB terms. I meant that there is an outstanding mutable borrow active, but you create an independent &'static mut
to the same location before the original mutable borrow expires. I wrote up some notes in //#
comments here. Probably the important bits are about the new
case within the body of the local transmute
function.
Here's another playground just to illustrate that Miri doesn't currently catch the "two existing &mut
to the same memory" case (or this particular version of it anyway).
I think it's different from your citation in that I have a &mut
inside a ManuallyDrop
, not a &mut
to one or about a Box
in one. But I guess an argument could go, it's okay to have a &mut
inside a ManuallyDrop
and do something to make it invalid in some sense, so long as you never take it back out afterwards. That doesn't really jive with UCG 245 I think, though.
And finally, in my experience "do things to the &mut itself" doesn't mean you somehow get new abilities beyond your original lifetime. That is, is it really okay to create the &'static mut
during the original borrow, no matter what happens to the reference itself? Usually in my experience, it's really the borrow that matters.
1 Like
If we modify that to impl<'a> AnyRef<'a> for &'a mut u64
directly, Miri indeed emits an error (Rust Playground):
error: Undefined Behavior: trying to reborrow <3011> for Unique permission at alloc1620[0x0], but that tag does not exist in the borrow stack for this location
--> src/main.rs:33:5
|
33 | ret
| ^^^
| |
| trying to reborrow <3011> for Unique permission at alloc1620[0x0], but that tag does not exist in the borrow stack for this location
| this error occurs as part of a reborrow at alloc1620[0x0..0x8]
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <3011> was created by a retag at offsets [0x0..0x8]
--> src/main.rs:28:15
|
28 | let ret = std::mem::transmute_copy::<T, U>(&manually_drop);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <3011> was later invalidated at offsets [0x0..0x8]
--> src/main.rs:30:17
|
30 | let inner = ManuallyDrop::into_inner(manually_drop);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: inside `transmute::<&mut u64, &mut u64>` at src/main.rs:33:5
But it emits no error (Rust Playground) if manually_drop
isn't touched again. This makes sense given my mental model of transmute
, assuming that T
has a unique reference:
- When the function is entered,
val
is a valid object of type T
.
-
val
is moved into ManuallyDrop::new()
. Now, manually_drop
is a valid object of type ManuallyDrop<T>
.
-
mem::transmute_copy(&manually_drop)
is called, producing the return value. The return value is retagged to become a valid object of type U
, and manually_drop
simultaneously becomes an invalid object of type ManuallyDrop<T>
.
-
manually_drop
falls out of scope. It should be well-formed for an invalid ManuallyDrop<T>
to fall out of scope, since otherwise methods like ManuallyDrop::drop()
would be impossible to use. Indeed, manually_drop
is never even touched in the MIR after the transmute_copy()
returns.
- The valid
U
is returned.
Therefore, at no point do we have two valid mutable references: the first is invalidated the moment the second is retagged. That's why Miri emits an error on your modified example, since you pull an invalid object out of manually_drop
. Just letting manually_drop
fall out of scope does not assert its validity in the same way.
Meanwhile, I was somewhat worried about the transmute_copy
w.r.t. retagging. But looking at Stacked Borrows, it handles this scenario without issue: it sees that we have Unique
access from the original val
's tag, and assigns that to the return value's tag.
Also, I had another worry, that a Drop
impl could potentially observe the transient <T as AnyRef>::Static
. Luckily, it seems like this isn't the case: not even Drop
impls can be specialized on the 'static
lifetime. Thus, Drop::drop()
cannot assume that the fields live longer than the original 'a
. (This reinforces my belief that 'static
specialization would make many unsafe
interfaces unsound.)
2 Likes
After sleeping on it and re-reading the documented ManuallyDrop
behavior and safety notes, I think I'm pretty much in agreement. The thing I needed to solidify it in my mental model was really that transmute{,_copy}
, or perhaps we should say unsafe
context generally, can result in accessible objects with valid bit-patterns lying around which nonetheless can no longer be used without invoking UB. That is, I needed a nudge with regards to transmute, not ManuallyDrop
-- the latter has no special behavior around &mut
specifically.
The transmute_copy
is similar to a ManuallyDrop::take
here; being able to use a normal transmute
would be nicer. I think SB moves the tag correctly because it acts on locations, but didn't look into it. I convinced myself the lifetime being extended in transient state is fine too.
There may be a way to trigger unsoundness due to 'static
anyway, but I only tried a surface level impl Drop
on the 'static
variant as well; a way around it would probably be considered a soundness bug at a higher level.