From db3e837e02139f55d82dd521a53909420f648340 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 22 Aug 2024 10:12:22 -0700 Subject: [PATCH] Add an `_unchecked_lifetime` version of the Cpp interop wrap fns. This is useful/necessary when the caller wants to tie the View to a lifetime other than a stack frame, including 'static. The documentation is clarified that the preexisting one should be preferred in most situations, and only use the new one where tying the View lifetime to a stack frame isn't suitable. PiperOrigin-RevId: 666396427 --- rust/codegen_traits.rs | 60 +++++++++++++++++++- rust/test/cpp/interop/main.rs | 13 +++++ rust/test/cpp/interop/test_utils.cc | 5 ++ src/google/protobuf/compiler/rust/message.cc | 16 ++++++ 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/rust/codegen_traits.rs b/rust/codegen_traits.rs index 94972e34f..6d91c19bd 100644 --- a/rust/codegen_traits.rs +++ b/rust/codegen_traits.rs @@ -150,15 +150,41 @@ pub(crate) mod interop { /// must not be deleted. fn __unstable_as_raw_message(&self) -> *const c_void; - /// 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. + /// Wraps the provided pointer as a MessageView. + /// + /// This takes a ref of a pointer so that a stack variable's lifetime + /// can be used for a safe lifetime; under most cases this is + /// the correct lifetime and this should be used as: + /// ``` + /// fn called_from_cpp(msg: *const c_void) { + /// // `msg` is known live for the current stack frame, so view's + /// // lifetime is also tied to the current stack frame here: + /// let view = unsafe { __unstable_wrap_raw_message(&msg); } + /// do_something_with_view(view); + /// } + /// ``` /// /// # 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 *const c_void) -> Self; + + /// Wraps the provided pointer as a MessageView. + /// + /// Unlike `__unstable_wrap_raw_message` this has no constraints + /// on lifetime: the caller has a free choice for the lifetime. + /// + /// As this is much easier to get the lifetime wrong than + /// `__unstable_wrap_raw_message`, prefer using that wherever + /// your lifetime can be tied to a stack lifetime, and only use this one + /// if its not possible (e.g. with a 'static lifetime). + /// + /// # Safety + /// - The underlying message must be for the same type as `Self` + /// - The underlying message must be alive for the caller-chosen 'msg + /// and not mutated while the wrapper is live. + unsafe fn __unstable_wrap_raw_message_unchecked_lifetime(raw: *const c_void) -> Self; } /// Traits related to message mut interop. Note that these trait fns @@ -176,11 +202,39 @@ pub(crate) mod interop { /// Wraps the provided C++ pointer as a MessageMut. /// + /// This takes a ref of a pointer so that a stack variable's lifetime + /// can be used for a safe lifetime; under most cases this is + /// the correct lifetime and this should be used as: + /// ``` + /// fn called_from_cpp(msg: *mut c_void) { + /// // `msg` is known live for the current stack frame, so mut's + /// // lifetime is also tied to the current stack frame here: + /// let m = unsafe { __unstable_wrap_raw_message_mut(&mut msg); } + /// do_something_with_mut(m); + /// } + /// /// # 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 *mut c_void) -> Self; + + /// Wraps the provided pointer as a MessageMut. + /// + /// Unlike `__unstable_wrap_raw_message_mut` this has no constraints + /// on lifetime: the caller has a free choice for the lifetime. + /// + /// As this is much easier to get the lifetime wrong than + /// `__unstable_wrap_raw_message_mut`, prefer using that wherever + /// the lifetime can be tied to a stack lifetime, and only use this one + /// if its not possible (e.g. with a 'static lifetime). + /// + /// # Safety + /// - The underlying message must be for the same type as `Self` + /// - The underlying message must be alive for the caller-chosen 'msg + /// and not mutated while the wrapper is live. + #[cfg(cpp_kernel)] + unsafe fn __unstable_wrap_raw_message_mut_unchecked_lifetime(raw: *mut c_void) -> Self; } } diff --git a/rust/test/cpp/interop/main.rs b/rust/test/cpp/interop/main.rs index e30df483a..1b4781bb3 100644 --- a/rust/test/cpp/interop/main.rs +++ b/rust/test/cpp/interop/main.rs @@ -35,6 +35,8 @@ extern "C" { fn NewWithExtension() -> *mut c_void; fn GetBytesExtension(msg: *const c_void) -> PtrAndLen; + + fn GetConstStaticTestAllTypes() -> *const c_void; } #[gtest] @@ -138,3 +140,14 @@ fn smuggle_extension() { unsafe { GetBytesExtension(msg2.as_mut().__unstable_as_raw_message_mut()).as_ref() }; assert_eq!(bytes, b"smuggled"); } + +#[gtest] +fn view_of_const_static() { + let view: TestAllTypesView<'static> = unsafe { + TestAllTypesView::__unstable_wrap_raw_message_unchecked_lifetime( + GetConstStaticTestAllTypes(), + ) + }; + assert_eq!(view.optional_int64(), 0); + assert_eq!(view.default_int32(), 41); +} diff --git a/rust/test/cpp/interop/test_utils.cc b/rust/test/cpp/interop/test_utils.cc index c6f5d8400..beac499fb 100644 --- a/rust/test/cpp/interop/test_utils.cc +++ b/rust/test/cpp/interop/test_utils.cc @@ -59,3 +59,8 @@ extern "C" int32_t TakeOwnershipAndGetOptionalInt32( delete msg; return i; } + +extern "C" const void* GetConstStaticTestAllTypes() { + static const auto* msg = new protobuf_unittest::TestAllTypes; + return msg; +} diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc index bed8bcc6d..be85bffc6 100644 --- a/src/google/protobuf/compiler/rust/message.cc +++ b/src/google/protobuf/compiler/rust/message.cc @@ -1339,6 +1339,14 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { $pbr$::RawMessage::new(*msg as *mut _).unwrap()) } } + unsafe fn __unstable_wrap_raw_message_mut_unchecked_lifetime( + msg: *mut std::ffi::c_void) -> Self { + Self { + inner: $pbr$::MutatorMessageRef::wrap_raw( + $pbi$::Private, + $pbr$::RawMessage::new(msg as *mut _).unwrap()) + } + } fn __unstable_as_raw_message_mut(&mut self) -> *mut std::ffi::c_void { self.raw_msg().as_ptr() as *mut _ } @@ -1349,6 +1357,10 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { msg: &'a *const std::ffi::c_void) -> Self { Self::new($pbi$::Private, $pbr$::RawMessage::new(*msg as *mut _).unwrap()) } + unsafe fn __unstable_wrap_raw_message_unchecked_lifetime( + msg: *const std::ffi::c_void) -> Self { + Self::new($pbi$::Private, $pbr$::RawMessage::new(msg as *mut _).unwrap()) + } fn __unstable_as_raw_message(&self) -> *const std::ffi::c_void { self.msg.as_ptr() as *const _ } @@ -1365,6 +1377,10 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { msg: &'a *const std::ffi::c_void) -> Self { Self::new($pbi$::Private, $pbr$::RawMessage::new(*msg as *mut _).unwrap()) } + unsafe fn __unstable_wrap_raw_message_unchecked_lifetime( + msg: *const std::ffi::c_void) -> Self { + Self::new($pbi$::Private, $pbr$::RawMessage::new(msg as *mut _).unwrap()) + } fn __unstable_as_raw_message(&self) -> *const std::ffi::c_void { self.msg.as_ptr() as *const _ }