From 5fdd8fac4481273e1e651a9433556a4785824744 Mon Sep 17 00:00:00 2001 From: Joel Wurtz Date: Wed, 25 Oct 2023 15:15:43 +0200 Subject: [PATCH] feat(zend): add helper for try catch and bailout in PHP (#275) * feat(zend): add helper for try catch and bailout in PHP * feat(try): add bindings for bailout * fix(try): add missing feature flag for test * feat(try): add a test that expose memory leak problem * feat(try): make bailout unsafe and explain why * feat(bailout): flag bailout as a panic function * feat(embed): add try catch on script / eval --- allowed_bindings.rs | 3 +- build.rs | 2 + docsrs_bindings.rs | 3 + src/embed/embed.c | 2 +- src/embed/ffi.rs | 2 +- src/embed/mod.rs | 92 +++++++++++++++--------- src/ffi.rs | 6 ++ src/wrapper.c | 14 ++++ src/wrapper.h | 4 +- src/zend/mod.rs | 4 ++ src/zend/try_catch.rs | 164 ++++++++++++++++++++++++++++++++++++++++++ 11 files changed, 259 insertions(+), 37 deletions(-) create mode 100644 src/zend/try_catch.rs diff --git a/allowed_bindings.rs b/allowed_bindings.rs index 4a5dbfc..ca2195b 100644 --- a/allowed_bindings.rs +++ b/allowed_bindings.rs @@ -261,5 +261,6 @@ bind! { zend_file_handle, zend_stream_init_filename, php_execute_script, - zend_register_module_ex + zend_register_module_ex, + _zend_bailout } diff --git a/build.rs b/build.rs index 4ee8041..78c9cf3 100644 --- a/build.rs +++ b/build.rs @@ -248,6 +248,8 @@ fn main() -> Result<()> { for path in [ manifest.join("src").join("wrapper.h"), manifest.join("src").join("wrapper.c"), + manifest.join("src").join("embed").join("embed.h"), + manifest.join("src").join("embed").join("embed.c"), manifest.join("allowed_bindings.rs"), manifest.join("windows_build.rs"), manifest.join("unix_build.rs"), diff --git a/docsrs_bindings.rs b/docsrs_bindings.rs index 2d22490..f7e54f1 100644 --- a/docsrs_bindings.rs +++ b/docsrs_bindings.rs @@ -789,6 +789,9 @@ pub struct _zend_class_entry__bindgen_ty_4__bindgen_ty_2 { pub builtin_functions: *const _zend_function_entry, pub module: *mut _zend_module_entry, } +extern "C" { + pub fn _zend_bailout(filename: *const ::std::os::raw::c_char, lineno: u32) -> !; +} extern "C" { pub static mut zend_interrupt_function: ::std::option::Option; diff --git a/src/embed/embed.c b/src/embed/embed.c index d8b3a78..ae7d8bc 100644 --- a/src/embed/embed.c +++ b/src/embed/embed.c @@ -3,7 +3,7 @@ // We actually use the PHP embed API to run PHP code in test // At some point we might want to use our own SAPI to do that void* ext_php_rs_embed_callback(int argc, char** argv, void* (*callback)(void *), void *ctx) { - void *result; + void *result = NULL; PHP_EMBED_START_BLOCK(argc, argv) diff --git a/src/embed/ffi.rs b/src/embed/ffi.rs index 3a1f6d7..b52ce6a 100644 --- a/src/embed/ffi.rs +++ b/src/embed/ffi.rs @@ -10,7 +10,7 @@ extern "C" { pub fn ext_php_rs_embed_callback( argc: c_int, argv: *mut *mut c_char, - func: unsafe extern "C" fn(*const c_void) -> *mut c_void, + func: unsafe extern "C" fn(*const c_void) -> *const c_void, ctx: *const c_void, ) -> *mut c_void; } diff --git a/src/embed/mod.rs b/src/embed/mod.rs index 581e385..0ad64a6 100644 --- a/src/embed/mod.rs +++ b/src/embed/mod.rs @@ -13,10 +13,10 @@ use crate::ffi::{ zend_stream_init_filename, ZEND_RESULT_CODE_SUCCESS, }; use crate::types::{ZendObject, Zval}; -use crate::zend::ExecutorGlobals; +use crate::zend::{panic_wrapper, try_catch, ExecutorGlobals}; use parking_lot::{const_rwlock, RwLock}; use std::ffi::{c_char, c_void, CString, NulError}; -use std::panic::{catch_unwind, resume_unwind, RefUnwindSafe}; +use std::panic::{resume_unwind, RefUnwindSafe}; use std::path::Path; use std::ptr::null_mut; @@ -29,6 +29,13 @@ pub enum EmbedError { ExecuteScriptError, InvalidEvalString(NulError), InvalidPath, + CatchError, +} + +impl EmbedError { + pub fn is_bailout(&self) -> bool { + matches!(self, EmbedError::CatchError) + } } static RUN_FN_LOCK: RwLock<()> = const_rwlock(()); @@ -79,10 +86,12 @@ impl Embed { zend_stream_init_filename(&mut file_handle, path.as_ptr()); } - if unsafe { php_execute_script(&mut file_handle) } { - Ok(()) - } else { - Err(EmbedError::ExecuteScriptError) + let exec_result = try_catch(|| unsafe { php_execute_script(&mut file_handle) }); + + match exec_result { + Err(_) => Err(EmbedError::CatchError), + Ok(true) => Ok(()), + Ok(false) => Err(EmbedError::ExecuteScriptError), } } @@ -93,6 +102,12 @@ impl Embed { /// Which means subsequent calls to `Embed::eval` or `Embed::run_script` will be able to access /// variables defined in previous calls /// + /// # Returns + /// + /// * R - The result of the function passed to this method + /// + /// R must implement [`Default`] so it can be returned in case of a bailout + /// /// # Example /// /// ``` @@ -105,41 +120,36 @@ impl Embed { /// assert_eq!(foo.unwrap().string().unwrap(), "foo"); /// }); /// ``` - pub fn run(func: F) { + pub fn run R + RefUnwindSafe>(func: F) -> R + where + R: Default, + { // @TODO handle php thread safe // // This is to prevent multiple threads from running php at the same time // At some point we should detect if php is compiled with thread safety and avoid doing that in this case let _guard = RUN_FN_LOCK.write(); - unsafe extern "C" fn wrapper(ctx: *const c_void) -> *mut c_void { - // we try to catch panic here so we correctly shutdown php if it happens - // mandatory when we do assert on test as other test would not run correctly - let panic = catch_unwind(|| { - (*(ctx as *const F))(); - }); - - let panic_ptr = Box::into_raw(Box::new(panic)); - - panic_ptr as *mut c_void - } - let panic = unsafe { ext_php_rs_embed_callback( 0, null_mut(), - wrapper::, + panic_wrapper::, &func as *const F as *const c_void, ) }; + // This can happen if there is a bailout if panic.is_null() { - return; + return R::default(); } - if let Err(err) = unsafe { *Box::from_raw(panic as *mut std::thread::Result<()>) } { - // we resume the panic here so it can be catched correctly by the test framework - resume_unwind(err); + match unsafe { *Box::from_raw(panic as *mut std::thread::Result) } { + Ok(r) => r, + Err(err) => { + // we resume the panic here so it can be catched correctly by the test framework + resume_unwind(err); + } } } @@ -170,21 +180,18 @@ impl Embed { let mut result = Zval::new(); - // this eval is very limited as it only allow simple code, it's the same eval used by php -r - let exec_result = unsafe { + let exec_result = try_catch(|| unsafe { zend_eval_string( cstr.as_ptr() as *const c_char, &mut result, b"run\0".as_ptr() as *const _, ) - }; + }); - let exception = ExecutorGlobals::take_exception(); - - if exec_result != ZEND_RESULT_CODE_SUCCESS { - Err(EmbedError::ExecuteError(exception)) - } else { - Ok(result) + match exec_result { + Err(_) => Err(EmbedError::CatchError), + Ok(ZEND_RESULT_CODE_SUCCESS) => Ok(result), + Ok(_) => Err(EmbedError::ExecuteError(ExecutorGlobals::take_exception())), } } } @@ -244,4 +251,23 @@ mod tests { panic!("test panic"); }); } + + #[test] + fn test_return() { + let foo = Embed::run(|| { + return "foo"; + }); + + assert_eq!(foo, "foo"); + } + + #[test] + fn test_eval_bailout() { + Embed::run(|| { + let result = Embed::eval("str_repeat('a', 100_000_000_000_000);"); + + assert!(result.is_err()); + assert!(result.unwrap_err().is_bailout()); + }); + } } diff --git a/src/ffi.rs b/src/ffi.rs index 92614c4..a6c3d94 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -26,6 +26,12 @@ extern "C" { pub fn ext_php_rs_zend_object_alloc(obj_size: usize, ce: *mut zend_class_entry) -> *mut c_void; pub fn ext_php_rs_zend_object_release(obj: *mut zend_object); pub fn ext_php_rs_executor_globals() -> *mut zend_executor_globals; + pub fn ext_php_rs_zend_try_catch( + func: unsafe extern "C" fn(*const c_void) -> *const c_void, + ctx: *const c_void, + result: *mut *mut c_void, + ) -> bool; + pub fn ext_php_rs_zend_bailout() -> !; } include!(concat!(env!("OUT_DIR"), "/bindings.rs")); diff --git a/src/wrapper.c b/src/wrapper.c index faf585e..a1fd900 100644 --- a/src/wrapper.c +++ b/src/wrapper.c @@ -39,3 +39,17 @@ zend_executor_globals *ext_php_rs_executor_globals() { return &executor_globals; #endif } + +bool ext_php_rs_zend_try_catch(void* (*callback)(void *), void *ctx, void **result) { + zend_try { + *result = callback(ctx); + } zend_catch { + return true; + } zend_end_try(); + + return false; +} + +void ext_php_rs_zend_bailout() { + zend_bailout(); +} diff --git a/src/wrapper.h b/src/wrapper.h index ed9dea6..e4e5551 100644 --- a/src/wrapper.h +++ b/src/wrapper.h @@ -30,4 +30,6 @@ void ext_php_rs_set_known_valid_utf8(zend_string *zs); const char *ext_php_rs_php_build_id(); void *ext_php_rs_zend_object_alloc(size_t obj_size, zend_class_entry *ce); void ext_php_rs_zend_object_release(zend_object *obj); -zend_executor_globals *ext_php_rs_executor_globals(); \ No newline at end of file +zend_executor_globals *ext_php_rs_executor_globals(); +bool ext_php_rs_zend_try_catch(void* (*callback)(void *), void *ctx, void **result); +void ext_php_rs_zend_bailout(); diff --git a/src/zend/mod.rs b/src/zend/mod.rs index b3b1cfb..af8a5c2 100644 --- a/src/zend/mod.rs +++ b/src/zend/mod.rs @@ -9,6 +9,7 @@ mod globals; mod handlers; mod ini_entry_def; mod module; +mod try_catch; use crate::{error::Result, ffi::php_printf}; use std::ffi::CString; @@ -22,6 +23,9 @@ pub use globals::ExecutorGlobals; pub use handlers::ZendObjectHandlers; pub use ini_entry_def::IniEntryDef; pub use module::ModuleEntry; +#[cfg(feature = "embed")] +pub(crate) use try_catch::panic_wrapper; +pub use try_catch::{bailout, try_catch}; // Used as the format string for `php_printf`. const FORMAT_STR: &[u8] = b"%s\0"; diff --git a/src/zend/try_catch.rs b/src/zend/try_catch.rs new file mode 100644 index 0000000..f74b427 --- /dev/null +++ b/src/zend/try_catch.rs @@ -0,0 +1,164 @@ +use crate::ffi::{ext_php_rs_zend_bailout, ext_php_rs_zend_try_catch}; +use std::ffi::c_void; +use std::panic::{catch_unwind, resume_unwind, RefUnwindSafe}; +use std::ptr::null_mut; + +#[derive(Debug)] +pub struct CatchError; + +pub(crate) unsafe extern "C" fn panic_wrapper R + RefUnwindSafe>( + ctx: *const c_void, +) -> *const c_void { + // we try to catch panic here so we correctly shutdown php if it happens + // mandatory when we do assert on test as other test would not run correctly + let panic = catch_unwind(|| (*(ctx as *mut F))()); + + Box::into_raw(Box::new(panic)) as *mut c_void +} + +/// PHP propose a try catch mechanism in C using setjmp and longjmp (bailout) +/// It store the arg of setjmp into the bailout field of the global executor +/// If a bailout is triggered, the executor will jump to the setjmp and restore the previous setjmp +/// +/// try_catch allow to use this mechanism +/// +/// # Returns +/// +/// * `Ok(R)` - The result of the function +/// * `Err(CatchError)` - A bailout occurred during the execution +pub fn try_catch R + RefUnwindSafe>(func: F) -> Result { + let mut panic_ptr = null_mut(); + let has_bailout = unsafe { + ext_php_rs_zend_try_catch( + panic_wrapper::, + &func as *const F as *const c_void, + (&mut panic_ptr) as *mut *mut c_void, + ) + }; + + let panic = panic_ptr as *mut std::thread::Result; + + // can be null if there is a bailout + if panic.is_null() || has_bailout { + return Err(CatchError); + } + + match unsafe { *Box::from_raw(panic as *mut std::thread::Result) } { + Ok(r) => Ok(r), + Err(err) => { + // we resume the panic here so it can be catched correctly by the test framework + resume_unwind(err); + } + } +} + +/// Trigger a bailout +/// +/// This function will stop the execution of the current script +/// and jump to the last try catch block +/// +/// # Safety +/// +/// This function is unsafe because it can cause memory leaks +/// Since it will jump to the last try catch block, it will not call the destructor of the current scope +/// +/// When using this function you should ensure that all the memory allocated in the current scope is released +/// +pub unsafe fn bailout() -> ! { + ext_php_rs_zend_bailout(); +} + +#[cfg(feature = "embed")] +#[cfg(test)] +mod tests { + use crate::embed::Embed; + use crate::zend::{bailout, try_catch}; + use std::ptr::null_mut; + + #[test] + fn test_catch() { + Embed::run(|| { + let catch = try_catch(|| { + unsafe { + bailout(); + } + + #[allow(unreachable_code)] + { + assert!(false); + } + }); + + assert!(catch.is_err()); + }); + } + + #[test] + fn test_no_catch() { + Embed::run(|| { + let catch = try_catch(|| { + assert!(true); + }); + + assert!(catch.is_ok()); + }); + } + + #[test] + fn test_bailout() { + Embed::run(|| { + unsafe { + bailout(); + } + + #[allow(unreachable_code)] + { + assert!(false); + } + }); + } + + #[test] + #[should_panic] + fn test_panic() { + Embed::run(|| { + let _ = try_catch(|| { + panic!("should panic"); + }); + }); + } + + #[test] + fn test_return() { + let foo = Embed::run(|| { + let result = try_catch(|| { + return "foo"; + }); + + assert!(result.is_ok()); + + result.unwrap() + }); + + assert_eq!(foo, "foo"); + } + + #[test] + fn test_memory_leak() { + let mut ptr = null_mut(); + + let _ = try_catch(|| { + let mut result = "foo".to_string(); + ptr = &mut result; + + unsafe { + bailout(); + } + }); + + // Check that the string is never released + let result = unsafe { &*ptr as &str }; + + assert_eq!(result, "foo"); + } +}