From 256b6710d0f7e4f4b84bfe0b1ca14353c8476999 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 16 Apr 2020 03:01:16 +0200 Subject: [PATCH] Remove transmutes from UniquePtr/UniqueRef implementation (#352) --- src/V8.rs | 6 ++-- src/array_buffer.rs | 2 +- src/inspector.rs | 14 ++++---- src/isolate.rs | 2 +- src/lib.rs | 2 +- src/platform/mod.rs | 2 +- src/platform/task.rs | 2 +- src/support.rs | 80 +++++++++++++++++++++----------------------- tests/test_api.rs | 2 +- 9 files changed, 54 insertions(+), 58 deletions(-) diff --git a/src/V8.rs b/src/V8.rs index 8edf0fc5..da25db30 100644 --- a/src/V8.rs +++ b/src/V8.rs @@ -7,7 +7,7 @@ use std::sync::Mutex; use std::vec::Vec; use crate::platform::Platform; -use crate::support::UniquePtr; +use crate::support::UniqueRef; extern "C" { fn v8__V8__SetFlagsFromCommandLine(argc: *mut c_int, argv: *mut *mut c_char); @@ -91,10 +91,10 @@ pub fn get_version() -> &'static str { // V8::ShutdownPlatform is called. /// Sets the v8::Platform to use. This should be invoked before V8 is /// initialized. -pub fn initialize_platform(platform: UniquePtr) { +pub fn initialize_platform(platform: UniqueRef) { let mut global_state_guard = GLOBAL_STATE.lock().unwrap(); assert_eq!(*global_state_guard, Uninitialized); - unsafe { v8__V8__InitializePlatform(&mut *platform.into_raw()) }; + unsafe { v8__V8__InitializePlatform(platform.into_raw()) }; *global_state_guard = PlatformInitialized; } diff --git a/src/array_buffer.rs b/src/array_buffer.rs index 90eb2476..c6b3724b 100644 --- a/src/array_buffer.rs +++ b/src/array_buffer.rs @@ -138,7 +138,7 @@ fn test_default_allocator() { } impl Delete for Allocator { - fn delete(&'static mut self) { + fn delete(&mut self) { unsafe { v8__ArrayBuffer__Allocator__DELETE(self) }; } } diff --git a/src/inspector.rs b/src/inspector.rs index 9ae086a8..d77110e3 100644 --- a/src/inspector.rs +++ b/src/inspector.rs @@ -70,9 +70,7 @@ extern "C" { stack_trace: &mut V8StackTrace, ) -> (); - fn v8_inspector__V8InspectorSession__DELETE( - this: &'static mut V8InspectorSession, - ); + fn v8_inspector__V8InspectorSession__DELETE(this: &mut V8InspectorSession); fn v8_inspector__V8InspectorSession__dispatchProtocolMessage( session: *mut V8InspectorSession, message: &StringView, @@ -83,14 +81,14 @@ extern "C" { break_details: &StringView, ); - fn v8_inspector__StringBuffer__DELETE(this: &'static mut StringBuffer) -> (); + fn v8_inspector__StringBuffer__DELETE(this: &mut StringBuffer) -> (); fn v8_inspector__StringBuffer__string(this: &mut StringBuffer) -> &StringView; fn v8_inspector__StringBuffer__create( source: &StringView, ) -> UniquePtr; - fn v8_inspector__V8Inspector__DELETE(this: &'static mut V8Inspector); + fn v8_inspector__V8Inspector__DELETE(this: &mut V8Inspector); fn v8_inspector__V8Inspector__create( isolate: *mut Isolate, client: *mut V8InspectorClient, @@ -586,7 +584,7 @@ impl V8InspectorSession { } impl Delete for V8InspectorSession { - fn delete(&'static mut self) { + fn delete(&mut self) { unsafe { v8_inspector__V8InspectorSession__DELETE(self) }; } } @@ -618,7 +616,7 @@ impl StringBuffer { } impl Delete for StringBuffer { - fn delete(&'static mut self) { + fn delete(&mut self) { unsafe { v8_inspector__StringBuffer__DELETE(self) } } } @@ -894,7 +892,7 @@ impl V8Inspector { } impl Delete for V8Inspector { - fn delete(&'static mut self) { + fn delete(&mut self) { unsafe { v8_inspector__V8Inspector__DELETE(self) }; } } diff --git a/src/isolate.rs b/src/isolate.rs index 5b470e1b..52135800 100644 --- a/src/isolate.rs +++ b/src/isolate.rs @@ -576,7 +576,7 @@ impl CreateParams { } impl Delete for CreateParams { - fn delete(&'static mut self) { + fn delete(&mut self) { unsafe { v8__Isolate__CreateParams__DELETE(self) } } } diff --git a/src/lib.rs b/src/lib.rs index fc43675d..6f56bfff 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,7 +5,7 @@ //! ```rust //! use rusty_v8 as v8; //! -//! let platform = v8::new_default_platform(); +//! let platform = v8::new_default_platform().unwrap(); //! v8::V8::initialize_platform(platform); //! v8::V8::initialize(); //! diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 977bf9bf..1f87f944 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -23,7 +23,7 @@ pub fn new_default_platform() -> UniquePtr { pub struct Platform(Opaque); impl Delete for Platform { - fn delete(&'static mut self) { + fn delete(&mut self) { unsafe { v8__Platform__DELETE(self) } } } diff --git a/src/platform/task.rs b/src/platform/task.rs index 26b67bd2..9e6d6bf8 100644 --- a/src/platform/task.rs +++ b/src/platform/task.rs @@ -42,7 +42,7 @@ impl Task { } impl Delete for Task { - fn delete(&'static mut self) { + fn delete(&mut self) { unsafe { v8__Task__DELETE(self) } } } diff --git a/src/support.rs b/src/support.rs index ee11579f..1fdb4d69 100644 --- a/src/support.rs +++ b/src/support.rs @@ -1,10 +1,11 @@ use std::cell::UnsafeCell; use std::marker::PhantomData; -use std::mem::replace; +use std::mem::forget; use std::mem::size_of; use std::mem::transmute; use std::ops::Deref; use std::ops::DerefMut; +use std::ptr::null_mut; use std::ptr::NonNull; // TODO use libc::intptr_t when stable. @@ -20,14 +21,14 @@ pub type Opaque = [u8; 0]; pub trait Delete where - Self: Sized + 'static, + Self: Sized, { - fn delete(&'static mut self) -> (); + fn delete(&mut self) -> (); } /// Pointer to object allocated on the C++ heap. The pointer may be null. #[repr(transparent)] -pub struct UniquePtr(Option<&'static mut T>) +pub struct UniquePtr(Option>) where T: Delete; @@ -39,22 +40,25 @@ where Self(None) } - pub fn new(r: &'static mut T) -> Self { - Self(Some(r)) - } - - pub unsafe fn from_raw(p: *mut T) -> Self { - transmute(p) + pub unsafe fn from_raw(ptr: *mut T) -> Self { + Self(UniqueRef::try_from_raw(ptr)) } pub fn into_raw(self) -> *mut T { - unsafe { transmute(self) } + self + .0 + .map(|unique_ref| unique_ref.into_raw()) + .unwrap_or_else(null_mut) } pub fn unwrap(self) -> UniqueRef { - let p = self.into_raw(); - assert!(!p.is_null()); - unsafe { UniqueRef::from_raw(p) } + self.0.unwrap() + } + + unsafe fn _static_assert_has_pointer_repr() { + let dummy: fn() -> Self = || unimplemented!(); + let _ptr: *mut T = transmute(dummy()); + let _ref: &mut T = transmute(dummy()); } } @@ -63,7 +67,7 @@ where T: Delete, { fn from(unique_ref: UniqueRef) -> Self { - unsafe { Self::from_raw(unique_ref.into_raw()) } + Self(Some(unique_ref)) } } @@ -71,7 +75,7 @@ impl Deref for UniquePtr where T: Delete, { - type Target = Option<&'static mut T>; + type Target = Option>; fn deref(&self) -> &Self::Target { &self.0 } @@ -86,20 +90,9 @@ where } } -impl Drop for UniquePtr -where - T: Delete, -{ - fn drop(&mut self) { - if let Some(v) = self.0.take() { - Delete::delete(v) - } - } -} - /// Pointer to object allocated on the C++ heap. The pointer may not be null. #[repr(transparent)] -pub struct UniqueRef(&'static mut T) +pub struct UniqueRef(NonNull) where T: Delete; @@ -107,10 +100,6 @@ impl UniqueRef where T: Delete, { - pub fn new(r: &'static mut T) -> Self { - Self(r) - } - pub fn make_shared(self) -> SharedRef where T: Shared, @@ -118,12 +107,24 @@ where self.into() } - pub unsafe fn from_raw(p: *mut T) -> Self { - transmute(NonNull::new(p)) + pub unsafe fn from_raw(ptr: *mut T) -> Self { + Self::try_from_raw(ptr).unwrap() } pub fn into_raw(self) -> *mut T { - unsafe { transmute(self) } + let ptr = self.0.as_ptr(); + forget(self); + ptr + } + + unsafe fn try_from_raw(ptr: *mut T) -> Option { + NonNull::new(ptr).map(Self) + } + + unsafe fn _static_assert_has_pointer_repr() { + let dummy: fn() -> Self = || unimplemented!(); + let _ptr: *mut T = transmute(dummy()); + let _ref: &mut T = transmute(dummy()); } } @@ -133,7 +134,7 @@ where { type Target = T; fn deref(&self) -> &Self::Target { - self.0 + unsafe { self.0.as_ref() } } } @@ -142,7 +143,7 @@ where T: Delete, { fn deref_mut(&mut self) -> &mut Self::Target { - self.0 + unsafe { self.0.as_mut() } } } @@ -151,10 +152,7 @@ where T: Delete, { fn drop(&mut self) { - let inner = replace(&mut self.0, unsafe { - transmute(NonNull::<&'static mut T>::dangling()) - }); - Delete::delete(inner) + Delete::delete(&mut **self) } } diff --git a/tests/test_api.rs b/tests/test_api.rs index a0eed28f..0b31c404 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -29,7 +29,7 @@ fn setup() -> SetupGuard { let mut g = INIT_LOCK.lock().unwrap(); *g += 1; if *g == 1 { - v8::V8::initialize_platform(v8::new_default_platform()); + v8::V8::initialize_platform(v8::new_default_platform().unwrap()); v8::V8::initialize(); } SetupGuard {}