Soundness conflicts

[RalfJung]

Sometimes it happens that two unsafe-using libraries are sound in isolation, but unsound when combined. Each time that happens, Rust has to decide which side to consider sound. Ultimately this boils down to precisely specifying the safety invariants of all our types -- but doing that requires something like RustBelt, so it is hard to have that discussion in full generality. (Also that would involve T-types at least as much as WG-UCG / T-opsem.)

But meanwhile, it would be good to collect the cases of these conflicts here that we find out there in the wild.

The most famous case of this is of course leakpocalypse: Rc vs pre-Rust-1.0-scoped-threads, which famously got decided in favor of Rc (and mem::forget). Another case is that without union and ManuallyDrop, josephine would be sound. Again the resolution for the ecosystem is clearly in favor of unions and ManuallyDrop.

The point of this thread is not to discuss any of these conflicts and figure out which side we want to bless. It is solely to collect the known cases in a central location.

• take_mut / replace_with vs partial-borrow: see here for details and discussion.
• mk_static vs Stack Tokens: see reddit for details.
• Pin is seriously under-specified and probably has quite a few of these conflicts, here is one.
• "Partial move out of Drop type" vs a pattern that assumes that exactly that does not happen (coming up in this RFC).
• TLS vs stackful coroutines.
• Allowing vs not allowing some lifetime subtyping on dyn Trait.
• Safe life-before-main vs main assuming there are no other threads and libraries assuming their life-before-main is called before their API is used
• Commuting &MaybeUninit<Cell<T>> to &Cell<MaybeUninit<T>> vs the uninit crate

Things get extremely murky when you consider macros -- personally I am skeptical of anything that relies on macro tricks for soundness; "obviously" many of those can be broken if the language gains new features (and obviously we want to have the freedom to add new features to Rust). But for completeness' sake, we can also collect cases involving macros:

• pinned-init vs sufficiently expressive type-alias-impl-trait
• pin! macro vs movable coroutines / generators: pin!() (and any other unsafe code relying on local variables having a fixed location) in a coroutine or gen block is unsound rust#153688
• lien::scope! macro vs async functions (Loophole in some structured loan-backed products (a.k.a., unsoundness in the API) twilligon/lien#1)

[RalfJung]

I am pretty sure I saw another case of this recently, but forgot where...

[oskgo]

I might have an example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0debe7608d5093f1f38d246b50847faf

The two conflicting APIs here are

• The ability to read from any non_null properly aligned pointer to a ZST
• An API that uses owned ZSTs as scope tokens to only allow executing certain functions in a given context.

AFAIK Vec uses the first, but in a way that is consistent with the second point. The second point is similar to things used in take_mut and Stack Tokens with the difference that the tokens are owned, which allows them to be used in situations where lifetimes are insufficient.

[RustyYato]

Both APIs you presented are unsound on their own, for different reasons:

The ability to read from any non_null properly aligned pointer to a ZST

This is unsound on it's own, since it allows reading core::convert::Infallible in safe code. But even if that wasn't the case, we already have safe apis like ghost which rely on ZST tokens for safety.

An API that uses owned ZSTs as scope tokens to only allow executing certain functions in a given context.

This code allows the Foo token to escape the closure so it isn't safe. (This one is easily fixable, just brand Foo with a lifetime).
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=bcfd683ad394e9c4d4727155d72041ab

Another example of this idea is ghost, which I linked above

[oskgo]

I should have considered empty types but this can be easily fixed:

fn nonempty_try_read<T: 'static>(_: &T, p: NonNull<T>) -> Option<T> {
    if (size_of::<T>() == 0) & ((p.as_ptr() as usize) % align_of::<T>() == 0) {
        Some(unsafe {read(p.as_ptr())})
    } else {
        None
    }
}

I admit that this is getting contrived.

How do we know that ghost is sound?

[RustyYato]

So sorry I meant ghost-cell, which was proven correct in RustBelt. See the docs for details

[RalfJung]

Even the fixed version doesn't account for the fact that T can be non-Copy.

What Vec does is perfectly fine and compatible with GhostCell. It will only return to the user exactly as many instances of the ZST as have been in the original Vec. Your nonempty_try_read is buggy but Vec is not.

The status of non-Copy ZST is (to my understanding) well-established. It is probably not as well documented though... I wouldn't even know where such docs would go.^^ There is some prior discussion though:

• Shared and Unique slices and ZST #168
• Constructing ZSTs from thin air #250

I will mark these comments as resolved since there is no soundness conflict here.

[Jules-Bertholet]

rust-lang/rfcs#3466 (the proposed language feature could be emulated with a macro) is another case.

[RalfJung]

A conflict is between two APIs. Which is the pair of APIs here? Presumably your DerefMove vs something that relies on not being able to move out of a ManuallyDrop? We have into_inner though so it has to be a bit more subtle.

EDIT: Ah, found the explanation in the RFC. Would have been helpful to not make me go search for it. :)

[RalfJung]

That one is a bit different in nature, since it's much more syntactic -- there currently is no syntax for partially moving out of a type that implements Drop, and therefore unsafe code might rely on it not happening. The effect is pretty much the same as the semantic conflicts I listed, but there's no safety invariant that we can carefully specify to avoid it.

This is why I am always very nervous about macro-based approaches or anything else that make syntactic arguments.

[steffahn]

pyo3’s Ungil trait implementation (at least on stable Rust) using the Send trait, in conflict with send_wrapper, as documented here. And I believe, as an alternative to send_wrapper, using scoped_tls would be another way to ”smuggle“ the !Send + !'static data beyond the Ungil bound.

[RalfJung]

That doesn't sound like a conflict: pyo3 is abusing Send because they don't have an auto trait that reflects what they really want, so pyo3 is unsound and send_wrapper and scoped_tls are sound. The pyo3 docs even say as much.

[matklad]

work-stealing coroutines vs thread locals: https://internals.rust-lang.org/t/what-is-the-current-safety-story-for-library-based-stackful-coroutines/14863

[RalfJung]

Hm, TLS is not really a library feature, it's a language feature, and it's not like there is an open question of whether or not we have TLS... but sure I can add it to the list.

[Jules-Bertholet]

pyo3 is abusing Send because they don't have an auto trait that reflects what they really want

Note that scoped_tls is a problem even if pyo3 replaces Send with a custom auto trait: PyO3/pyo3#3640

Same for objc2's usage of AutoreleaseSafe: madsmtm/objc2#540

[adamreichold]

Hm, TLS is not really a library feature, it's a language feature, and it's not like there is an open question of whether or not we have TLS... but sure I can add it to the list.

I think what the PyO3 discussion shows is that when libraries venture into language territory - as PyO3's handling of Python's heap, GIL and associated invariants seems to do - a way to exert more control over the boundaries which make up a thread would be really helpful.

For example, something like an "as-if" version of thread::spawn which gives me all Rust-level guarantees and type system interactions currently reserved for operating system threads but without the overhead of actually creating a new operation system thread. Though I am not sure whether this is possible at all due to code written e.g. in C might then break these "as-if" barriers similar to the set_env mess?

Admittedly, this is also probably not the right issue to discuss this, but I just wanted to highlight that this is not just a unsafe library X versus Y problem, but a more fundamental question of how libraries can put the type system notions currently bound to operating system threads to their own uses. So maybe a rather long-winded +1 to the parent comment...