From a51c26a0087212f155e801153b7ce07d68035fe3 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 16:48:54 +0200 Subject: [PATCH 1/9] Add ABI-stable bytes::Bytes lookalike --- src/_lib.rs | 3 + src/bytes.rs | 292 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 295 insertions(+) create mode 100644 src/bytes.rs diff --git a/src/_lib.rs b/src/_lib.rs index 957ed9eac7..98cc43c91a 100644 --- a/src/_lib.rs +++ b/src/_lib.rs @@ -310,6 +310,9 @@ mod tuple; pub mod option; +pub +mod bytes; + cfg_alloc! { #[doc(inline)] pub use string::String; diff --git a/src/bytes.rs b/src/bytes.rs new file mode 100644 index 0000000000..c3e4714e2b --- /dev/null +++ b/src/bytes.rs @@ -0,0 +1,292 @@ +use core::{ + borrow::Borrow, + fmt::Debug, + hash::Hash, + ops::{Deref, Not, RangeBounds}, +}; + +use alloc::sync::Arc; +use safer_ffi_proc_macros::derive_ReprC; + +/// A slice of bytes optimized for sharing ownership of it and its subslices. +/// +/// Typically, [`Bytes`] can constructed from `&[u8]`, `Arc<[u8]>` or `Arc>`. +#[derive_ReprC] +#[repr(C)] +pub struct Bytes<'a> { + start: &'a u8, + len: usize, + data: *const (), + capacity: usize, + vtable: &'a BytesVt, +} + +extern "C" fn noop(_: *const (), _: usize) {} +impl<'a> Bytes<'a> { + /// Constructs an empty slice. + /// + /// Since [`Bytes`] are immutable slices, this mostly serves to create default values. + pub const fn empty() -> Self { + Self::from_static([].as_slice()) + } + /// Constructs a [`Bytes`] referring to static data. + /// + /// This is preferable to `>::from` in the sense that even if the value is cast to a non-static lifetime, [`Self::upgrade`] won't need to reallocate to recover the `'static` lifetime. + pub const fn from_static(data: &'static [u8]) -> Self { + const VT: BytesVt = BytesVt { + retain: Some(noop), + release: Some(noop), + }; + Self { + start: unsafe { &*data.as_ptr() }, + len: data.len(), + data: data.as_ptr().cast(), + capacity: data.len(), + vtable: &VT, + } + } + /// Slices `self` in-place: + /// ``` + /// let data = b"Hello there".as_slice(); + /// let bytes = Bytes::from_static(data); + /// bytes.subslice(3..7); + /// assert_eq!(&data[3..7], bytes.as_slice()); + /// ``` + /// # Panics + /// If the range's end is out of bounds, or if the range's start is greater than its end. + pub fn subslice>(&mut self, range: R) { + let start = match range.start_bound() { + core::ops::Bound::Included(i) => *i, + core::ops::Bound::Excluded(i) => *i + 1, + core::ops::Bound::Unbounded => 0, + }; + let end = match range.end_bound() { + core::ops::Bound::Included(i) => *i + 1, + core::ops::Bound::Excluded(i) => *i, + core::ops::Bound::Unbounded => self.len, + }; + assert!(start <= end); + assert!(end <= self.len); + let len = end - start; + self.start = unsafe { &*(self.start as *const u8).add(start) }; + self.len = len; + } + /// Slices `self` in-place, returning it. + /// ``` + /// let data = b"Hello there".as_slice(); + /// let bytes = Bytes::from_static(data); + /// assert_eq!(&data[3..7], bytes.subsliced(3..7).as_slice()); + /// ``` + /// # Panics + /// If the range's end is out of bounds, or if the range's start is greater than its end. + pub fn subsliced>(mut self, range: R) -> Self { + self.subslice(range); + self + } + /// Splits the slice at `index`. + /// ``` + /// let data = b"Hello there".as_slice(); + /// let index = 5; + /// let (l, r) = data.split_at(index); + /// let (bl, br) = Bytes::from_static(data).split_at(index).unwrap(); + /// assert_eq!((l, r), (bl.as_slice(), br.as_slice())); + /// ``` + /// + /// If re-allocating was necessary to create a second owner, both returned subslices will refer to a common buffer. + /// + /// # Errors + /// Returns `self` if `index` is out of bounds. + pub fn split_at(self, index: usize) -> Result<(Self, Self), Self> { + if index <= self.len { + let mut left = self.clone(); + let mut right = left.clone(); + left.len = index; + right.len -= index; + right.start = unsafe { &*(self.start as *const u8).add(index) }; + Ok((left, right)) + } else { + Err(self) + } + } + /// Returns the slice's contents. + pub const fn as_slice(&self) -> &[u8] { + unsafe { core::slice::from_raw_parts(self.start, self.len) } + } + /// Proves that the slice can be held onto for arbitrary durations, or copies it into a new one that does. + pub fn upgrade(self) -> Bytes<'static> { + if !self.vtable.is_borrowed() { + return unsafe { core::mem::transmute(self) }; + } + Arc::<[u8]>::from(self.as_slice()).into() + } + /// Attempts to prove that the slice has a static lifetime. + /// # Errors + /// Returns the original instance if it couldn't be proven to be `'static`. + pub fn noalloc_upgrade(self) -> Result, Self> { + if !self.vtable.is_borrowed() { + Ok(unsafe { core::mem::transmute(self) }) + } else { + Err(self) + } + } + /// Only calls [`Clone::clone`] if no reallocation would be necessary for it, returning `None` if it would have been. + pub fn noalloc_clone(&self) -> Option { + self.clone_will_allocate().not().then(|| self.clone()) + } + /// Returns `true` if a call to [`Self::upgrade`] would cause an allocation. + pub const fn upgrade_will_allocate(&self) -> bool { + self.vtable.is_borrowed() + } + /// Returns `true` if a call to [`Clone::clone`] would cause an allocation. + pub const fn clone_will_allocate(&self) -> bool { + self.vtable.retain.is_none() + } +} +impl Default for Bytes<'_> { + fn default() -> Self { + Bytes::empty() + } +} +impl AsRef<[u8]> for Bytes<'_> { + fn as_ref(&self) -> &[u8] { + self.as_slice() + } +} +impl Deref for Bytes<'_> { + type Target = [u8]; + fn deref(&self) -> &Self::Target { + self.as_slice() + } +} +impl Borrow<[u8]> for Bytes<'_> { + fn borrow(&self) -> &[u8] { + self.as_slice() + } +} +impl Debug for Bytes<'_> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("Bytes") + .field("data", &self.as_slice()) + .field("owned", &!self.upgrade_will_allocate()) + .field("shared", &!self.clone_will_allocate()) + .finish() + } +} +impl Hash for Bytes<'_> { + fn hash(&self, state: &mut H) { + self.as_slice().hash(state) + } +} +impl> PartialEq for Bytes<'_> { + fn eq(&self, other: &T) -> bool { + self.as_slice() == other.as_ref() + } +} +impl Eq for Bytes<'_> {} +impl<'a> From<&'a [u8]> for Bytes<'a> { + fn from(data: &'a [u8]) -> Self { + static VT: BytesVt = BytesVt { + release: None, + retain: Some(noop), + }; + Bytes { + start: unsafe { &*data.as_ptr() }, + len: data.len(), + data: data.as_ptr().cast(), + capacity: data.len(), + vtable: &VT, + } + } +} +impl From> for Bytes<'static> { + fn from(data: Arc<[u8]>) -> Self { + extern "C" fn retain(this: *const (), capacity: usize) { + unsafe { + Arc::increment_strong_count(core::ptr::slice_from_raw_parts( + this.cast::(), + capacity, + )) + } + } + extern "C" fn release(this: *const (), capacity: usize) { + unsafe { + Arc::decrement_strong_count(core::ptr::slice_from_raw_parts( + this.cast::(), + capacity, + )) + } + } + static VT: BytesVt = BytesVt { + release: Some(release), + retain: Some(retain), + }; + let capacity = data.len(); + Bytes { + start: unsafe { &*data.as_ptr() }, + len: data.len(), + data: Arc::into_raw(data) as *const (), + capacity, + vtable: &VT, + } + } +} +impl + Send + Sync> From> for Bytes<'static> { + fn from(value: Arc) -> Self { + extern "C" fn retain(this: *const (), _: usize) { + unsafe { Arc::increment_strong_count(this.cast::()) } + } + extern "C" fn release(this: *const (), _: usize) { + unsafe { Arc::decrement_strong_count(this.cast::()) } + } + let data: &[u8] = value.as_ref().as_ref(); + Bytes { + start: unsafe { &*data.as_ptr() }, + len: data.len(), + capacity: data.len(), + data: Arc::into_raw(value) as *const (), + vtable: &BytesVt { + release: Some(release::), + retain: Some(retain::), + }, + } + } +} +unsafe impl Send for Bytes<'_> {} +unsafe impl Sync for Bytes<'_> {} + +#[derive_ReprC] +#[repr(C)] +#[derive(Debug, Clone, Copy)] +pub struct BytesVt { + retain: Option, + release: Option, +} +impl BytesVt { + const fn is_borrowed(&self) -> bool { + self.release.is_none() + } +} + +impl Clone for Bytes<'_> { + fn clone(&self) -> Self { + if let Some(retain) = &self.vtable.retain { + retain(self.data, self.capacity); + Self { + start: self.start, + len: self.len, + data: self.data, + capacity: self.capacity, + vtable: self.vtable, + } + } else { + Arc::<[u8]>::from(self.as_slice()).into() + } + } +} +impl Drop for Bytes<'_> { + fn drop(&mut self) { + if let Some(release) = &self.vtable.release { + release(self.data, self.capacity) + } + } +} From 5719e4288ea11fbe6461593f182124820baf896a Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 16:52:40 +0200 Subject: [PATCH 2/9] Make `Arc`s optional --- src/bytes.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/bytes.rs b/src/bytes.rs index c3e4714e2b..3ce63d6b5f 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -5,6 +5,7 @@ use core::{ ops::{Deref, Not, RangeBounds}, }; +#[cfg(feature = "alloc")] use alloc::sync::Arc; use safer_ffi_proc_macros::derive_ReprC; @@ -198,6 +199,7 @@ impl<'a> From<&'a [u8]> for Bytes<'a> { } } } +#[cfg(feature = "alloc")] impl From> for Bytes<'static> { fn from(data: Arc<[u8]>) -> Self { extern "C" fn retain(this: *const (), capacity: usize) { @@ -230,6 +232,7 @@ impl From> for Bytes<'static> { } } } +#[cfg(feature = "alloc")] impl + Send + Sync> From> for Bytes<'static> { fn from(value: Arc) -> Self { extern "C" fn retain(this: *const (), _: usize) { From 8722f21f73e85cdfdfdf6d5c3c31ecd6fd2803c4 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 16:57:56 +0200 Subject: [PATCH 3/9] Forgot a pair of cfgs --- src/bytes.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/bytes.rs b/src/bytes.rs index 3ce63d6b5f..bc474d29a6 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -113,6 +113,7 @@ impl<'a> Bytes<'a> { pub const fn as_slice(&self) -> &[u8] { unsafe { core::slice::from_raw_parts(self.start, self.len) } } + #[cfg(feature = "alloc")] /// Proves that the slice can be held onto for arbitrary durations, or copies it into a new one that does. pub fn upgrade(self) -> Bytes<'static> { if !self.vtable.is_borrowed() { @@ -132,7 +133,15 @@ impl<'a> Bytes<'a> { } /// Only calls [`Clone::clone`] if no reallocation would be necessary for it, returning `None` if it would have been. pub fn noalloc_clone(&self) -> Option { - self.clone_will_allocate().not().then(|| self.clone()) + let retain = self.vtable.retain.as_ref()?; + retain(self.data, self.capacity); + Self { + start: self.start, + len: self.len, + data: self.data, + capacity: self.capacity, + vtable: self.vtable, + } } /// Returns `true` if a call to [`Self::upgrade`] would cause an allocation. pub const fn upgrade_will_allocate(&self) -> bool { @@ -270,20 +279,10 @@ impl BytesVt { } } +#[cfg(feature = "alloc")] impl Clone for Bytes<'_> { fn clone(&self) -> Self { - if let Some(retain) = &self.vtable.retain { - retain(self.data, self.capacity); - Self { - start: self.start, - len: self.len, - data: self.data, - capacity: self.capacity, - vtable: self.vtable, - } - } else { - Arc::<[u8]>::from(self.as_slice()).into() - } + self.noalloc_clone().unwrap_or_else(|| Arc::<[u8]>::from(self.as_slice()).into()) } } impl Drop for Bytes<'_> { From c245fd19e41413fab61747c36c030d6542c03de9 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 16:59:27 +0200 Subject: [PATCH 4/9] OK this PR is getting squashed --- src/bytes.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bytes.rs b/src/bytes.rs index bc474d29a6..ccc76b2976 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -135,13 +135,13 @@ impl<'a> Bytes<'a> { pub fn noalloc_clone(&self) -> Option { let retain = self.vtable.retain.as_ref()?; retain(self.data, self.capacity); - Self { + Some(Self { start: self.start, len: self.len, data: self.data, capacity: self.capacity, vtable: self.vtable, - } + }) } /// Returns `true` if a call to [`Self::upgrade`] would cause an allocation. pub const fn upgrade_will_allocate(&self) -> bool { From 05e284a51900d4305534e01df18dfdf71124a611 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 17:01:35 +0200 Subject: [PATCH 5/9] Squashed, I say --- src/bytes.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bytes.rs b/src/bytes.rs index ccc76b2976..84573a676f 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -84,6 +84,7 @@ impl<'a> Bytes<'a> { self.subslice(range); self } + #[cfg(feature = "alloc")] /// Splits the slice at `index`. /// ``` /// let data = b"Hello there".as_slice(); From 19730c9193eb02a988f873153f557908eddc6361 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 17:29:41 +0200 Subject: [PATCH 6/9] Fix doctests and add a bit of fuzzing --- Cargo.lock | 58 +++++++++++++++++++++++++++++++++++++++++-- Cargo.toml | 70 +++++++++++++--------------------------------------- src/bytes.rs | 31 ++++++++++++++++++++--- 3 files changed, 101 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f329c86ab8..5fc35291dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -216,6 +216,17 @@ dependencies = [ "slab", ] +[[package]] +name = "getrandom" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4567c8db10ae91089c99af84c68c38da3ec2f087c3f82960bcdbf3656b6f4d7" +dependencies = [ + "cfg-if", + "libc", + "wasi", +] + [[package]] name = "ghost" version = "0.1.17" @@ -286,9 +297,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.140" +version = "0.2.155" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "99227334921fae1a979cf0bfdfcc6b3e5ce376ef57e16fb6fb3ea2ed6095f80c" +checksum = "97b3888a4aecf77e811145cadf6eef5901f4782c53886191b2f693f24761847c" [[package]] name = "log" @@ -455,6 +466,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "ppv-lite86" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" + [[package]] name = "prettyplease" version = "0.1.24" @@ -483,6 +500,36 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom", +] + [[package]] name = "ref-cast" version = "1.0.15" @@ -549,6 +596,7 @@ dependencies = [ "macro_rules_attribute", "once_cell", "paste", + "rand", "safer-ffi", "safer_ffi-proc_macros", "scopeguard", @@ -756,6 +804,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "wasi" +version = "0.11.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" + [[package]] name = "wasm-bindgen" version = "0.2.84" diff --git a/Cargo.toml b/Cargo.toml index a2832df507..cfc3d60410 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,55 +3,35 @@ path = "src/_lib.rs" [package] name = "safer-ffi" -version = "0.1.8" # Keep in sync -authors = [ - "Daniel Henry-Mantilla ", -] +version = "0.1.8" # Keep in sync +authors = ["Daniel Henry-Mantilla "] edition = "2021" description = "Write safer FFI code in Rust without polluting it with unsafe code" -keywords = [ - "ffi", "no_std", "framework", "safety", "bindings", -] +keywords = ["ffi", "no_std", "framework", "safety", "bindings"] license = "MIT" repository = "https://github.com/getditto/safer_ffi" readme = "README.md" -exclude = [ - "/guide", -] +exclude = ["/guide"] [features] -default = [ - "std", -] +default = ["std"] # Document under the following features: all but for debug or experimental. -docs = [ - "headers", - "default", - "nightly", - "tokio", -] +docs = ["headers", "default", "nightly", "tokio"] nightly = [] alloc = [] -std = [ - "alloc", - "scopeguard/use_std", -] +std = ["alloc", "scopeguard/use_std"] proc_macros = [] # Deprecated -async-fn = [ - "safer_ffi-proc_macros/async-fn", -] +async-fn = ["safer_ffi-proc_macros/async-fn"] -debug_proc_macros = [ - "safer_ffi-proc_macros/verbose-expansions", -] +debug_proc_macros = ["safer_ffi-proc_macros/verbose-expansions"] dyn-traits = [ "safer_ffi-proc_macros/dyn-traits", @@ -59,29 +39,14 @@ dyn-traits = [ "std", ] -futures = [ - "dep:futures", - "dyn-traits", -] +futures = ["dep:futures", "dyn-traits"] -tokio = [ - "async-compat", - "dep:tokio", - "futures", -] +tokio = ["async-compat", "dep:tokio", "futures"] -headers = [ - "inventory", - "safer_ffi-proc_macros/headers", - "std", -] -python-headers = [ - "headers", -] +headers = ["inventory", "safer_ffi-proc_macros/headers", "std"] +python-headers = ["headers"] # Tweak the generated `.h` ever so slightly. -c-headers-with-fn-style = [ - "headers", -] +c-headers-with-fn-style = ["headers"] # PRIVATE FEATURE / not part of crates.io package! js = [ @@ -110,6 +75,7 @@ internal-tests = [ [dev-dependencies] safer-ffi.path = "." safer-ffi.features = ["internal-tests"] +rand = "0.8.5" [dependencies] async-compat.optional = true @@ -139,9 +105,7 @@ scopeguard.default-features = false tokio.optional = true tokio.version = "1.26.0" -tokio.features = [ - "rt", -] +tokio.features = ["rt"] uninit.version = "0.5.0" uninit.default-features = false @@ -159,7 +123,7 @@ version = "0.0.3" [dependencies.safer_ffi-proc_macros] path = "src/proc_macro" -version = "=0.1.8" # Keep in sync +version = "=0.1.8" # Keep in sync [workspace] members = [ diff --git a/src/bytes.rs b/src/bytes.rs index 84573a676f..ab942133a1 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -2,7 +2,7 @@ use core::{ borrow::Borrow, fmt::Debug, hash::Hash, - ops::{Deref, Not, RangeBounds}, + ops::{Deref, RangeBounds}, }; #[cfg(feature = "alloc")] @@ -48,8 +48,9 @@ impl<'a> Bytes<'a> { } /// Slices `self` in-place: /// ``` + /// # use safer_ffi::bytes::Bytes; /// let data = b"Hello there".as_slice(); - /// let bytes = Bytes::from_static(data); + /// let mut bytes = Bytes::from_static(data); /// bytes.subslice(3..7); /// assert_eq!(&data[3..7], bytes.as_slice()); /// ``` @@ -74,6 +75,7 @@ impl<'a> Bytes<'a> { } /// Slices `self` in-place, returning it. /// ``` + /// # use safer_ffi::bytes::Bytes; /// let data = b"Hello there".as_slice(); /// let bytes = Bytes::from_static(data); /// assert_eq!(&data[3..7], bytes.subsliced(3..7).as_slice()); @@ -87,6 +89,7 @@ impl<'a> Bytes<'a> { #[cfg(feature = "alloc")] /// Splits the slice at `index`. /// ``` + /// # use safer_ffi::bytes::Bytes; /// let data = b"Hello there".as_slice(); /// let index = 5; /// let (l, r) = data.split_at(index); @@ -283,7 +286,8 @@ impl BytesVt { #[cfg(feature = "alloc")] impl Clone for Bytes<'_> { fn clone(&self) -> Self { - self.noalloc_clone().unwrap_or_else(|| Arc::<[u8]>::from(self.as_slice()).into()) + self.noalloc_clone() + .unwrap_or_else(|| Arc::<[u8]>::from(self.as_slice()).into()) } } impl Drop for Bytes<'_> { @@ -293,3 +297,24 @@ impl Drop for Bytes<'_> { } } } + +#[cfg(feature = "alloc")] +#[test] +fn fuzz() { + use rand::Rng; + let mut rng = rand::thread_rng(); + for _ in 0..1000 { + let data = (0..rng.gen_range(10..100)) + .map(|_| rng.gen()) + .collect::>(); + let bytes: Bytes<'_> = data.clone().into(); + assert_eq!(bytes.as_slice(), &*data); + for _ in 0..100 { + let start = rng.gen_range(0..data.len()); + let end = rng.gen_range(start..=data.len()); + assert_eq!(bytes.clone().subsliced(start..end), &data[start..end]); + let (l, r) = bytes.clone().split_at(start).unwrap(); + assert_eq!((l.as_slice(), r.as_slice()), data.split_at(start)); + } + } +} From 0422178996380b07447dbd6bfab0e15859f14786 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 19:39:55 +0200 Subject: [PATCH 7/9] Address PR comments --- Cargo.toml | 73 ++++++++++++++++++++++++++++++++----------- src/_lib.rs | 12 ++++---- src/bytes.rs | 87 +++++++++++++++++++++++++++++----------------------- 3 files changed, 109 insertions(+), 63 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cfc3d60410..e3474693be 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,35 +3,55 @@ path = "src/_lib.rs" [package] name = "safer-ffi" -version = "0.1.8" # Keep in sync -authors = ["Daniel Henry-Mantilla "] +version = "0.1.8" # Keep in sync +authors = [ + "Daniel Henry-Mantilla ", +] edition = "2021" description = "Write safer FFI code in Rust without polluting it with unsafe code" -keywords = ["ffi", "no_std", "framework", "safety", "bindings"] +keywords = [ + "ffi", "no_std", "framework", "safety", "bindings", +] license = "MIT" repository = "https://github.com/getditto/safer_ffi" readme = "README.md" -exclude = ["/guide"] +exclude = [ + "/guide", +] [features] -default = ["std"] +default = [ + "std", +] # Document under the following features: all but for debug or experimental. -docs = ["headers", "default", "nightly", "tokio"] +docs = [ + "headers", + "default", + "nightly", + "tokio", +] nightly = [] alloc = [] -std = ["alloc", "scopeguard/use_std"] +std = [ + "alloc", + "scopeguard/use_std", +] proc_macros = [] # Deprecated -async-fn = ["safer_ffi-proc_macros/async-fn"] +async-fn = [ + "safer_ffi-proc_macros/async-fn", +] -debug_proc_macros = ["safer_ffi-proc_macros/verbose-expansions"] +debug_proc_macros = [ + "safer_ffi-proc_macros/verbose-expansions", +] dyn-traits = [ "safer_ffi-proc_macros/dyn-traits", @@ -39,14 +59,29 @@ dyn-traits = [ "std", ] -futures = ["dep:futures", "dyn-traits"] +futures = [ + "dep:futures", + "dyn-traits", +] -tokio = ["async-compat", "dep:tokio", "futures"] +tokio = [ + "async-compat", + "dep:tokio", + "futures", +] -headers = ["inventory", "safer_ffi-proc_macros/headers", "std"] -python-headers = ["headers"] +headers = [ + "inventory", + "safer_ffi-proc_macros/headers", + "std", +] +python-headers = [ + "headers", +] # Tweak the generated `.h` ever so slightly. -c-headers-with-fn-style = ["headers"] +c-headers-with-fn-style = [ + "headers", +] # PRIVATE FEATURE / not part of crates.io package! js = [ @@ -75,7 +110,6 @@ internal-tests = [ [dev-dependencies] safer-ffi.path = "." safer-ffi.features = ["internal-tests"] -rand = "0.8.5" [dependencies] async-compat.optional = true @@ -105,7 +139,9 @@ scopeguard.default-features = false tokio.optional = true tokio.version = "1.26.0" -tokio.features = ["rt"] +tokio.features = [ + "rt", +] uninit.version = "0.5.0" uninit.default-features = false @@ -123,7 +159,8 @@ version = "0.0.3" [dependencies.safer_ffi-proc_macros] path = "src/proc_macro" -version = "=0.1.8" # Keep in sync +version = "=0.1.8" # Keep in sync +rand = "0.8.5" [workspace] members = [ @@ -141,4 +178,4 @@ features = ["docs"] [[test]] name = "ffi-tests" -path = "ffi_tests/src/lib.rs" +path = "ffi_tests/src/lib.rs" \ No newline at end of file diff --git a/src/_lib.rs b/src/_lib.rs index 98cc43c91a..d5457c3d41 100644 --- a/src/_lib.rs +++ b/src/_lib.rs @@ -261,6 +261,9 @@ cfg_alloc! { mod boxed; } +pub +mod bytes; + #[doc(inline)] pub use self::c_char_module::c_char; #[path = "c_char.rs"] @@ -290,6 +293,9 @@ pub use dyn_traits::futures; pub mod libc; +pub +mod option; + pub mod ptr; @@ -307,12 +313,6 @@ use tuple::*; pub mod tuple; -pub -mod option; - -pub -mod bytes; - cfg_alloc! { #[doc(inline)] pub use string::String; diff --git a/src/bytes.rs b/src/bytes.rs index ab942133a1..c459b2e390 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -15,7 +15,7 @@ use safer_ffi_proc_macros::derive_ReprC; #[derive_ReprC] #[repr(C)] pub struct Bytes<'a> { - start: &'a u8, + start: core::ptr::NonNull, len: usize, data: *const (), capacity: usize, @@ -39,7 +39,7 @@ impl<'a> Bytes<'a> { release: Some(noop), }; Self { - start: unsafe { &*data.as_ptr() }, + start: unsafe { core::ptr::NonNull::new_unchecked(data.as_ptr().cast_mut()) }, len: data.len(), data: data.as_ptr().cast(), capacity: data.len(), @@ -51,12 +51,12 @@ impl<'a> Bytes<'a> { /// # use safer_ffi::bytes::Bytes; /// let data = b"Hello there".as_slice(); /// let mut bytes = Bytes::from_static(data); - /// bytes.subslice(3..7); + /// bytes.shrink_to(3..7); /// assert_eq!(&data[3..7], bytes.as_slice()); /// ``` /// # Panics /// If the range's end is out of bounds, or if the range's start is greater than its end. - pub fn subslice>(&mut self, range: R) { + pub fn shrink_to>(&mut self, range: R) { let start = match range.start_bound() { core::ops::Bound::Included(i) => *i, core::ops::Bound::Excluded(i) => *i + 1, @@ -70,10 +70,10 @@ impl<'a> Bytes<'a> { assert!(start <= end); assert!(end <= self.len); let len = end - start; - self.start = unsafe { &*(self.start as *const u8).add(start) }; + self.start = unsafe { core::ptr::NonNull::new_unchecked(self.start.as_ptr().add(start)) }; self.len = len; } - /// Slices `self` in-place, returning it. + /// Convenience around [`Self::shrink_to`] for better method chaining. /// ``` /// # use safer_ffi::bytes::Bytes; /// let data = b"Hello there".as_slice(); @@ -83,7 +83,7 @@ impl<'a> Bytes<'a> { /// # Panics /// If the range's end is out of bounds, or if the range's start is greater than its end. pub fn subsliced>(mut self, range: R) -> Self { - self.subslice(range); + self.shrink_to(range); self } #[cfg(feature = "alloc")] @@ -107,7 +107,8 @@ impl<'a> Bytes<'a> { let mut right = left.clone(); left.len = index; right.len -= index; - right.start = unsafe { &*(self.start as *const u8).add(index) }; + right.start = + unsafe { core::ptr::NonNull::new_unchecked(right.start.as_ptr().add(index)) }; Ok((left, right)) } else { Err(self) @@ -115,11 +116,11 @@ impl<'a> Bytes<'a> { } /// Returns the slice's contents. pub const fn as_slice(&self) -> &[u8] { - unsafe { core::slice::from_raw_parts(self.start, self.len) } + unsafe { core::slice::from_raw_parts(self.start.as_ptr(), self.len) } } #[cfg(feature = "alloc")] /// Proves that the slice can be held onto for arbitrary durations, or copies it into a new one that does. - pub fn upgrade(self) -> Bytes<'static> { + pub fn upgrade(self: Bytes<'a>) -> Bytes<'static> { if !self.vtable.is_borrowed() { return unsafe { core::mem::transmute(self) }; } @@ -128,7 +129,7 @@ impl<'a> Bytes<'a> { /// Attempts to prove that the slice has a static lifetime. /// # Errors /// Returns the original instance if it couldn't be proven to be `'static`. - pub fn noalloc_upgrade(self) -> Result, Self> { + pub fn noalloc_upgrade(self: Bytes<'a>) -> Result, Self> { if !self.vtable.is_borrowed() { Ok(unsafe { core::mem::transmute(self) }) } else { @@ -137,8 +138,8 @@ impl<'a> Bytes<'a> { } /// Only calls [`Clone::clone`] if no reallocation would be necessary for it, returning `None` if it would have been. pub fn noalloc_clone(&self) -> Option { - let retain = self.vtable.retain.as_ref()?; - retain(self.data, self.capacity); + let retain = self.vtable.retain?; + unsafe { retain(self.data, self.capacity) }; Some(Self { start: self.start, len: self.len, @@ -204,7 +205,7 @@ impl<'a> From<&'a [u8]> for Bytes<'a> { retain: Some(noop), }; Bytes { - start: unsafe { &*data.as_ptr() }, + start: core::ptr::NonNull::from(data).cast(), len: data.len(), data: data.as_ptr().cast(), capacity: data.len(), @@ -215,21 +216,17 @@ impl<'a> From<&'a [u8]> for Bytes<'a> { #[cfg(feature = "alloc")] impl From> for Bytes<'static> { fn from(data: Arc<[u8]>) -> Self { - extern "C" fn retain(this: *const (), capacity: usize) { - unsafe { - Arc::increment_strong_count(core::ptr::slice_from_raw_parts( - this.cast::(), - capacity, - )) - } + unsafe extern "C" fn retain(this: *const (), capacity: usize) { + Arc::increment_strong_count(core::ptr::slice_from_raw_parts( + this.cast::(), + capacity, + )) } - extern "C" fn release(this: *const (), capacity: usize) { - unsafe { - Arc::decrement_strong_count(core::ptr::slice_from_raw_parts( - this.cast::(), - capacity, - )) - } + unsafe extern "C" fn release(this: *const (), capacity: usize) { + Arc::decrement_strong_count(core::ptr::slice_from_raw_parts( + this.cast::(), + capacity, + )) } static VT: BytesVt = BytesVt { release: Some(release), @@ -237,7 +234,7 @@ impl From> for Bytes<'static> { }; let capacity = data.len(); Bytes { - start: unsafe { &*data.as_ptr() }, + start: core::ptr::NonNull::<[u8]>::from(data.as_ref()).cast(), len: data.len(), data: Arc::into_raw(data) as *const (), capacity, @@ -246,17 +243,17 @@ impl From> for Bytes<'static> { } } #[cfg(feature = "alloc")] -impl + Send + Sync> From> for Bytes<'static> { +impl<'a, T: Sized + AsRef<[u8]> + Send + Sync + 'a> From> for Bytes<'a> { fn from(value: Arc) -> Self { - extern "C" fn retain(this: *const (), _: usize) { + unsafe extern "C" fn retain(this: *const (), _: usize) { unsafe { Arc::increment_strong_count(this.cast::()) } } - extern "C" fn release(this: *const (), _: usize) { + unsafe extern "C" fn release(this: *const (), _: usize) { unsafe { Arc::decrement_strong_count(this.cast::()) } } let data: &[u8] = value.as_ref().as_ref(); Bytes { - start: unsafe { &*data.as_ptr() }, + start: core::ptr::NonNull::<[u8]>::from(data.as_ref()).cast(), len: data.len(), capacity: data.len(), data: Arc::into_raw(value) as *const (), @@ -267,15 +264,27 @@ impl + Send + Sync> From> for Bytes<'static> { } } } -unsafe impl Send for Bytes<'_> {} -unsafe impl Sync for Bytes<'_> {} +unsafe impl<'a> Send for Bytes<'a> +where + &'a [u8]: Send, + Arc<[u8]>: Send, + Arc + Send + Sync>: Send, +{ +} +unsafe impl<'a> Sync for Bytes<'a> +where + &'a [u8]: Send, + Arc<[u8]>: Send, + Arc + Send + Sync>: Send, +{ +} #[derive_ReprC] #[repr(C)] #[derive(Debug, Clone, Copy)] pub struct BytesVt { - retain: Option, - release: Option, + retain: Option, + release: Option, } impl BytesVt { const fn is_borrowed(&self) -> bool { @@ -292,8 +301,8 @@ impl Clone for Bytes<'_> { } impl Drop for Bytes<'_> { fn drop(&mut self) { - if let Some(release) = &self.vtable.release { - release(self.data, self.capacity) + if let Some(release) = self.vtable.release { + unsafe { release(self.data, self.capacity) } } } } From 7a63bd78ee6c1778c0c0fd04f94bda625437ded7 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 19:43:44 +0200 Subject: [PATCH 8/9] Got trolled by bounds and CI, this whole thing will be squashed anyway --- src/bytes.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/bytes.rs b/src/bytes.rs index c459b2e390..70e98d0136 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -264,6 +264,7 @@ impl<'a, T: Sized + AsRef<[u8]> + Send + Sync + 'a> From> for Bytes<'a> { } } } +#[cfg(feature = "alloc")] unsafe impl<'a> Send for Bytes<'a> where &'a [u8]: Send, @@ -271,6 +272,7 @@ where Arc + Send + Sync>: Send, { } +#[cfg(feature = "alloc")] unsafe impl<'a> Sync for Bytes<'a> where &'a [u8]: Send, @@ -279,6 +281,11 @@ where { } +#[cfg(not(feature = "alloc"))] +unsafe impl<'a> Send for Bytes<'a> where &'a [u8]: Send {} +#[cfg(not(feature = "alloc"))] +unsafe impl<'a> Sync for Bytes<'a> where &'a [u8]: Send {} + #[derive_ReprC] #[repr(C)] #[derive(Debug, Clone, Copy)] From 2a7be39d777bde89eee740b811f5b59b43a8c7b6 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Wed, 29 May 2024 19:51:47 +0200 Subject: [PATCH 9/9] Whoops --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e3474693be..935b4e8797 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -110,6 +110,7 @@ internal-tests = [ [dev-dependencies] safer-ffi.path = "." safer-ffi.features = ["internal-tests"] +rand = "0.8.5" [dependencies] async-compat.optional = true @@ -160,7 +161,6 @@ version = "0.0.3" [dependencies.safer_ffi-proc_macros] path = "src/proc_macro" version = "=0.1.8" # Keep in sync -rand = "0.8.5" [workspace] members = [ @@ -178,4 +178,4 @@ features = ["docs"] [[test]] name = "ffi-tests" -path = "ffi_tests/src/lib.rs" \ No newline at end of file +path = "ffi_tests/src/lib.rs"