From 7d2b74f17e94c00d891c6b5d6d7cb11a357cbfd5 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 8 Dec 2023 17:44:03 +0100 Subject: [PATCH 1/2] Fix GH-12905: FFI::new interacts badly with observers Because these functions are copied and not properly registered (which we can't), the observer code doesn't add the temporaries on startup. Add them via a callback during startup. --- ext/ffi/ffi.c | 17 +++++++++++++++++ ext/ffi/tests/gh12905.phpt | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 ext/ffi/tests/gh12905.phpt diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index 9be5ac340590c..5d2104c5f2a5c 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -26,6 +26,7 @@ #include "zend_closures.h" #include "zend_weakrefs.h" #include "main/SAPI.h" +#include "zend_observer.h" #include @@ -5301,6 +5302,19 @@ static zend_result zend_ffi_preload(char *preload) /* {{{ */ } /* }}} */ +static zend_result (*prev_zend_post_startup_cb)(void); +static zend_result ffi_fixup_temporaries(void) { + if (ZEND_OBSERVER_ENABLED) { + ++zend_ffi_new_fn.T; + ++zend_ffi_cast_fn.T; + ++zend_ffi_type_fn.T; + } + if (prev_zend_post_startup_cb) { + return prev_zend_post_startup_cb(); + } + return SUCCESS; +} + /* {{{ ZEND_MINIT_FUNCTION */ ZEND_MINIT_FUNCTION(ffi) { @@ -5322,6 +5336,9 @@ ZEND_MINIT_FUNCTION(ffi) memcpy(&zend_ffi_type_fn, zend_hash_str_find_ptr(&zend_ffi_ce->function_table, "type", sizeof("type")-1), sizeof(zend_internal_function)); zend_ffi_type_fn.fn_flags &= ~ZEND_ACC_STATIC; + prev_zend_post_startup_cb = zend_post_startup_cb; + zend_post_startup_cb = ffi_fixup_temporaries; + memcpy(&zend_ffi_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers)); zend_ffi_handlers.get_constructor = zend_fake_get_constructor; zend_ffi_handlers.free_obj = zend_ffi_free_obj; diff --git a/ext/ffi/tests/gh12905.phpt b/ext/ffi/tests/gh12905.phpt new file mode 100644 index 0000000000000..adcc32db2907c --- /dev/null +++ b/ext/ffi/tests/gh12905.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-12905 (FFI::new interacts badly with observers) +--EXTENSIONS-- +ffi +zend_test +--SKIPIF-- + +--INI-- +ffi.enable=1 +zend_test.observer.enabled=1 +zend_test.observer.observe_all=1 +zend_test.observer.show_return_value=0 +--FILE-- +new("int"); +?> +--EXPECTF-- + + + + + + + + + From 23462010006c0be3e0d041877f16008304405300 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 11 Dec 2023 17:53:10 +0100 Subject: [PATCH 2/2] [ci skip] Add comment --- ext/ffi/ffi.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index 5d2104c5f2a5c..d4bf6a13ac845 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -5302,6 +5302,12 @@ static zend_result zend_ffi_preload(char *preload) /* {{{ */ } /* }}} */ +/* The startup code for observers adds a temporary to each function for internal use. + * The "new", "cast", and "type" functions in FFI are both static and non-static. + * Only the static versions are in the function table and the non-static versions are not. + * This means the non-static versions will be skipped by the observers startup code. + * This function fixes that by incrementing the temporary count for the non-static versions. + */ static zend_result (*prev_zend_post_startup_cb)(void); static zend_result ffi_fixup_temporaries(void) { if (ZEND_OBSERVER_ENABLED) {