From 318c5d6c963d0e5d8ece89e804b4e696c6011955 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 8 Jul 2019 21:34:36 -0700 Subject: [PATCH 1/8] Clarify `Box` representation and its use in FFI This officializes what was only shown as a code example in [the unsafe code guidelines](https://rust-lang.github.io/unsafe-code-guidelines/layout/function-pointers.html?highlight=box#use) and follows [the discussion](https://github.com/rust-lang/unsafe-code-guidelines/issues/157) in the corresponding repository. It is also related to [the issue](https://github.com/rust-lang/rust/issues/52976) regarding marking `Box` `#[repr(transparent)]`. --- src/liballoc/boxed.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 01dee0a394337..50174c3f279a2 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,6 +63,28 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! +//! `Box` has the same representation as `*mut T`. In particular, when +//! `T: Sized`, this means that `Box` has the same representation as +//! a C pointer, making the following code valid in FFI: +//! +//! ```c +//! /* C header */ +//! struct Foo* foo(); /* Returns ownership */ +//! void bar(struct Foo*); /* `bar` takes ownership */ +//! ``` +//! +//! ``` +//! #[repr(C)] +//! pub struct Foo; +//! +//! #[no_mangle] +//! pub extern "C" fn foo() -> Box { +//! Box::new(Foo) +//! } +//! +//! #[no_mangle] +//! pub extern "C" fn bar(_: Option>) {} +//! ``` //! //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html From aea94230c476fea2ee073a1bc672125e4586d4b5 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Sun, 25 Aug 2019 23:25:56 -0700 Subject: [PATCH 2/8] Update Box representation comment based on reviews --- src/liballoc/boxed.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 50174c3f279a2..b2b23cc9671b1 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,9 +63,8 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! `Box` has the same representation as `*mut T`. In particular, when -//! `T: Sized`, this means that `Box` has the same representation as -//! a C pointer, making the following code valid in FFI: +//! `Box` has the same ABI as `&mut T`. In particular, when `T: Sized`, +//! this allows using `Box` in FFI: //! //! ```c //! /* C header */ From 812ec6a3bf775c1564ed3b12374c4ee81bfa94b8 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Wed, 30 Oct 2019 09:43:04 -0700 Subject: [PATCH 3/8] Update FFI example - Use meaningful names - Clarify comments - Fix C function declaration --- src/liballoc/boxed.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index b2b23cc9671b1..a502a5b0a0b3a 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -68,8 +68,8 @@ //! //! ```c //! /* C header */ -//! struct Foo* foo(); /* Returns ownership */ -//! void bar(struct Foo*); /* `bar` takes ownership */ +//! struct Foo* foo_new(void); /* Returns ownership to the caller */ +//! void foo_delete(struct Foo*); /* Takes ownership from the caller */ //! ``` //! //! ``` @@ -77,12 +77,12 @@ //! pub struct Foo; //! //! #[no_mangle] -//! pub extern "C" fn foo() -> Box { +//! pub extern "C" fn foo_new() -> Box { //! Box::new(Foo) //! } //! //! #[no_mangle] -//! pub extern "C" fn bar(_: Option>) {} +//! pub extern "C" fn foo_delete(_: Option>) {} //! ``` //! //! [dereferencing]: ../../std/ops/trait.Deref.html From ead115949017533de244049c58f4b6886243eda7 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 9 Dec 2019 22:49:59 -0800 Subject: [PATCH 4/8] Use Niko's wording --- src/liballoc/boxed.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index a502a5b0a0b3a..a7e09d72b4a92 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,14 +63,25 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! `Box` has the same ABI as `&mut T`. In particular, when `T: Sized`, -//! this allows using `Box` in FFI: +//! So long as `T: Sized`, a `Box` is guaranteed to be represented as a +//! single pointer and is also ABI-compatible with C pointers (i.e. the C type +//! `T*`). This means that you have Rust code which passes ownership of a +//! `Box` to C code by using `Box` as the type on the Rust side, and +//! `T*` as the corresponding type on the C side. As an example, consider this +//! C header which declares functions that create and destroy some kind of +//! `Foo` value: //! //! ```c //! /* C header */ //! struct Foo* foo_new(void); /* Returns ownership to the caller */ //! void foo_delete(struct Foo*); /* Takes ownership from the caller */ //! ``` +//! +//! These two functions might be implemented in Rust as follows. Here, the +//! `struct Foo*` type from C is translated to `Box`, which captures +//! the ownership constraints. Note also that the nullable argument to +//! `foo_delete` is represented in Rust as `Option>`, since `Box` +//! cannot be null. //! //! ``` //! #[repr(C)] @@ -84,6 +95,14 @@ //! #[no_mangle] //! pub extern "C" fn foo_delete(_: Option>) {} //! ``` +//! +//! Even though `Box` has the same representation and C ABI as a C pointer, +//! this does not mean that you can convert an arbitrary `T*` into a `Box` +//! and expect things to work. `Box` values will always be fully aligned, +//! non-null pointers. Moreover, the destructor for `Box` will attempt to +//! free the value with the global allocator. In general, the best practice +//! is to only use `Box` for pointers that originated from the global +//! allocator. //! //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html From fe6ddd5d15c4f0108c32fb731b688f34fbeba788 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 9 Dec 2019 22:56:37 -0800 Subject: [PATCH 5/8] Specify behavior when passed a null pointer --- src/liballoc/boxed.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index a7e09d72b4a92..2f24086bf2c98 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -73,8 +73,12 @@ //! //! ```c //! /* C header */ -//! struct Foo* foo_new(void); /* Returns ownership to the caller */ -//! void foo_delete(struct Foo*); /* Takes ownership from the caller */ +//! +//! /* Returns ownership to the caller */ +//! struct Foo* foo_new(void); +//! +//! /* Takes ownership from the caller; no-op when invoked with NULL */ +//! void foo_delete(struct Foo*); //! ``` //! //! These two functions might be implemented in Rust as follows. Here, the From 1a26df77272d3bafc9e9e689a5a4a314896050f5 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Tue, 10 Dec 2019 00:00:25 -0800 Subject: [PATCH 6/8] Remove trailing whitespace --- src/liballoc/boxed.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 2f24086bf2c98..c3732b245f420 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -73,14 +73,14 @@ //! //! ```c //! /* C header */ -//! +//! //! /* Returns ownership to the caller */ //! struct Foo* foo_new(void); -//! +//! //! /* Takes ownership from the caller; no-op when invoked with NULL */ //! void foo_delete(struct Foo*); //! ``` -//! +//! //! These two functions might be implemented in Rust as follows. Here, the //! `struct Foo*` type from C is translated to `Box`, which captures //! the ownership constraints. Note also that the nullable argument to @@ -99,7 +99,7 @@ //! #[no_mangle] //! pub extern "C" fn foo_delete(_: Option>) {} //! ``` -//! +//! //! Even though `Box` has the same representation and C ABI as a C pointer, //! this does not mean that you can convert an arbitrary `T*` into a `Box` //! and expect things to work. `Box` values will always be fully aligned, From cb1cc1181e884d0f53c444af0b6a21189af83bea Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Tue, 10 Dec 2019 22:18:17 -0800 Subject: [PATCH 7/8] Fix description based on review --- src/liballoc/boxed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index c3732b245f420..d50b4b8dc4a7b 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -65,7 +65,7 @@ //! //! So long as `T: Sized`, a `Box` is guaranteed to be represented as a //! single pointer and is also ABI-compatible with C pointers (i.e. the C type -//! `T*`). This means that you have Rust code which passes ownership of a +//! `T*`). This means that you can have Rust code which passes ownership of a //! `Box` to C code by using `Box` as the type on the Rust side, and //! `T*` as the corresponding type on the C side. As an example, consider this //! C header which declares functions that create and destroy some kind of From fafa4897985f932490960e90ddd2ff39134e967e Mon Sep 17 00:00:00 2001 From: Nicholas Matsakis Date: Wed, 11 Dec 2019 10:32:11 -0500 Subject: [PATCH 8/8] clarify that `Box` should only be used when defined *in Rust* --- src/liballoc/boxed.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index d50b4b8dc4a7b..c25495bec41ee 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,13 +63,14 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! So long as `T: Sized`, a `Box` is guaranteed to be represented as a -//! single pointer and is also ABI-compatible with C pointers (i.e. the C type -//! `T*`). This means that you can have Rust code which passes ownership of a -//! `Box` to C code by using `Box` as the type on the Rust side, and -//! `T*` as the corresponding type on the C side. As an example, consider this -//! C header which declares functions that create and destroy some kind of -//! `Foo` value: +//! So long as `T: Sized`, a `Box` is guaranteed to be represented +//! as a single pointer and is also ABI-compatible with C pointers +//! (i.e. the C type `T*`). This means that if you have extern "C" +//! Rust functions that will be called from C, you can define those +//! Rust functions using `Box` types, and use `T*` as corresponding +//! type on the C side. As an example, consider this C header which +//! declares functions that create and destroy some kind of `Foo` +//! value: //! //! ```c //! /* C header */ @@ -108,6 +109,14 @@ //! is to only use `Box` for pointers that originated from the global //! allocator. //! +//! **Important.** At least at present, you should avoid using +//! `Box` types for functions that are defined in C but invoked +//! from Rust. In those cases, you should directly mirror the C types +//! as closely as possible. Using types like `Box` where the C +//! definition is just using `T*` can lead to undefined behavior, as +//! described in [rust-lang/unsafe-code-guidelines#198][ucg#198]. +//! +//! [ucg#198]: https://github.com/rust-lang/unsafe-code-guidelines/issues/198 //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html //! [`Global`]: ../alloc/struct.Global.html