From 60c65ffb530162eac28b7ec4fcee0c7b4161aa6d Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 6 Oct 2023 18:07:37 +0100 Subject: [PATCH 1/3] Zend: Add ZPP F type check for callables that do not free trampolines As refetching it with the new FCC API does get tedious --- UPGRADING.INTERNALS | 5 +++++ Zend/zend_API.c | 13 ++++++++----- Zend/zend_API.h | 28 ++++++++++++++++++---------- docs/parameter-parsing-api.md | 10 ++++++++-- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 43050396876b0..9488e9843d364 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -19,6 +19,11 @@ PHP 8.4 INTERNALS UPGRADE NOTES to do this at the call site anymore. Writing the handle should happen after successful registration. +* ZPP now accepts a F or Z_PARAM_FUNC_NO_TRAMPOLINE_FREE type check. + This is identical to the 'f' or Z_PARAM_FUNC type check, except the FCC is + always initialized because it doesn't free trampolines. + Trampolines MUST be freed using zend_release_fcall_info_cache() or consumed. + ======================== 2. Build system changes ======================== diff --git a/Zend/zend_API.c b/Zend/zend_API.c index c5273568f7f1e..82a56f5b6abc6 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -981,6 +981,7 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec } break; + case 'F': case 'f': { zend_fcall_info *fci = va_arg(*va, zend_fcall_info *); @@ -995,10 +996,12 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec if (zend_fcall_info_init(arg, 0, fci, fcc, NULL, &is_callable_error) == SUCCESS) { ZEND_ASSERT(!is_callable_error); - /* Release call trampolines: The function may not get called, in which case - * the trampoline will leak. Force it to be refetched during - * zend_call_function instead. */ - zend_release_fcall_info_cache(fcc); + if (c == 'f') { + /* Release call trampolines: The function may not get called, in which case + * the trampoline will leak. Force it to be refetched during + * zend_call_function instead. */ + zend_release_fcall_info_cache(fcc); + } break; } @@ -1109,7 +1112,7 @@ static zend_result zend_parse_va_args(uint32_t num_args, const char *type_spec, case 'o': case 'O': case 'z': case 'Z': case 'C': case 'h': - case 'f': case 'A': + case 'f': case 'F': case 'A': case 'H': case 'p': case 'S': case 'P': case 'L': case 'n': diff --git a/Zend/zend_API.h b/Zend/zend_API.h index 0bcce3ceb6e4f..097a4c8ce04a5 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -1794,9 +1794,9 @@ ZEND_API ZEND_COLD void zend_argument_value_error(uint32_t arg_num, const char * Z_PARAM_DOUBLE_EX(dest, is_null, 1, 0) /* old "f" */ -#define Z_PARAM_FUNC_EX(dest_fci, dest_fcc, check_null, deref) \ +#define Z_PARAM_FUNC_EX(dest_fci, dest_fcc, check_null, deref, free_trampoline) \ Z_PARAM_PROLOGUE(deref, 0); \ - if (UNEXPECTED(!zend_parse_arg_func(_arg, &dest_fci, &dest_fcc, check_null, &_error))) { \ + if (UNEXPECTED(!zend_parse_arg_func(_arg, &dest_fci, &dest_fcc, check_null, &_error, free_trampoline))) { \ if (!_error) { \ _expected_type = check_null ? Z_EXPECTED_FUNC_OR_NULL : Z_EXPECTED_FUNC; \ _error_code = ZPP_ERROR_WRONG_ARG; \ @@ -1807,13 +1807,19 @@ ZEND_API ZEND_COLD void zend_argument_value_error(uint32_t arg_num, const char * } \ #define Z_PARAM_FUNC(dest_fci, dest_fcc) \ - Z_PARAM_FUNC_EX(dest_fci, dest_fcc, 0, 0) + Z_PARAM_FUNC_EX(dest_fci, dest_fcc, 0, 0, true) + +#define Z_PARAM_FUNC_NO_TRAMPOLINE_FREE(dest_fci, dest_fcc) \ + Z_PARAM_FUNC_EX(dest_fci, dest_fcc, 0, 0, false) #define Z_PARAM_FUNC_OR_NULL(dest_fci, dest_fcc) \ - Z_PARAM_FUNC_EX(dest_fci, dest_fcc, 1, 0) + Z_PARAM_FUNC_EX(dest_fci, dest_fcc, 1, 0, true) + +#define Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(dest_fci, dest_fcc) \ + Z_PARAM_FUNC_EX(dest_fci, dest_fcc, 1, 0, false) #define Z_PARAM_FUNC_OR_NULL_WITH_ZVAL(dest_fci, dest_fcc, dest_zp) \ - Z_PARAM_FUNC_EX(dest_fci, dest_fcc, 1, 0) \ + Z_PARAM_FUNC_EX(dest_fci, dest_fcc, 1, 0, true) \ Z_PARAM_GET_PREV_ZVAL(dest_zp) /* old "h" */ @@ -2428,7 +2434,7 @@ static zend_always_inline bool zend_parse_arg_resource(zval *arg, zval **dest, b return 1; } -static zend_always_inline bool zend_parse_arg_func(zval *arg, zend_fcall_info *dest_fci, zend_fcall_info_cache *dest_fcc, bool check_null, char **error) +static zend_always_inline bool zend_parse_arg_func(zval *arg, zend_fcall_info *dest_fci, zend_fcall_info_cache *dest_fcc, bool check_null, char **error, bool free_trampoline) { if (check_null && UNEXPECTED(Z_TYPE_P(arg) == IS_NULL)) { dest_fci->size = 0; @@ -2437,10 +2443,12 @@ static zend_always_inline bool zend_parse_arg_func(zval *arg, zend_fcall_info *d } else if (UNEXPECTED(zend_fcall_info_init(arg, 0, dest_fci, dest_fcc, NULL, error) != SUCCESS)) { return 0; } - /* Release call trampolines: The function may not get called, in which case - * the trampoline will leak. Force it to be refetched during - * zend_call_function instead. */ - zend_release_fcall_info_cache(dest_fcc); + if (free_trampoline) { + /* Release call trampolines: The function may not get called, in which case + * the trampoline will leak. Force it to be refetched during + * zend_call_function instead. */ + zend_release_fcall_info_cache(dest_fcc); + } return 1; } diff --git a/docs/parameter-parsing-api.md b/docs/parameter-parsing-api.md index a5a405e6a241a..fae10f2fec8ae 100644 --- a/docs/parameter-parsing-api.md +++ b/docs/parameter-parsing-api.md @@ -70,8 +70,14 @@ A - array or object (zval*) b - boolean (bool) C - class (zend_class_entry*) d - double (double) -f - function or array containing php method call info (returned as - zend_fcall_info and zend_fcall_info_cache) +f - PHP callable containing php function/method call info (returned as + zend_fcall_info and zend_fcall_info_cache). + The FCC may be uninitialized if the callable is a trampoline. +F - PHP callable containing php function/method call info (returned as + zend_fcall_info and zend_fcall_info_cache). + The FCC will *always* be initialized, even if the callable is a trampoline. + A trampoline *must* be consumed or released with + zend_release_fcall_info_cache(). h - array (returned as HashTable*) H - array or HASH_OF(object) (returned as HashTable*) l - long (zend_long) From 90d4232325b026218017b13cb0017f2285256a55 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 6 Oct 2023 18:18:03 +0100 Subject: [PATCH 2/3] ext/libxml: Use new F ZPP modifier --- ext/libxml/libxml.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c index db3f696ce0381..6ddbdff5fb800 100644 --- a/ext/libxml/libxml.c +++ b/ext/libxml/libxml.c @@ -1181,7 +1181,7 @@ PHP_FUNCTION(libxml_set_external_entity_loader) zend_fcall_info_cache fcc; ZEND_PARSE_PARAMETERS_START(1, 1) - Z_PARAM_FUNC_OR_NULL(fci, fcc) + Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(fci, fcc) ZEND_PARSE_PARAMETERS_END(); /* Unset old callback if it's defined */ @@ -1189,12 +1189,6 @@ PHP_FUNCTION(libxml_set_external_entity_loader) zend_fcc_dtor(&LIBXML(entity_loader_callback)); } if (ZEND_FCI_INITIALIZED(fci)) { /* argument not null */ - if (!ZEND_FCC_INITIALIZED(fcc)) { - zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fcc, NULL); - /* Call trampoline has been cleared by zpp. Refetch it, because we want to deal - * with it outselves. It is important that it is not refetched on every call, - * because calls may occur from different scopes. */ - } zend_fcc_dup(&LIBXML(entity_loader_callback), &fcc); } RETURN_TRUE; From c8a2bb2850094726fea0754174004cf6c4ddf761 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 6 Oct 2023 18:18:12 +0100 Subject: [PATCH 3/3] ext/spl: Use new F ZPP modifier --- ext/spl/spl_iterators.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c index 4fcf2b8872722..ab9b79f73c81a 100644 --- a/ext/spl/spl_iterators.c +++ b/ext/spl/spl_iterators.c @@ -1436,15 +1436,9 @@ static spl_dual_it_object* spl_dual_it_construct(INTERNAL_FUNCTION_PARAMETERS, z case DIT_CallbackFilterIterator: case DIT_RecursiveCallbackFilterIterator: { zend_fcall_info fci; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "Of", &zobject, ce_inner, &fci, &intern->u.callback_filter) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "OF", &zobject, ce_inner, &fci, &intern->u.callback_filter) == FAILURE) { return NULL; } - if (!ZEND_FCC_INITIALIZED(intern->u.callback_filter)) { - /* Call trampoline has been cleared by zpp. Refetch it, because we want to deal - * with it outselves. It is important that it is not refetched on every call, - * because calls may occur from different scopes. */ - zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &intern->u.callback_filter, NULL); - } zend_fcc_addref(&intern->u.callback_filter); break; }