From 4f6fbd98b3c7f1a4963cc626c9844cda3dfdc68d Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Tue, 11 Oct 2022 05:37:25 +0100 Subject: [PATCH 1/5] Do not use zend_fcall_info_argp() for ticks and shutdown functions Instead handle the copying and safe reallocation of the param zvals ourself. --- ext/standard/basic_functions.c | 43 +++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 60d484341a0aa..8dd0a75195a04 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1594,36 +1594,45 @@ PHP_FUNCTION(forward_static_call_array) } /* }}} */ -static void fci_addref(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) +static void php_user_callable_addref_with_params(zend_fcall_info *fci, zend_fcall_info_cache *fcc, const zval *params) { Z_TRY_ADDREF(fci->function_name); - if (fci_cache->object) { - GC_ADDREF(fci_cache->object); + fci->params = NULL; + if (params) { + ZEND_ASSERT(fci->param_count > 0); + fci->params = (zval *) safe_erealloc(fci->params, sizeof(zval), fci->param_count, 0); + for (uint32_t i = 0; i < fci->param_count; ++i) { + ZVAL_COPY(&fci->params[i], ¶ms[i]); + } } + zend_fcc_addref(fcc); } -static void fci_release(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) +static void php_user_callable_dtor_with_params(zend_fcall_info *fci, zend_fcall_info_cache *fcc) { zval_ptr_dtor(&fci->function_name); - if (fci_cache->object) { - zend_object_release(fci_cache->object); + if (fci->params) { + ZEND_ASSERT(fci->param_count > 0); + for (uint32_t i = 0; i < fci->param_count; ++i) { + zval_ptr_dtor(&fci->params[i]); + } + efree(fci->params); } + zend_fcc_dtor(fcc); } void user_shutdown_function_dtor(zval *zv) /* {{{ */ { php_shutdown_function_entry *shutdown_function_entry = Z_PTR_P(zv); - zend_fcall_info_args_clear(&shutdown_function_entry->fci, true); - fci_release(&shutdown_function_entry->fci, &shutdown_function_entry->fci_cache); + php_user_callable_dtor_with_params(&shutdown_function_entry->fci, &shutdown_function_entry->fci_cache); efree(shutdown_function_entry); } /* }}} */ void user_tick_function_dtor(user_tick_function_entry *tick_function_entry) /* {{{ */ { - zend_fcall_info_args_clear(&tick_function_entry->fci, true); - fci_release(&tick_function_entry->fci, &tick_function_entry->fci_cache); + php_user_callable_dtor_with_params(&tick_function_entry->fci, &tick_function_entry->fci_cache); } /* }}} */ @@ -1721,16 +1730,14 @@ PHPAPI void php_free_shutdown_functions(void) /* {{{ */ PHP_FUNCTION(register_shutdown_function) { php_shutdown_function_entry entry; - zval *params = NULL; - uint32_t param_count = 0; bool status; + zval *params = NULL; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "f*", &entry.fci, &entry.fci_cache, ¶ms, ¶m_count) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "f*", &entry.fci, &entry.fci_cache, ¶ms, &entry.fci.param_count) == FAILURE) { RETURN_THROWS(); } - fci_addref(&entry.fci, &entry.fci_cache); - zend_fcall_info_argp(&entry.fci, param_count, params); + php_user_callable_addref_with_params(&entry.fci, &entry.fci_cache, params); status = append_user_shutdown_function(&entry); ZEND_ASSERT(status); @@ -2310,15 +2317,13 @@ PHP_FUNCTION(register_tick_function) { user_tick_function_entry tick_fe; zval *params = NULL; - uint32_t param_count = 0; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "f*", &tick_fe.fci, &tick_fe.fci_cache, ¶ms, ¶m_count) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "f*", &tick_fe.fci, &tick_fe.fci_cache, ¶ms, &tick_fe.fci.param_count) == FAILURE) { RETURN_THROWS(); } tick_fe.calling = false; - fci_addref(&tick_fe.fci, &tick_fe.fci_cache); - zend_fcall_info_argp(&tick_fe.fci, param_count, params); + php_user_callable_addref_with_params(&tick_fe.fci, &tick_fe.fci_cache, params); if (!BG(user_tick_functions)) { BG(user_tick_functions) = (zend_llist *) emalloc(sizeof(zend_llist)); From e25a5d11b75f61066db597db397edca3d880b659 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Tue, 11 Oct 2022 04:24:36 +0100 Subject: [PATCH 2/5] Remove zend_fcall_info_call() API The last parameter which is named zval *args, does NOT set the FCI params field. It is expected to be a PHP array which gets looped over to set the arguments which is also a zval pointer... Since PHP 8.0, the named_params field is a more appropriate way of doing this. Also remove zend_fcall_info_args_save() and zend_fcall_info_args_restore() as they were only used in the implementation of zend_fcall_info_call() --- Zend/zend_API.c | 40 ---------------------------------------- Zend/zend_API.h | 14 -------------- 2 files changed, 54 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index d629b7166a58b..bf5af0b172df1 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -4136,23 +4136,6 @@ ZEND_API void zend_fcall_info_args_clear(zend_fcall_info *fci, bool free_mem) /* } /* }}} */ -ZEND_API void zend_fcall_info_args_save(zend_fcall_info *fci, uint32_t *param_count, zval **params) /* {{{ */ -{ - *param_count = fci->param_count; - *params = fci->params; - fci->param_count = 0; - fci->params = NULL; -} -/* }}} */ - -ZEND_API void zend_fcall_info_args_restore(zend_fcall_info *fci, uint32_t param_count, zval *params) /* {{{ */ -{ - zend_fcall_info_args_clear(fci, 1); - fci->param_count = param_count; - fci->params = params; -} -/* }}} */ - ZEND_API zend_result zend_fcall_info_args_ex(zend_fcall_info *fci, zend_function *func, zval *args) /* {{{ */ { zval *arg, *params; @@ -4234,29 +4217,6 @@ ZEND_API void zend_fcall_info_argn(zend_fcall_info *fci, uint32_t argc, ...) /* } /* }}} */ -ZEND_API zend_result zend_fcall_info_call(zend_fcall_info *fci, zend_fcall_info_cache *fcc, zval *retval_ptr, zval *args) /* {{{ */ -{ - zval retval, *org_params = NULL; - uint32_t org_count = 0; - zend_result result; - - fci->retval = retval_ptr ? retval_ptr : &retval; - if (args) { - zend_fcall_info_args_save(fci, &org_count, &org_params); - zend_fcall_info_args(fci, args); - } - result = zend_call_function(fci, fcc); - - if (!retval_ptr && Z_TYPE(retval) != IS_UNDEF) { - zval_ptr_dtor(&retval); - } - if (args) { - zend_fcall_info_args_restore(fci, org_count, org_params); - } - return result; -} -/* }}} */ - ZEND_API void zend_get_callable_zval_from_fcc(const zend_fcall_info_cache *fcc, zval *callable) { if (fcc->closure) { diff --git a/Zend/zend_API.h b/Zend/zend_API.h index acdc5b06b3d52..d29aaad50ed4e 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -701,15 +701,6 @@ ZEND_API zend_result zend_fcall_info_init(zval *callable, uint32_t check_flags, */ ZEND_API void zend_fcall_info_args_clear(zend_fcall_info *fci, bool free_mem); -/** Save current arguments from zend_fcall_info *fci - * params array will be set to NULL - */ -ZEND_API void zend_fcall_info_args_save(zend_fcall_info *fci, uint32_t *param_count, zval **params); - -/** Free arguments connected with zend_fcall_info *fci and set back saved ones. - */ -ZEND_API void zend_fcall_info_args_restore(zend_fcall_info *fci, uint32_t param_count, zval *params); - /** Set or clear the arguments in the zend_call_info struct taking care of * refcount. If args is NULL and arguments are set then those are cleared. */ @@ -734,11 +725,6 @@ ZEND_API void zend_fcall_info_argv(zend_fcall_info *fci, uint32_t argc, va_list */ ZEND_API void zend_fcall_info_argn(zend_fcall_info *fci, uint32_t argc, ...); -/** Call a function using information created by zend_fcall_info_init()/args(). - * If args is given then those replace the argument info in fci is temporarily. - */ -ZEND_API zend_result zend_fcall_info_call(zend_fcall_info *fci, zend_fcall_info_cache *fcc, zval *retval, zval *args); - /* Zend FCC API to store and handle PHP userland functions */ static zend_always_inline bool zend_fcc_equals(const zend_fcall_info_cache* a, const zend_fcall_info_cache* b) { From 43079d5103b86050740224e62187825e833aadbe Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Tue, 11 Oct 2022 04:45:41 +0100 Subject: [PATCH 3/5] Remove zend_fcall_info_argn() and zend_fcall_info_argv() Usage of these function heap allocates the zvals when they can already be used directly with an FCI --- Zend/zend_API.c | 27 --------------------------- Zend/zend_API.h | 12 ------------ 2 files changed, 39 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index bf5af0b172df1..00afb31432164 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -4190,33 +4190,6 @@ ZEND_API void zend_fcall_info_argp(zend_fcall_info *fci, uint32_t argc, zval *ar } /* }}} */ -ZEND_API void zend_fcall_info_argv(zend_fcall_info *fci, uint32_t argc, va_list *argv) /* {{{ */ -{ - zend_fcall_info_args_clear(fci, !argc); - - if (argc) { - zval *arg; - fci->param_count = argc; - fci->params = (zval *) erealloc(fci->params, fci->param_count * sizeof(zval)); - - for (uint32_t i = 0; i < argc; ++i) { - arg = va_arg(*argv, zval *); - ZVAL_COPY(&fci->params[i], arg); - } - } -} -/* }}} */ - -ZEND_API void zend_fcall_info_argn(zend_fcall_info *fci, uint32_t argc, ...) /* {{{ */ -{ - va_list argv; - - va_start(argv, argc); - zend_fcall_info_argv(fci, argc, &argv); - va_end(argv); -} -/* }}} */ - ZEND_API void zend_get_callable_zval_from_fcc(const zend_fcall_info_cache *fcc, zval *callable) { if (fcc->closure) { diff --git a/Zend/zend_API.h b/Zend/zend_API.h index d29aaad50ed4e..d1679317a5e3e 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -713,18 +713,6 @@ ZEND_API zend_result zend_fcall_info_args_ex(zend_fcall_info *fci, zend_function */ ZEND_API void zend_fcall_info_argp(zend_fcall_info *fci, uint32_t argc, zval *argv); -/** Set arguments in the zend_fcall_info struct taking care of refcount. - * If argc is 0 the arguments which are set will be cleared, else pass - * a variable amount of zval** arguments. - */ -ZEND_API void zend_fcall_info_argv(zend_fcall_info *fci, uint32_t argc, va_list *argv); - -/** Set arguments in the zend_fcall_info struct taking care of refcount. - * If argc is 0 the arguments which are set will be cleared, else pass - * a variable amount of zval** arguments. - */ -ZEND_API void zend_fcall_info_argn(zend_fcall_info *fci, uint32_t argc, ...); - /* Zend FCC API to store and handle PHP userland functions */ static zend_always_inline bool zend_fcc_equals(const zend_fcall_info_cache* a, const zend_fcall_info_cache* b) { From 2157d2d260505965fafd59a6c9572ce00330b40a Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 19 Oct 2022 16:21:09 +0100 Subject: [PATCH 4/5] Remove zend_fcall_info_argp() This function reallocates the param zvals on the heap, when in general they can be directly assigned to the FCI --- Zend/zend_API.c | 15 --------------- Zend/zend_API.h | 6 ------ 2 files changed, 21 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 00afb31432164..1b7b5d06866cc 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -4175,21 +4175,6 @@ ZEND_API zend_result zend_fcall_info_args(zend_fcall_info *fci, zval *args) /* { } /* }}} */ -ZEND_API void zend_fcall_info_argp(zend_fcall_info *fci, uint32_t argc, zval *argv) /* {{{ */ -{ - zend_fcall_info_args_clear(fci, !argc); - - if (argc) { - fci->param_count = argc; - fci->params = (zval *) erealloc(fci->params, fci->param_count * sizeof(zval)); - - for (uint32_t i = 0; i < argc; ++i) { - ZVAL_COPY(&fci->params[i], &argv[i]); - } - } -} -/* }}} */ - ZEND_API void zend_get_callable_zval_from_fcc(const zend_fcall_info_cache *fcc, zval *callable) { if (fcc->closure) { diff --git a/Zend/zend_API.h b/Zend/zend_API.h index d1679317a5e3e..1f4d62ba24a9c 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -707,12 +707,6 @@ ZEND_API void zend_fcall_info_args_clear(zend_fcall_info *fci, bool free_mem); ZEND_API zend_result zend_fcall_info_args(zend_fcall_info *fci, zval *args); ZEND_API zend_result zend_fcall_info_args_ex(zend_fcall_info *fci, zend_function *func, zval *args); -/** Set arguments in the zend_fcall_info struct taking care of refcount. - * If argc is 0 the arguments which are set will be cleared, else pass - * a variable amount of zval** arguments. - */ -ZEND_API void zend_fcall_info_argp(zend_fcall_info *fci, uint32_t argc, zval *argv); - /* Zend FCC API to store and handle PHP userland functions */ static zend_always_inline bool zend_fcc_equals(const zend_fcall_info_cache* a, const zend_fcall_info_cache* b) { From 2903fa7dcb40910ad8f0df5d5f267c5c5c5ac217 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 11 Sep 2023 00:08:49 +0100 Subject: [PATCH 5/5] [W.I.P.] Document API removals in UPGRADING.INTERNALS TODO better description of what to do in each case --- UPGRADING.INTERNALS | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index c4ee1db4c769d..daceb631fb986 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -14,6 +14,14 @@ PHP 8.4 INTERNALS UPGRADE NOTES 1. Internal API changes ======================== +* The following FCI APIs have been removed as they were confusing: + - zend_fcall_info_call() + - zend_fcall_info_args_save() + - zend_fcall_info_args_restore() + - zend_fcall_info_argn() + - zend_fcall_info_argv() + - zend_fcall_info_argp() + ======================== 2. Build system changes ========================