diff --git a/rust/cpp_kernel/cpp.rs b/rust/cpp_kernel/cpp.rs index 65b51c9b40..ab315516d3 100644 --- a/rust/cpp_kernel/cpp.rs +++ b/rust/cpp_kernel/cpp.rs @@ -30,27 +30,73 @@ // Rust Protobuf runtime using the C++ kernel. -use std::alloc::{dealloc, Layout}; +use std::alloc; +use std::alloc::Layout; use std::boxed::Box; +use std::cell::UnsafeCell; +use std::fmt; +use std::marker::PhantomData; +use std::mem::MaybeUninit; use std::ops::Deref; use std::ptr::NonNull; use std::slice; +/// A wrapper over a `proto2::Arena`. +/// +/// This is not a safe wrapper per se, because the allocation functions still +/// have sharp edges (see their safety docs for more info). +/// +/// This is an owning type and will automatically free the arena when +/// dropped. +/// +/// Note that this type is neither `Sync` nor `Send`. +/// /// TODO(b/272728844): Replace this placeholder code with a real implementation. -#[repr(C)] pub struct Arena { - _data: [u8; 0], + ptr: NonNull, + _not_sync: PhantomData>, } impl Arena { - pub unsafe fn new() -> *mut Self { - let arena = Box::new(Arena { _data: [] }); - Box::leak(arena) as *mut _ + /// Allocates a fresh arena. + #[inline] + pub fn new() -> Self { + Self { ptr: NonNull::dangling(), _not_sync: PhantomData } + } + + /// Returns the raw, C++-managed pointer to the arena. + #[inline] + pub fn raw(&self) -> ! { + unimplemented!() + } + + /// Allocates some memory on the arena. + /// + /// # Safety + /// + /// `layout`'s alignment must be less than `UPB_MALLOC_ALIGN`. + #[inline] + pub unsafe fn alloc(&self, layout: Layout) -> &mut [MaybeUninit] { + unimplemented!() + } + + /// Resizes some memory on the arena. + /// + /// # Safety + /// + /// After calling this function, `ptr` is essentially zapped. `old` must + /// be the layout `ptr` was allocated with via [`Arena::alloc()`]. `new`'s + /// alignment must be less than `UPB_MALLOC_ALIGN`. + #[inline] + pub unsafe fn resize(&self, ptr: *mut u8, old: Layout, new: Layout) -> &[MaybeUninit] { + unimplemented!() } +} - pub unsafe fn free(arena: *mut Self) { - let arena = Box::from_raw(arena); - std::mem::drop(arena); +impl Drop for Arena { + #[inline] + fn drop(&mut self) { + // unimplemented } } @@ -63,7 +109,6 @@ impl Arena { // LINT.IfChange // copybara:strip_end #[repr(C)] -#[derive(Debug)] pub struct SerializedData { /// Owns the memory. data: NonNull, @@ -89,11 +134,17 @@ impl Deref for SerializedData { impl Drop for SerializedData { fn drop(&mut self) { unsafe { - dealloc(self.data.as_ptr(), Layout::array::(self.len).unwrap()); + alloc::dealloc(self.data.as_ptr(), Layout::array::(self.len).unwrap()); }; } } +impl fmt::Debug for SerializedData { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Debug::fmt(self.deref(), f) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/rust/protobuf.rs b/rust/protobuf.rs index 99653752b7..3569b1c6ab 100644 --- a/rust/protobuf.rs +++ b/rust/protobuf.rs @@ -35,12 +35,7 @@ //! this crate exists is to be able to use `protobuf` as a crate name for both //! cpp and upb kernels from user code. -#[cfg(cpp_kernel)] -pub use protobuf_cpp::__runtime; #[cfg(cpp_kernel)] pub use protobuf_cpp::*; - -#[cfg(upb_kernel)] -pub use protobuf_upb::__runtime; #[cfg(upb_kernel)] pub use protobuf_upb::*; diff --git a/rust/shared.rs b/rust/shared.rs index ed69760fdb..3ee25fd04d 100644 --- a/rust/shared.rs +++ b/rust/shared.rs @@ -38,7 +38,6 @@ pub extern crate cpp as __runtime; #[cfg(upb_kernel)] pub extern crate upb as __runtime; -pub use __runtime::Arena; pub use __runtime::SerializedData; use std::fmt; diff --git a/rust/upb_kernel/upb.rs b/rust/upb_kernel/upb.rs index 9f4bb90409..f19e4dd154 100644 --- a/rust/upb_kernel/upb.rs +++ b/rust/upb_kernel/upb.rs @@ -28,46 +28,122 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -// Rust Protobuf runtime using the UPB kernel. +//! UPB FFI wrapper code for use by Rust Protobuf. -/// Represents UPB's upb_Arena. +use std::alloc; +use std::alloc::Layout; +use std::cell::UnsafeCell; +use std::fmt; +use std::marker::PhantomData; +use std::mem::MaybeUninit; use std::ops::Deref; use std::ptr::NonNull; use std::slice; +/// See `upb/port/def.inc`. +const UPB_MALLOC_ALIGN: usize = 8; + +/// A UPB-managed pointer to a raw arena. +pub type RawArena = NonNull; + +/// The data behind a [`RawArena`]. Do not use this type. #[repr(C)] -pub struct Arena { +pub struct RawArenaData { _data: [u8; 0], - _marker: core::marker::PhantomData<(*mut u8, core::marker::PhantomPinned)>, +} + +/// A wrapper over a `upb_Arena`. +/// +/// This is not a safe wrapper per se, because the allocation functions still +/// have sharp edges (see their safety docs for more info). +/// +/// This is an owning type and will automatically free the arena when +/// dropped. +/// +/// Note that this type is neither `Sync` nor `Send`. +pub struct Arena { + raw: RawArena, + _not_sync: PhantomData>, +} + +extern "C" { + fn upb_Arena_New() -> RawArena; + fn upb_Arena_Free(arena: RawArena); + fn upb_Arena_Malloc(arena: RawArena, size: usize) -> *mut u8; + fn upb_Arena_Realloc(arena: RawArena, ptr: *mut u8, old: usize, new: usize) -> *mut u8; } impl Arena { - pub unsafe fn new() -> *mut Self { - upb_Arena_New() + /// Allocates a fresh arena. + #[inline] + pub fn new() -> Self { + Self { raw: unsafe { upb_Arena_New() }, _not_sync: PhantomData } + } + + /// Returns the raw, UPB-managed pointer to the arena. + #[inline] + pub fn raw(&self) -> RawArena { + self.raw } - pub unsafe fn free(arena: *mut Self) { - upb_Arena_Free(arena) + /// Allocates some memory on the arena. + /// + /// # Safety + /// + /// `layout`'s alignment must be less than `UPB_MALLOC_ALIGN`. + #[inline] + pub unsafe fn alloc(&self, layout: Layout) -> &mut [MaybeUninit] { + debug_assert!(layout.align() <= UPB_MALLOC_ALIGN); + let ptr = upb_Arena_Malloc(self.raw, layout.size()); + if ptr.is_null() { + alloc::handle_alloc_error(layout); + } + + slice::from_raw_parts_mut(ptr.cast(), layout.size()) + } + + /// Resizes some memory on the arena. + /// + /// # Safety + /// + /// After calling this function, `ptr` is essentially zapped. `old` must + /// be the layout `ptr` was allocated with via [`Arena::alloc()`]. `new`'s + /// alignment must be less than `UPB_MALLOC_ALIGN`. + #[inline] + pub unsafe fn resize(&self, ptr: *mut u8, old: Layout, new: Layout) -> &[MaybeUninit] { + debug_assert!(new.align() <= UPB_MALLOC_ALIGN); + let ptr = upb_Arena_Realloc(self.raw, ptr, old.size(), new.size()); + if ptr.is_null() { + alloc::handle_alloc_error(new); + } + + slice::from_raw_parts_mut(ptr.cast(), new.size()) } } -extern "C" { - pub fn upb_Arena_New() -> *mut Arena; - pub fn upb_Arena_Free(arena: *mut Arena); +impl Drop for Arena { + #[inline] + fn drop(&mut self) { + unsafe { + upb_Arena_Free(self.raw); + } + } } -/// Represents serialized Protobuf wire format data. It's typically produced by -/// `.serialize()`. -#[derive(Debug)] +/// Represents serialized Protobuf wire format data. +/// +/// It's typically produced by `::serialize()`. pub struct SerializedData { data: NonNull, len: usize, - arena: *mut Arena, + + // The arena that owns `data`. + _arena: Arena, } impl SerializedData { - pub unsafe fn from_raw_parts(arena: *mut Arena, data: NonNull, len: usize) -> Self { - SerializedData { arena, data, len } + pub unsafe fn from_raw_parts(arena: Arena, data: NonNull, len: usize) -> Self { + SerializedData { _arena: arena, data, len } } } @@ -78,9 +154,9 @@ impl Deref for SerializedData { } } -impl Drop for SerializedData { - fn drop(&mut self) { - unsafe { Arena::free(self.arena) }; +impl fmt::Debug for SerializedData { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Debug::fmt(self.deref(), f) } } @@ -90,13 +166,13 @@ mod tests { #[test] fn test_arena_new_and_free() { - let arena = unsafe { Arena::new() }; - unsafe { Arena::free(arena) }; + let arena = Arena::new(); + drop(arena); } #[test] fn test_serialized_data_roundtrip() { - let arena = unsafe { Arena::new() }; + let arena = Arena::new(); let original_data = b"Hello world"; let len = original_data.len(); diff --git a/src/google/protobuf/compiler/rust/generator.cc b/src/google/protobuf/compiler/rust/generator.cc index 9b4071fa30..33946f5b34 100644 --- a/src/google/protobuf/compiler/rust/generator.cc +++ b/src/google/protobuf/compiler/rust/generator.cc @@ -97,6 +97,7 @@ bool RustGenerator::Generate(const FileDescriptor* file_desc, auto v = file.printer().WithVars({ {"std", "::__std"}, {"pb", "::__pb"}, + {"pbi", "::__pb::__runtime"}, {"NonNull", "::__std::ptr::NonNull"}, }); diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc index 99a33c1d35..8a4627a816 100644 --- a/src/google/protobuf/compiler/rust/message.cc +++ b/src/google/protobuf/compiler/rust/message.cc @@ -56,7 +56,10 @@ void MessageStructFields(Context msg) { case Kernel::kUpb: msg.Emit(R"rs( msg: $NonNull$, - arena: *mut $pb$::Arena, + //~ rustc incorrectly thinks this field is never read, even though + //~ it has a destructor! + #[allow(dead_code)] + arena: $pbi$::Arena, )rs"); return; } @@ -74,9 +77,11 @@ void MessageNew(Context msg) { case Kernel::kUpb: msg.Emit({{"new_thunk", Thunk(msg, "new")}}, R"rs( - let arena = unsafe { $pb$::Arena::new() }; - let msg = unsafe { $new_thunk$(arena) }; - $Msg$ { msg, arena } + let arena = unsafe { $pbi$::Arena::new() }; + Self { + msg: unsafe { $new_thunk$(arena.raw()) }, + arena, + } )rs"); return; } @@ -94,10 +99,10 @@ void MessageSerialize(Context msg) { case Kernel::kUpb: msg.Emit({{"serialize_thunk", Thunk(msg, "serialize")}}, R"rs( - let arena = unsafe { $pb$::__runtime::upb_Arena_New() }; + let arena = $pbi$::Arena::new(); let mut len = 0; unsafe { - let data = $serialize_thunk$(self.msg, arena, &mut len); + let data = $serialize_thunk$(self.msg, arena.raw(), &mut len); $pb$::SerializedData::from_raw_parts(arena, data, len) } )rs"); @@ -152,7 +157,7 @@ void MessageExterns(Context msg) { fn $new_thunk$() -> $NonNull$; fn $delete_thunk$(raw_msg: $NonNull$); fn $serialize_thunk$(raw_msg: $NonNull$) -> $pb$::SerializedData; - fn $deserialize_thunk$( raw_msg: $NonNull$, data: $pb$::SerializedData) -> bool; + fn $deserialize_thunk$(raw_msg: $NonNull$, data: $pb$::SerializedData) -> bool; )rs"); return; @@ -163,13 +168,9 @@ void MessageExterns(Context msg) { {"serialize_thunk", Thunk(msg, "serialize")}, }, R"rs( - fn $new_thunk$(arena: *mut $pb$::Arena) -> $NonNull$; - fn $serialize_thunk$( - msg: $NonNull$, - arena: *mut $pb$::Arena, - len: &mut usize, - ) -> $NonNull$; - )rs"); + fn $new_thunk$(arena: $pbi$::RawArena) -> $NonNull$; + fn $serialize_thunk$(msg: $NonNull$, arena: $pbi$::RawArena, len: &mut usize) -> $NonNull$; + )rs"); return; } @@ -347,12 +348,12 @@ void MessageGenerator::GenerateThunksCc(Context msg) { }}, }, R"cc( - // $abi$ is a workaround for a syntax highlight bug in VSCode. However, - // that confuses clang-format (it refuses to keep the newline after - // `$abi${`). Disabling clang-format for the block. + //~ $abi$ is a workaround for a syntax highlight bug in VSCode. However, + //~ that confuses clang-format (it refuses to keep the newline after + //~ `$abi${`). Disabling clang-format for the block. // clang-format off extern $abi$ { - void * $new_thunk$(){return new $QualifiedMsg$(); } + void* $new_thunk$(){return new $QualifiedMsg$(); } void $delete_thunk$(void* ptr) { delete static_cast<$QualifiedMsg$*>(ptr); } google::protobuf::rust_internal::SerializedData $serialize_thunk$($QualifiedMsg$* msg) { return google::protobuf::rust_internal::SerializeMsg(msg);