Replace `NonNull<u8>` for raw messages with a dedicated opaque pointer These are more type safe, and more clearly distinguish between a raw message and serialized data. This also defines a macro to create new opaque pointer types, and switches `RawArena` to using it. PiperOrigin-RevId: 552957136
diff --git a/rust/cpp.rs b/rust/cpp.rs index 9b683bb..46cead7 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs
@@ -30,6 +30,7 @@ // Rust Protobuf runtime using the C++ kernel. +use crate::__internal::RawArena; use std::alloc::Layout; use std::cell::UnsafeCell; use std::fmt; @@ -50,7 +51,7 @@ #[derive(Debug)] pub struct Arena { #[allow(dead_code)] - ptr: NonNull<u8>, + ptr: RawArena, _not_sync: PhantomData<UnsafeCell<()>>, }
diff --git a/rust/internal.rs b/rust/internal.rs index ada1025..7423a3c 100644 --- a/rust/internal.rs +++ b/rust/internal.rs
@@ -37,6 +37,54 @@ /// Used to protect internal-only items from being used accidentally. pub struct Private; +/// Defines a set of opaque pointers and a unique non-accessible pointees. +/// +/// This provides a type safety benefit over using `NonNull<u8>` everywhere. +/// The [Rustonomicon][nomicon] currently recommends a zero-sized struct, +/// though this should use [`extern type`] when that is stabilized. +/// +/// Because this defines a new private module, it can only be called once per +/// module. +/// +/// [nomicon]: https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs +/// [`extern type`]: https://github.com/rust-lang/rust/issues/43467 +macro_rules! define_opaque_nonnulls { + ($($(#[$meta:meta])* $vis:vis type $name:ident = NonNull<$raw_name:ident>;)*) => { + mod _opaque_pointees { + $( + #[doc = concat!("Opaque pointee for [`", stringify!($name), "`][pointer]")] + /// + /// This type is not meant to be dereferenced in Rust code. + /// It is only meant to provide type safety for raw pointers + /// which are manipulated behind FFI. + #[doc = concat!("[pointer]: super::", stringify!($name))] + #[repr(C)] + pub struct $raw_name { + _data: [u8; 0], + _marker: ::std::marker::PhantomData<(*mut u8, ::std::marker::PhantomPinned)>, + } + )* + } + $( + $(#[$meta])* + /// + /// This is an opaque pointer used for FFI: + /// do not dereference this type in Rust code. + $vis type $name = ::std::ptr::NonNull<_opaque_pointees::$raw_name>; + )* + }; +} + +pub(crate) use define_opaque_nonnulls; + +define_opaque_nonnulls!( + /// A raw pointer to the underlying arena for this runtime. + pub type RawArena = NonNull<RawArenaData>; + + /// A raw pointer to the underlying message for this runtime. + pub type RawMessage = NonNull<RawMessageData>; +); + /// Represents an ABI-stable version of `NonNull<[u8]>`/`string_view` (a /// borrowed slice of bytes) for FFI use only. ///
diff --git a/rust/test/cpp/interop/main.rs b/rust/test/cpp/interop/main.rs index 8821fb7..3158430 100644 --- a/rust/test/cpp/interop/main.rs +++ b/rust/test/cpp/interop/main.rs
@@ -28,7 +28,8 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::ptr::NonNull; +use protobuf_cpp::__internal::PtrAndLen; +use protobuf_cpp::__internal::RawMessage; use unittest_proto::proto2_unittest::TestAllExtensions; use unittest_proto::proto2_unittest::TestAllTypes; @@ -46,12 +47,12 @@ // Helper functions invoking C++ Protobuf APIs directly in C++. // Defined in `test_utils.cc`. extern "C" { - fn DeserializeTestAllTypes(data: *const u8, len: usize) -> NonNull<u8>; - fn MutateTestAllTypes(msg: NonNull<u8>); - fn SerializeTestAllTypes(msg: NonNull<u8>) -> protobuf_cpp::__runtime::SerializedData; + fn DeserializeTestAllTypes(data: *const u8, len: usize) -> RawMessage; + fn MutateTestAllTypes(msg: RawMessage); + fn SerializeTestAllTypes(msg: RawMessage) -> protobuf_cpp::__runtime::SerializedData; - fn NewWithExtension() -> NonNull<u8>; - fn GetBytesExtension(msg: NonNull<u8>) -> protobuf_cpp::__internal::PtrAndLen; + fn NewWithExtension() -> RawMessage; + fn GetBytesExtension(msg: RawMessage) -> PtrAndLen; } #[test]
diff --git a/rust/upb.rs b/rust/upb.rs index 204d3db..a887052 100644 --- a/rust/upb.rs +++ b/rust/upb.rs
@@ -30,6 +30,7 @@ //! UPB FFI wrapper code for use by Rust Protobuf. +use crate::__internal::RawArena; use std::alloc; use std::alloc::Layout; use std::cell::UnsafeCell; @@ -43,15 +44,6 @@ /// See `upb/port/def.inc`. const UPB_MALLOC_ALIGN: usize = 8; -/// A UPB-managed pointer to a raw arena. -pub type RawArena = NonNull<RawArenaData>; - -/// The data behind a [`RawArena`]. Do not use this type. -#[repr(C)] -pub struct RawArenaData { - _data: [u8; 0], -} - /// A wrapper over a `upb_Arena`. /// /// This is not a safe wrapper per se, because the allocation functions still
diff --git a/src/google/protobuf/compiler/rust/accessors/singular_bytes.cc b/src/google/protobuf/compiler/rust/accessors/singular_bytes.cc index b9e0a89..831fec1 100644 --- a/src/google/protobuf/compiler/rust/accessors/singular_bytes.cc +++ b/src/google/protobuf/compiler/rust/accessors/singular_bytes.cc
@@ -87,10 +87,10 @@ {"clearer_thunk", Thunk(field, "clear")}, }, R"rs( - fn $hazzer_thunk$(raw_msg: $NonNull$<u8>) -> bool; - fn $getter_thunk$(raw_msg: $NonNull$<u8>) -> $pbi$::PtrAndLen; - fn $setter_thunk$(raw_msg: $NonNull$<u8>, val: *const u8, len: usize); - fn $clearer_thunk$(raw_msg: $NonNull$<u8>); + fn $hazzer_thunk$(raw_msg: $pbi$::RawMessage) -> bool; + fn $getter_thunk$(raw_msg: $pbi$::RawMessage) -> $pbi$::PtrAndLen; + fn $setter_thunk$(raw_msg: $pbi$::RawMessage, val: *const u8, len: usize); + fn $clearer_thunk$(raw_msg: $pbi$::RawMessage); )rs"); }
diff --git a/src/google/protobuf/compiler/rust/accessors/singular_scalar.cc b/src/google/protobuf/compiler/rust/accessors/singular_scalar.cc index 8053f21..2db381f 100644 --- a/src/google/protobuf/compiler/rust/accessors/singular_scalar.cc +++ b/src/google/protobuf/compiler/rust/accessors/singular_scalar.cc
@@ -99,10 +99,10 @@ {"clearer_thunk", Thunk(field, "clear")}, }, R"rs( - fn $hazzer_thunk$(raw_msg: $NonNull$<u8>) -> bool; - fn $getter_thunk$(raw_msg: $NonNull$<u8>) -> $Scalar$; - fn $setter_thunk$(raw_msg: $NonNull$<u8>, val: $Scalar$); - fn $clearer_thunk$(raw_msg: $NonNull$<u8>); + fn $hazzer_thunk$(raw_msg: $pbi$::RawMessage) -> bool; + fn $getter_thunk$(raw_msg: $pbi$::RawMessage) -> $Scalar$; + fn $setter_thunk$(raw_msg: $pbi$::RawMessage, val: $Scalar$); + fn $clearer_thunk$(raw_msg: $pbi$::RawMessage); )rs"); }
diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc index 6468f4b..24bbd88 100644 --- a/src/google/protobuf/compiler/rust/message.cc +++ b/src/google/protobuf/compiler/rust/message.cc
@@ -49,13 +49,13 @@ switch (msg.opts().kernel) { case Kernel::kCpp: msg.Emit(R"rs( - msg: $NonNull$<u8>, + msg: $pbi$::RawMessage, )rs"); return; case Kernel::kUpb: msg.Emit(R"rs( - msg: $NonNull$<u8>, + msg: $pbi$::RawMessage, //~ rustc incorrectly thinks this field is never read, even though //~ it has a destructor! #[allow(dead_code)] @@ -136,9 +136,7 @@ msg.Emit({{"deserialize_thunk", Thunk(msg, "parse")}}, R"rs( let arena = $pbr$::Arena::new(); let msg = unsafe { - $NonNull$::<u8>::new( - $deserialize_thunk$(data.as_ptr(), data.len(), arena.raw()) - ) + $deserialize_thunk$(data.as_ptr(), data.len(), arena.raw()) }; match msg { @@ -169,10 +167,10 @@ {"deserialize_thunk", Thunk(msg, "deserialize")}, }, R"rs( - fn $new_thunk$() -> $NonNull$<u8>; - fn $delete_thunk$(raw_msg: $NonNull$<u8>); - fn $serialize_thunk$(raw_msg: $NonNull$<u8>) -> $pbr$::SerializedData; - fn $deserialize_thunk$(raw_msg: $NonNull$<u8>, data: $pbr$::SerializedData) -> bool; + fn $new_thunk$() -> $pbi$::RawMessage; + fn $delete_thunk$(raw_msg: $pbi$::RawMessage); + fn $serialize_thunk$(raw_msg: $pbi$::RawMessage) -> $pbr$::SerializedData; + fn $deserialize_thunk$(raw_msg: $pbi$::RawMessage, data: $pbr$::SerializedData) -> bool; )rs"); return; @@ -184,9 +182,9 @@ {"deserialize_thunk", Thunk(msg, "parse")}, }, R"rs( - fn $new_thunk$(arena: $pbr$::RawArena) -> $NonNull$<u8>; - fn $serialize_thunk$(msg: $NonNull$<u8>, arena: $pbr$::RawArena, len: &mut usize) -> $NonNull$<u8>; - fn $deserialize_thunk$(data: *const u8, size: usize, arena: $pbr$::RawArena) -> *mut u8; + fn $new_thunk$(arena: $pbi$::RawArena) -> $pbi$::RawMessage; + fn $serialize_thunk$(msg: $pbi$::RawMessage, arena: $pbi$::RawArena, len: &mut usize) -> $NonNull$<u8>; + fn $deserialize_thunk$(data: *const u8, size: usize, arena: $pbi$::RawArena) -> Option<$pbi$::RawMessage>; )rs"); return; } @@ -304,7 +302,7 @@ #[derive(Debug, Copy, Clone)] pub struct $Msg$View<'a> { - msg: $NonNull$<u8>, + _msg: $pbi$::RawMessage, _phantom: std::marker::PhantomData<&'a ()>, } @@ -379,10 +377,10 @@ msg.printer().PrintRaw("\n"); msg.Emit({{"Msg", msg.desc().name()}}, R"rs( impl $Msg$ { - pub fn __unstable_wrap_cpp_grant_permission_to_break(msg: $NonNull$<u8>) -> Self { + pub fn __unstable_wrap_cpp_grant_permission_to_break(msg: $pbi$::RawMessage) -> Self { Self { msg } } - pub fn __unstable_cpp_repr_grant_permission_to_break(&mut self) -> $NonNull$<u8> { + pub fn __unstable_cpp_repr_grant_permission_to_break(&mut self) -> $pbi$::RawMessage { self.msg } }