From b0db5bd029e14f5107a1f081f30b06cce20153c2 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 20 Aug 2024 06:23:29 -0700 Subject: [PATCH] Change CppInterop traits to operate on *const or *mut c_void instead of protobuf::__runtime::RawMessage PiperOrigin-RevId: 665332958 --- rust/codegen_traits.rs | 30 +++++----- rust/cpp.rs | 6 ++ rust/test/cpp/interop/main.rs | 22 ++++---- src/google/protobuf/compiler/rust/message.cc | 58 ++++++++++---------- 4 files changed, 63 insertions(+), 53 deletions(-) diff --git a/rust/codegen_traits.rs b/rust/codegen_traits.rs index d23b9ecf6d..94972e34f0 100644 --- a/rust/codegen_traits.rs +++ b/rust/codegen_traits.rs @@ -8,7 +8,6 @@ //! Traits that are implemeted by codegen types. use crate::__internal::SealedInternal; -use crate::__runtime::RawMessage; use crate::{MutProxied, MutProxy, ViewProxy}; use create::Parse; use interop::{MessageMutInterop, MessageViewInterop, OwnedMessageInterop}; @@ -114,20 +113,21 @@ pub(crate) mod write { /// These traits are deliberately not available on the prelude, as they should /// be used rarely and with great care. pub(crate) mod interop { - use super::{RawMessage, SealedInternal}; + use super::SealedInternal; + use std::ffi::c_void; /// Traits related to owned message interop. Note that these trait fns /// are only available on C++ kernel as upb interop of owned messages /// requires more work to handle the Arena behavior. pub trait OwnedMessageInterop: SealedInternal { - /// Drops `self` and returns the `RawMessage` that it was wrapping + /// Drops `self` and returns an underlying pointer that it was wrapping /// without deleting it. /// - /// The caller is responsible for ensuring the returned RawMessage is + /// The caller is responsible for ensuring the returned pointer is /// subsequently deleted (eg by moving it into a std::unique_ptr in /// C++), or else it will leak. #[cfg(cpp_kernel)] - fn __unstable_leak_raw_message(self) -> RawMessage; + fn __unstable_leak_raw_message(self) -> *mut c_void; /// Takes exclusive ownership of the `raw_message`. /// @@ -136,49 +136,51 @@ pub(crate) mod interop { /// - The pointer passed in must not be used by the caller after being /// passed here (must not be read, written, or deleted) #[cfg(cpp_kernel)] - unsafe fn __unstable_take_ownership_of_raw_message(raw_message: RawMessage) -> Self; + unsafe fn __unstable_take_ownership_of_raw_message(raw_message: *mut c_void) -> Self; } /// Traits related to message view interop. pub trait MessageViewInterop<'msg>: SealedInternal { - /// Borrows `self` as an underlying `RawMessage`. + /// Borrows `self` as an underlying C++ raw pointer. /// /// Note that the returned Value must be used under the same constraints /// as though it were a borrow of `self`: it should be treated as a /// `const Message*` in C++, and not be mutated in any way, and any /// mutation to the parent message may invalidate it, and it /// must not be deleted. - fn __unstable_as_raw_message(&self) -> RawMessage; + fn __unstable_as_raw_message(&self) -> *const c_void; - /// Wraps the provided `RawMessage` as a MessageView. + /// Wraps the provided pointer as a MessageView. This takes a ref + /// of a pointer so that a stack variable's lifetime can be used + /// to help make the borrow checker safer. /// /// # Safety /// - The underlying message must be for the same type as `Self` /// - The underlying message must be alive for 'msg and not mutated /// while the wrapper is live. - unsafe fn __unstable_wrap_raw_message(raw: &'msg RawMessage) -> Self; + unsafe fn __unstable_wrap_raw_message(raw: &'msg *const c_void) -> Self; } /// Traits related to message mut interop. Note that these trait fns /// are only available on C++ kernel as upb interop of owned messages /// requires more work to handle the Arena behavior. pub trait MessageMutInterop<'msg>: SealedInternal { - /// Exclusive borrows `self` as a `RawMessage`. + /// Exclusive borrows `self` as an underlying mutable C++ raw pointer. /// /// Note that the returned Value must be used under the same constraints /// as though it were a mut borrow of `self`: it should be treated as a /// non-owned `Message*` in C++. And any mutation to the parent message /// may invalidate it, and it must not be deleted. #[cfg(cpp_kernel)] - fn __unstable_as_raw_message_mut(&mut self) -> RawMessage; + fn __unstable_as_raw_message_mut(&mut self) -> *mut c_void; - /// Wraps the provided `RawMessage` as a MessageMut. + /// Wraps the provided C++ pointer as a MessageMut. /// /// # Safety /// - The underlying message must be for the same type as `Self` /// - The underlying message must be alive for 'msg and not read or /// mutated while the wrapper is live. #[cfg(cpp_kernel)] - unsafe fn __unstable_wrap_raw_message_mut(raw: &'msg mut RawMessage) -> Self; + unsafe fn __unstable_wrap_raw_message_mut(raw: &'msg mut *mut c_void) -> Self; } } diff --git a/rust/cpp.rs b/rust/cpp.rs index 766a269aa5..3a6fdad664 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -365,6 +365,12 @@ impl<'msg> MutatorMessageRef<'msg> { MutatorMessageRef { msg: msg.msg, _phantom: PhantomData } } + /// # Safety + /// - The underlying pointer must be sound and live for the lifetime 'msg. + pub unsafe fn wrap_raw(_private: Private, raw: RawMessage) -> Self { + MutatorMessageRef { msg: raw, _phantom: PhantomData } + } + pub fn from_parent( _private: Private, _parent_msg: MutatorMessageRef<'msg>, diff --git a/rust/test/cpp/interop/main.rs b/rust/test/cpp/interop/main.rs index 398d94b8a0..e30df483a2 100644 --- a/rust/test/cpp/interop/main.rs +++ b/rust/test/cpp/interop/main.rs @@ -8,8 +8,9 @@ use googletest::prelude::*; use protobuf_cpp::prelude::*; -use protobuf_cpp::__runtime::{PtrAndLen, RawMessage}; +use protobuf_cpp::__runtime::PtrAndLen; use protobuf_cpp::{MessageMutInterop, MessageViewInterop, OwnedMessageInterop}; +use std::ffi::c_void; use unittest_rust_proto::{TestAllExtensions, TestAllTypes, TestAllTypesMut, TestAllTypesView}; macro_rules! proto_assert_eq { @@ -26,14 +27,14 @@ macro_rules! proto_assert_eq { // Helper functions invoking C++ Protobuf APIs directly in C++. // Defined in `test_utils.cc`. extern "C" { - fn TakeOwnershipAndGetOptionalInt32(msg: RawMessage) -> i32; - fn DeserializeTestAllTypes(data: *const u8, len: usize) -> RawMessage; - fn MutateTestAllTypes(msg: RawMessage); - fn SerializeTestAllTypes(msg: RawMessage) -> protobuf_cpp::__runtime::SerializedData; - fn DeleteTestAllTypes(msg: RawMessage); - - fn NewWithExtension() -> RawMessage; - fn GetBytesExtension(msg: RawMessage) -> PtrAndLen; + fn TakeOwnershipAndGetOptionalInt32(msg: *mut c_void) -> i32; + fn DeserializeTestAllTypes(data: *const u8, len: usize) -> *mut c_void; + fn MutateTestAllTypes(msg: *mut c_void); + fn SerializeTestAllTypes(msg: *const c_void) -> protobuf_cpp::__runtime::SerializedData; + fn DeleteTestAllTypes(msg: *mut c_void); + + fn NewWithExtension() -> *mut c_void; + fn GetBytesExtension(msg: *const c_void) -> PtrAndLen; } #[gtest] @@ -113,7 +114,8 @@ fn deserialize_in_cpp_into_view() { let data = msg1.serialize().unwrap(); let raw_msg = unsafe { DeserializeTestAllTypes((*data).as_ptr(), data.len()) }; - let msg2 = unsafe { TestAllTypesView::__unstable_wrap_raw_message(&raw_msg) }; + let const_msg = raw_msg as *const _; + let msg2 = unsafe { TestAllTypesView::__unstable_wrap_raw_message(&const_msg) }; proto_assert_eq!(msg1, msg2); diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc index 9def99cad6..d2dcb534ff 100644 --- a/src/google/protobuf/compiler/rust/message.cc +++ b/src/google/protobuf/compiler/rust/message.cc @@ -1315,61 +1315,61 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { if (ctx.is_cpp()) { ctx.Emit({{"Msg", RsSafeName(msg.name())}}, R"rs( impl<'a> $Msg$Mut<'a> { - //~ msg is a &mut so that the borrow checker enforces exclusivity to - //~ prevent constructing multiple Muts/Views from the same RawMessage. - pub fn __unstable_wrap_cpp_grant_permission_to_break( - msg: &'a mut $pbr$::RawMessage) -> Self { + pub unsafe fn __unstable_wrap_cpp_grant_permission_to_break( + msg: &'a mut *mut std::ffi::c_void) -> Self { Self { - inner: $pbr$::MutatorMessageRef::from_raw_msg($pbi$::Private, msg) + inner: $pbr$::MutatorMessageRef::wrap_raw( + $pbi$::Private, + $pbr$::RawMessage::new(*msg as *mut _).unwrap()) } } - pub fn __unstable_cpp_repr_grant_permission_to_break(self) -> $pbr$::RawMessage { - self.raw_msg() + pub fn __unstable_cpp_repr_grant_permission_to_break(self) -> *mut std::ffi::c_void { + self.raw_msg().as_ptr() as *mut _ } } impl<'a> $Msg$View<'a> { - //~ msg is a & so that the caller can claim the message is live for the - //~ corresponding lifetime. pub fn __unstable_wrap_cpp_grant_permission_to_break( - msg: &'a $pbr$::RawMessage) -> Self { - Self::new($pbi$::Private, *msg) + msg: &'a *const std::ffi::c_void) -> Self { + Self::new($pbi$::Private, $pbr$::RawMessage::new(*msg as *mut _).unwrap()) } - pub fn __unstable_cpp_repr_grant_permission_to_break(self) -> $pbr$::RawMessage { - self.msg + pub fn __unstable_cpp_repr_grant_permission_to_break(self) -> *const std::ffi::c_void { + self.msg.as_ptr() as *const _ } } impl $pb$::OwnedMessageInterop for $Msg$ { - unsafe fn __unstable_take_ownership_of_raw_message(msg: $pbr$::RawMessage) -> Self { - Self { inner: $pbr$::MessageInner { msg } } + unsafe fn __unstable_take_ownership_of_raw_message(msg: *mut std::ffi::c_void) -> Self { + Self { inner: $pbr$::MessageInner { msg: $pbr$::RawMessage::new(msg as *mut _).unwrap() } } } - fn __unstable_leak_raw_message(self) -> $pbr$::RawMessage { + fn __unstable_leak_raw_message(self) -> *mut std::ffi::c_void { let s = std::mem::ManuallyDrop::new(self); - s.raw_msg() + s.raw_msg().as_ptr() as *mut _ } } impl<'a> $pb$::MessageMutInterop<'a> for $Msg$Mut<'a> { unsafe fn __unstable_wrap_raw_message_mut( - msg: &'a mut $pbr$::RawMessage) -> Self { + msg: &'a mut *mut std::ffi::c_void) -> Self { Self { - inner: $pbr$::MutatorMessageRef::from_raw_msg($pbi$::Private, msg) + inner: $pbr$::MutatorMessageRef::wrap_raw( + $pbi$::Private, + $pbr$::RawMessage::new(*msg as *mut _).unwrap()) } } - fn __unstable_as_raw_message_mut(&mut self) -> $pbr$::RawMessage { - self.raw_msg() + fn __unstable_as_raw_message_mut(&mut self) -> *mut std::ffi::c_void { + self.raw_msg().as_ptr() as *mut _ } } impl<'a> $pb$::MessageViewInterop<'a> for $Msg$View<'a> { unsafe fn __unstable_wrap_raw_message( - msg: &'a $pbr$::RawMessage) -> Self { - Self::new($pbi$::Private, *msg) + msg: &'a *const std::ffi::c_void) -> Self { + Self::new($pbi$::Private, $pbr$::RawMessage::new(*msg as *mut _).unwrap()) } - fn __unstable_as_raw_message(&self) -> $pbr$::RawMessage { - self.msg + fn __unstable_as_raw_message(&self) -> *const std::ffi::c_void { + self.msg.as_ptr() as *const _ } } )rs"); @@ -1381,11 +1381,11 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { impl<'a> $pb$::MessageViewInterop<'a> for $Msg$View<'a> { unsafe fn __unstable_wrap_raw_message( - msg: &'a $pbr$::RawMessage) -> Self { - Self::new($pbi$::Private, *msg) + msg: &'a *const std::ffi::c_void) -> Self { + Self::new($pbi$::Private, $pbr$::RawMessage::new(*msg as *mut _).unwrap()) } - fn __unstable_as_raw_message(&self) -> $pbr$::RawMessage { - self.msg + fn __unstable_as_raw_message(&self) -> *const std::ffi::c_void { + self.msg.as_ptr() as *const _ } } )rs");