From f7ee6dc63529e6f7cb429a64d8d5c522ff2d6416 Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Sun, 17 Jul 2022 00:03:40 +0100 Subject: [PATCH 1/5] Add lua_xpush to 5.1-5.4 --- src/ffi/lua51/lua.rs | 6 ++++++ src/ffi/lua52/lua.rs | 6 ++++++ src/ffi/lua53/lua.rs | 6 ++++++ src/ffi/lua54/lua.rs | 6 ++++++ src/lua.rs | 6 ------ 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/ffi/lua51/lua.rs b/src/ffi/lua51/lua.rs index 6c99503..5503b7f 100644 --- a/src/ffi/lua51/lua.rs +++ b/src/ffi/lua51/lua.rs @@ -324,6 +324,12 @@ pub unsafe fn lua_tostring(L: *mut lua_State, i: c_int) -> *const c_char { lua_tolstring(L, i, ptr::null_mut()) } +#[inline(always)] +pub unsafe fn lua_xpush(from: *mut lua_State, to: *mut lua_State, idx: c_int) { + lua_pushvalue(from, idx); + lua_xmove(from, to, 1); +} + // // Debug API // diff --git a/src/ffi/lua52/lua.rs b/src/ffi/lua52/lua.rs index f8c7108..482e69f 100644 --- a/src/ffi/lua52/lua.rs +++ b/src/ffi/lua52/lua.rs @@ -410,6 +410,12 @@ pub unsafe fn lua_tostring(L: *mut lua_State, i: c_int) -> *const c_char { lua_tolstring(L, i, ptr::null_mut()) } +#[inline(always)] +pub unsafe fn lua_xpush(from: *mut lua_State, to: *mut lua_State, idx: c_int) { + lua_pushvalue(from, idx); + lua_xmove(from, to, 1); +} + // // Debug API // diff --git a/src/ffi/lua53/lua.rs b/src/ffi/lua53/lua.rs index 753408c..c0127bb 100644 --- a/src/ffi/lua53/lua.rs +++ b/src/ffi/lua53/lua.rs @@ -434,6 +434,12 @@ pub unsafe fn lua_replace(L: *mut lua_State, idx: c_int) { lua_pop(L, 1) } +#[inline(always)] +pub unsafe fn lua_xpush(from: *mut lua_State, to: *mut lua_State, idx: c_int) { + lua_pushvalue(from, idx); + lua_xmove(from, to, 1); +} + // // Debug API // diff --git a/src/ffi/lua54/lua.rs b/src/ffi/lua54/lua.rs index 113f324..f124c37 100644 --- a/src/ffi/lua54/lua.rs +++ b/src/ffi/lua54/lua.rs @@ -455,6 +455,12 @@ pub unsafe fn lua_replace(L: *mut lua_State, idx: c_int) { lua_pop(L, 1) } +#[inline(always)] +pub unsafe fn lua_xpush(from: *mut lua_State, to: *mut lua_State, idx: c_int) { + lua_pushvalue(from, idx); + lua_xmove(from, to, 1); +} + #[inline(always)] pub unsafe fn lua_newuserdata(L: *mut lua_State, sz: usize) -> *mut c_void { lua_newuserdatauv(L, sz, 1) diff --git a/src/lua.rs b/src/lua.rs index 77a79a7..1a20328 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -2394,12 +2394,6 @@ impl Lua { "Lua instance passed Value created from a different main Lua state" ); let extra = &*self.extra.get(); - #[cfg(not(feature = "luau"))] - { - ffi::lua_pushvalue(extra.ref_thread, lref.index); - ffi::lua_xmove(extra.ref_thread, self.state, 1); - } - #[cfg(feature = "luau")] ffi::lua_xpush(extra.ref_thread, self.state, lref.index); } From 059e41bafba68aa06d7c6b4079d8d67f1ac4ca0b Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Sun, 17 Jul 2022 10:57:36 +0100 Subject: [PATCH 2/5] Optimize `WrappedFailure` userdata detection. This is done by comparing a metatable pointer to previously saved one (it never changes). --- src/lua.rs | 17 ++++++++++++++++- src/util.rs | 36 ++++++++++++++++++++++++------------ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/lua.rs b/src/lua.rs index 1a20328..73bc1db 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -111,6 +111,9 @@ pub(crate) struct ExtraData { #[cfg(feature = "async")] recycled_thread_cache: Vec, + // Address of `WrappedFailure` metatable + wrapped_failure_mt_ptr: *const c_void, + // Index of `Option` userdata on the ref thread #[cfg(feature = "async")] ref_waker_idx: c_int, @@ -538,6 +541,13 @@ impl Lua { "Error while creating ref thread", ); + let wrapped_failure_mt_ptr = { + get_gc_metatable::(state); + let ptr = ffi::lua_topointer(state, -1); + ffi::lua_pop(state, 1); + ptr + }; + // Create empty Waker slot on the ref thread #[cfg(feature = "async")] let ref_waker_idx = { @@ -568,6 +578,7 @@ impl Lua { multivalue_cache: Vec::with_capacity(MULTIVALUE_CACHE_SIZE), #[cfg(feature = "async")] recycled_thread_cache: Vec::new(), + wrapped_failure_mt_ptr, #[cfg(feature = "async")] ref_waker_idx, #[cfg(not(feature = "luau"))] @@ -2293,6 +2304,8 @@ impl Lua { // Uses 2 stack spaces, does not call checkstack pub(crate) unsafe fn pop_value(&self) -> Value { let state = self.state; + let extra = &mut *self.extra.get(); + match ffi::lua_type(state, -1) { ffi::LUA_TNIL => { ffi::lua_pop(state, 1); @@ -2353,9 +2366,11 @@ impl Lua { ffi::LUA_TFUNCTION => Value::Function(Function(self.pop_ref())), ffi::LUA_TUSERDATA => { + let wrapped_failure_mt_ptr = extra.wrapped_failure_mt_ptr; // We must prevent interaction with userdata types other than UserData OR a WrappedError. // WrappedPanics are automatically resumed. - match get_gc_userdata::(state, -1).as_mut() { + match get_gc_userdata::(state, -1, wrapped_failure_mt_ptr).as_mut() + { Some(WrappedFailure::Error(err)) => { let err = err.clone(); ffi::lua_pop(state, 1); diff --git a/src/util.rs b/src/util.rs index aca50f1..592f54c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -203,7 +203,7 @@ pub unsafe fn pop_error(state: *mut ffi::lua_State, err_code: c_int) -> Error { "pop_error called with non-error return code" ); - match get_gc_userdata::(state, -1).as_mut() { + match get_gc_userdata::(state, -1, ptr::null()).as_mut() { Some(WrappedFailure::Error(err)) => { ffi::lua_pop(state, 1); err.clone() @@ -394,16 +394,28 @@ pub unsafe fn push_gc_userdata( } // Uses 2 stack spaces, does not call checkstack -pub unsafe fn get_gc_userdata(state: *mut ffi::lua_State, index: c_int) -> *mut T { +pub unsafe fn get_gc_userdata( + state: *mut ffi::lua_State, + index: c_int, + mt_ptr: *const c_void, +) -> *mut T { let ud = ffi::lua_touserdata(state, index) as *mut T; if ud.is_null() || ffi::lua_getmetatable(state, index) == 0 { return ptr::null_mut(); } - get_gc_metatable::(state); - let res = ffi::lua_rawequal(state, -1, -2); - ffi::lua_pop(state, 2); - if res == 0 { - return ptr::null_mut(); + if !mt_ptr.is_null() { + let ud_mt_ptr = ffi::lua_topointer(state, -1); + ffi::lua_pop(state, 1); + if !ptr::eq(ud_mt_ptr, mt_ptr) { + return ptr::null_mut(); + } + } else { + get_gc_metatable::(state); + let res = ffi::lua_rawequal(state, -1, -2); + ffi::lua_pop(state, 2); + if res == 0 { + return ptr::null_mut(); + } } ud } @@ -679,7 +691,7 @@ pub unsafe extern "C" fn error_traceback(state: *mut ffi::lua_State) -> c_int { return 1; } - if get_gc_userdata::(state, -1).is_null() { + if get_gc_userdata::(state, -1, ptr::null()).is_null() { let s = ffi::luaL_tolstring(state, -1, ptr::null_mut()); if ffi::lua_checkstack(state, ffi::LUA_TRACEBACK_STACK) != 0 { ffi::luaL_traceback(state, state, s, 0); @@ -706,7 +718,7 @@ pub unsafe extern "C" fn safe_pcall(state: *mut ffi::lua_State) -> c_int { ffi::lua_gettop(state) } else { if let Some(WrappedFailure::Panic(_)) = - get_gc_userdata::(state, -1).as_ref() + get_gc_userdata::(state, -1, ptr::null()).as_ref() { ffi::lua_error(state); } @@ -722,7 +734,7 @@ pub unsafe extern "C" fn safe_xpcall(state: *mut ffi::lua_State) -> c_int { ffi::luaL_checkstack(state, 2, ptr::null()); if let Some(WrappedFailure::Panic(_)) = - get_gc_userdata::(state, -1).as_ref() + get_gc_userdata::(state, -1, ptr::null()).as_ref() { 1 } else { @@ -752,7 +764,7 @@ pub unsafe extern "C" fn safe_xpcall(state: *mut ffi::lua_State) -> c_int { ffi::lua_gettop(state) - 1 } else { if let Some(WrappedFailure::Panic(_)) = - get_gc_userdata::(state, -1).as_ref() + get_gc_userdata::(state, -1, ptr::null()).as_ref() { ffi::lua_error(state); } @@ -836,7 +848,7 @@ pub unsafe fn init_error_registry(state: *mut ffi::lua_State) -> Result<()> { callback_error(state, |_| { check_stack(state, 3)?; - let err_buf = match get_gc_userdata::(state, -1).as_ref() { + let err_buf = match get_gc_userdata::(state, -1, ptr::null()).as_ref() { Some(WrappedFailure::Error(error)) => { let err_buf_key = &ERROR_PRINT_BUFFER_KEY as *const u8 as *const c_void; ffi::lua_rawgetp(state, ffi::LUA_REGISTRYINDEX, err_buf_key); From f75af6d75ff81e1695e09da8d9fbd1343b589c3a Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Sun, 17 Jul 2022 11:11:30 +0100 Subject: [PATCH 3/5] Fix clippy warnings --- src/lua.rs | 4 ++-- src/value.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lua.rs b/src/lua.rs index 73bc1db..14a91be 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -2961,14 +2961,14 @@ struct StateGuard<'a>(&'a mut LuaInner, *mut ffi::lua_State); impl<'a> StateGuard<'a> { fn new(inner: &'a mut LuaInner, mut state: *mut ffi::lua_State) -> Self { - mem::swap(&mut (*inner).state, &mut state); + mem::swap(&mut inner.state, &mut state); Self(inner, state) } } impl<'a> Drop for StateGuard<'a> { fn drop(&mut self) { - mem::swap(&mut (*self.0).state, &mut self.1); + mem::swap(&mut self.0.state, &mut self.1); } } diff --git a/src/value.rs b/src/value.rs index b77f2bc..2d7403f 100644 --- a/src/value.rs +++ b/src/value.rs @@ -239,7 +239,7 @@ impl<'a, 'lua> IntoIterator for &'a MultiValue<'lua> { #[inline] fn into_iter(self) -> Self::IntoIter { - (&self.0).iter().rev() + self.0.iter().rev() } } From f9ff6116dbec6ed7815d14c741f23eaee8b6621e Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Sun, 17 Jul 2022 11:33:51 +0100 Subject: [PATCH 4/5] Use MaybeUninit instead of hack in protect_lua_closure --- src/util.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/util.rs b/src/util.rs index 592f54c..e326c94 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,6 +1,7 @@ use std::any::{Any, TypeId}; use std::ffi::CStr; use std::fmt::Write; +use std::mem::MaybeUninit; use std::os::raw::{c_char, c_int, c_void}; use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe}; use std::sync::Arc; @@ -136,26 +137,21 @@ where F: Fn(*mut ffi::lua_State) -> R, R: Copy, { - union URes { - uninit: (), - init: R, - } - struct Params { function: F, - result: URes, + result: MaybeUninit, nresults: c_int, } unsafe extern "C" fn do_call(state: *mut ffi::lua_State) -> c_int where - R: Copy, F: Fn(*mut ffi::lua_State) -> R, + R: Copy, { let params = ffi::lua_touserdata(state, -1) as *mut Params; ffi::lua_pop(state, 1); - (*params).result.init = ((*params).function)(state); + (*params).result.write(((*params).function)(state)); if (*params).nresults == ffi::LUA_MULTRET { ffi::lua_gettop(state) @@ -174,7 +170,7 @@ where let mut params = Params { function: f, - result: URes { uninit: () }, + result: MaybeUninit::uninit(), nresults, }; @@ -185,7 +181,7 @@ where if ret == ffi::LUA_OK { // `LUA_OK` is only returned when the `do_call` function has completed successfully, so // `params.result` is definitely initialized. - Ok(params.result.init) + Ok(params.result.assume_init()) } else { Err(pop_error(state, ret)) } From d3b48cf2f314f780dfa1d2eb91645cc3c78db4ee Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Mon, 18 Jul 2022 00:59:17 +0100 Subject: [PATCH 5/5] Use Luau tags to mark userdata objects as destructed --- Cargo.toml | 2 +- src/ffi/luau/lua.rs | 8 +++++++- src/lua.rs | 10 +++++----- src/types.rs | 2 +- src/util.rs | 14 ++++++-------- 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3c72242..98bbcb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,7 +58,7 @@ cc = { version = "1.0" } pkg-config = { version = "0.3.17" } lua-src = { version = ">= 544.0.0, < 550.0.0", optional = true } luajit-src = { version = ">= 210.4.0, < 220.0.0", optional = true } -luau0-src = { version = "0.3.2", optional = true } +luau0-src = { version = "0.3.6", optional = true } [dev-dependencies] rustyline = "9.0" diff --git a/src/ffi/luau/lua.rs b/src/ffi/luau/lua.rs index 8aa83fc..cbc2c40 100644 --- a/src/ffi/luau/lua.rs +++ b/src/ffi/luau/lua.rs @@ -260,6 +260,7 @@ extern "C" { pub fn lua_concat(L: *mut lua_State, n: c_int); // TODO: lua_encodepointer pub fn lua_clock() -> c_double; + pub fn lua_setuserdatatag(L: *mut lua_State, idx: c_int, tag: c_int); pub fn lua_setuserdatadtor( L: *mut lua_State, tag: c_int, @@ -437,7 +438,12 @@ extern "C" { pub fn lua_setupvalue(L: *mut lua_State, funcindex: c_int, n: c_int) -> *const c_char; pub fn lua_singlestep(L: *mut lua_State, enabled: c_int); - pub fn lua_breakpoint(L: *mut lua_State, funcindex: c_int, line: c_int, enabled: c_int); + pub fn lua_breakpoint( + L: *mut lua_State, + funcindex: c_int, + line: c_int, + enabled: c_int, + ) -> c_int; pub fn lua_getcoverage( L: *mut lua_State, diff --git a/src/lua.rs b/src/lua.rs index 14a91be..60f8bc9 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -25,7 +25,7 @@ use crate::string::String; use crate::table::Table; use crate::thread::Thread; use crate::types::{ - Callback, CallbackUpvalue, DestructedUserdataMT, Integer, LightUserData, LuaRef, MaybeSend, + Callback, CallbackUpvalue, DestructedUserdata, Integer, LightUserData, LuaRef, MaybeSend, Number, RegistryKey, }; use crate::userdata::{AnyUserData, UserData, UserDataCell}; @@ -602,13 +602,13 @@ impl Lua { "Error while storing extra data", ); - // Register `DestructedUserdataMT` type + // Register `DestructedUserdata` type get_destructed_userdata_metatable(main_state); let destructed_mt_ptr = ffi::lua_topointer(main_state, -1); - let destructed_mt_typeid = Some(TypeId::of::()); + let destructed_ud_typeid = TypeId::of::(); (*extra.get()) .registered_userdata_mt - .insert(destructed_mt_ptr, destructed_mt_typeid); + .insert(destructed_mt_ptr, Some(destructed_ud_typeid)); ffi::lua_pop(main_state, 1); mlua_debug_assert!( @@ -2587,7 +2587,7 @@ impl Lua { let extra = &*self.extra.get(); match extra.registered_userdata_mt.get(&mt_ptr) { - Some(&type_id) if type_id == Some(TypeId::of::()) => { + Some(&type_id) if type_id == Some(TypeId::of::()) => { Err(Error::UserDataDestructed) } Some(&type_id) => Ok(type_id), diff --git a/src/types.rs b/src/types.rs index 68a7032..7b1f653 100644 --- a/src/types.rs +++ b/src/types.rs @@ -83,7 +83,7 @@ pub trait MaybeSend {} #[cfg(not(feature = "send"))] impl MaybeSend for T {} -pub(crate) struct DestructedUserdataMT; +pub(crate) struct DestructedUserdata; /// An auto generated key into the Lua registry. /// diff --git a/src/util.rs b/src/util.rs index e326c94..66b26e7 100644 --- a/src/util.rs +++ b/src/util.rs @@ -310,10 +310,7 @@ pub unsafe fn push_userdata(state: *mut ffi::lua_State, t: T, protect: bool) #[inline] pub unsafe fn push_userdata(state: *mut ffi::lua_State, t: T, protect: bool) -> Result<()> { unsafe extern "C" fn destructor(ud: *mut c_void) { - let ud = ud as *mut T; - if *(ud.offset(1) as *mut u8) == 0 { - ptr::drop_in_place(ud); - } + ptr::drop_in_place(ud as *mut T); } let size = mem::size_of::() + 1; @@ -325,7 +322,6 @@ pub unsafe fn push_userdata(state: *mut ffi::lua_State, t: T, protect: bool) ffi::lua_newuserdatadtor(state, size, destructor::) as *mut T }; ptr::write(ud, t); - *(ud.offset(1) as *mut u8) = 0; // Mark as not destructed Ok(()) } @@ -369,10 +365,12 @@ pub unsafe fn take_userdata(state: *mut ffi::lua_State) -> T { get_destructed_userdata_metatable(state); ffi::lua_setmetatable(state, -2); let ud = get_userdata::(state, -1); + + // Update userdata tag to disable destructor and mark as destructed + #[cfg(feature = "luau")] + ffi::lua_setuserdatatag(state, -1, 1); + ffi::lua_pop(state, 1); - if cfg!(feature = "luau") { - *(ud.offset(1) as *mut u8) = 1; // Mark as destructed - } ptr::read(ud) }