Repository navigation
Is Box FFI-safe? #157
Description
The section about the representation of pointers gives an example of C API using Box<T> on the Rust side and mapping it to struct T* on the C side. Box<T> is used both as parameter and return types. If it is sound to use Box<SomethingSized> across FFI boundaries and handle it as struct SomethingSized* or struct SomethingSized const* on the C side, wouldn't it be worth adding a short paragraph stating so to complement the example? It's reassuring to have a clear statement about what is or isn't sound.
If this is already specified somewhere, I apologize. I couldn't find a definitive answer.
Reacted by comex, Mattie Conover, Victor Sergienko and Luo.
Activity
gnzlbg commentedon Jul 7, 2019
Box<T>has the same layout as*mut T- that's guaranteed in theBoxdocumentation, which says that it is always valid to convert one into the other.The representation of pointers section guarantees that
*mut Thas the same layout as a C raw pointer iffT: Sized.In the example you mentioned,
T: Sized, soBox<T>is identical to aT*in C, and can be used in C FFI to interface with that.stephaneyfx commentedon Jul 7, 2019
@gnzlbg Thank you for your answer.
The
Boxdocumentation just says "A pointer type for heap allocation" and links to the module documentation. Theboxedmodule documentation does not state clearly that "Box<SomethingSized>has the same layout as*mut SomethingSized" or that they can be used interchangeably inextern "C"functions. Instead, it specifies thatBox::into_rawandBox::from_rawcan be used to convert between them: "More precisely, avalue: *mut T[...] may be converted into a box usingBox::<T>::from_raw(value). Conversely, the memory backing avalue: *mut Tobtained fromBox::<T>::into_rawmay be deallocated [...]". And this is what I have been doing in my FFI code, but advanced Rust developers on Discord suggested to avoid juggling withinto_rawandfrom_rawand just useBox<SomethingSized>and*mut SomethingSized/*const SomethingSizedinterchangeably instead, leading to lessunsafecode and fewer chances to leak. They also suggested that opening a documentation issue might be worth it.If manipulating these 2 types interchangeably in FFI is sound, it would be nice to have a clear statement in the docs rather than just a single code example happening to rely on this fact without further explanations. For example, the nomicon FFI section clearly states that
Option<extern "C" fn()>corresponds to a nullable function pointer in C: "SoOption<extern "C" fn(c_int) -> c_int>is a correct way to represent a nullable function pointer using the C ABI (corresponding to the C typeint (*)(int))."I can help and open a doc PR.
gnzlbg commentedon Jul 7, 2019
The section of the documentation I meant is here:
For me that's clear. Looking at the implementation of
Box<T>, it is notrepr(transparent), but that might just be a bug.gnzlbg commentedon Jul 7, 2019
Given that
Unique<T>isrepr(transparent)(see here), forBoxthe omission appears to be a bug.CryZe commentedon Jul 7, 2019
That sounds like you are meant to explicitly convert between them though. That doesn't necessarily imply they are layout compatible. I think that's still vague enough that it's worth clarifying either in Box's documentation or somewhere in the UCG reference.
I guess just adding #[transparent] on it might be enough though, if that happens.
gnzlbg commentedon Jul 7, 2019
Right now
Box<T>isn't transparent, but that's a bug: rust-lang/rust#52976Layout-wise, it has currently the same layout as a *mut T as long as
T: Sized, but that's unspecified. The right place to specify that would be the Box documentation.Lokathor commentedon Jul 7, 2019
The
repr(transparent)RFC statesSo, if the RFC was successfully implemented correctly then you can non-reliably use Box as if it was transparent already.
We should of course get it upgraded to be
repr(transparent)with full certainty, but until then code can limp along if absolutely necessary.stephaneyfx commentedon Jul 7, 2019
@gnzlbg Thank you for linking this other issue. I will open a PR for the
Boxdocumentation.stephaneyfx commentedon Jul 9, 2019
Closing as rust-lang/rust#62514 was made for the
Boxdocumentation as instructed.gnzlbg commentedon Jul 9, 2019
FWIW this PR #164 here would guarantee this for many cases, independently of whether
Boxdocuments it or istransparentor not.hanna-kruppe commentedon Jul 9, 2019
Hold up. #164 only talks about layout, and while we've been bikeshedding what exactly "layout" is, it isn't currently officially defined to include call ABI and the discussion being written up (#34) definitely happened back when layout including call ABI wasn't even on the table.
(This is actually part of a larger worry for me: all this bikeshedding about "layout" could cause immense confusion, and if we ever pull the trigger on redefining it we'll have to audit all uses of the term in existing write-ups.)
gnzlbg commentedon Jul 9, 2019
The intent of #164 is to include call ABI, if that gets included as part of layout. I suppose we could be more explicit here.
I think this kind of iteration is inevitable, and some of these terms will get tweaked until the validity discussions conclude. Before preparing an RFC, we need to make sure that the definitions are consistent, and that the text uses them appropriately.
Lokathor commentedon Jul 9, 2019
For clarification, if #164 happens, then that's basically saying "anything not explicitly
repr(transparent)implicitly is transparent anyway as long as it qualifies"?Clarify `Box<T>` representation and its use in FFI
gnzlbg commentedon Jul 10, 2019
@Lokathor I don't know the exact rules for
repr(transparent)on structs, but the only thingrepr(transparent)actually does is produce a nice error if the layout of a type would differ from the layout of one of its fields. It has nothing to do with what layout that type has if you omitrepr(transparent), and I think it does make sense to guarantee them to be the same, at least for some cases.Maybe we would need a
repr(transparent)sub section in some general layout page about this.Lokathor commentedon Jul 10, 2019
Yeah if it's just an error check against possible future changes that's important to know.
gnzlbg commentedon Jul 10, 2019
Well, right now is the only thing that's guaranteed to give you that layout. If you omit
repr(transparent), you might or might not get the layout - you definitely cannot "just" rely on it, and would need to inspect compiler internals, etc. and would be depending on an implementation detail.If we were to guarantee #164, you could just rely on it, always. Independently of whether you use
repr(transparent)or not.In general, this wouldn't matter, because if you can stamp
#[repr(transparent)], why not just do that ?But for
Box<T, A>one cannot stamp#[repr(transparent)]becauseAisn't necessarily a ZST. Without #164, you can't, however, rely on it even ifAwere a ZST becauseBoxisn't transparent.hanna-kruppe commentedon Jul 10, 2019
Also note that even after #164 is merged, it'll be just a proposal not yet approved by the lang team.
Lokathor commentedon Jul 10, 2019
It'll be a proposal that's also already backed by an approved RFC ;3
gnzlbg commentedon Jul 10, 2019
There is no RFC backing this. If anything, the
repr(transparent)RFC made the opposite case: that these types of guarantees should be very explicit, so that people don't have to know a bunch of rules to know what they can or cannot rely on. If a struct isrepr(transparent), then that's clearly and explicitly part of that type's API, and you are statically guaranteed to get an error if the type cannot berepr(transparent). I think this is a fair point.I imagine that, at some point, once "layout" is 100% set in stone, we could come up with a set of APIs that allow users to query the layout properties of a type, and write code to check, e.g., whether two types are layout compatible, and/or how compatible they are. That would allow writing reliable unsafe code, that is safe even if a third-party library decides to silently remove a
repr(transparent)annotation, which is something that can happen today with repr(transparent), but that unsafe code cannot defend itself against.Lokathor commentedon Jul 10, 2019
in my comment above, #157 (comment), the quote block
is a direct quote from the RFC. the only thing that 164 would change would be a promotion from "implementation detail" to "actually we're set on this now".
RalfJung commentedon Jul 13, 2019
Indeed, and that change is not backed by any RFC.
We should really resolve the question "what is layout" ASAP and then review our existing documents to make sure we are stating the things we want to state (and probably link to the glossary definition of layout every time we say things like "the layout of X and Y are equal").
Rollup merge of rust-lang#62514 - stephaneyfx:box-ffi, r=nikomatsakis