From 269d547043c9d3c59d7c9d773eb1ef174dcc5c44 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 8 Apr 2023 21:09:57 +0200 Subject: [PATCH 01/15] PoC --- ext/standard/array.c | 77 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 14 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index dc0d434d1883e..f2f69584022f5 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3864,6 +3864,38 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM } /* }}} */ +static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg) +{ + ZEND_ASSERT(HT_IS_PACKED(Z_ARRVAL_P(arg))); + + /* 2 refs: the CV and the argument */ + if (Z_REFCOUNT_P(arg) != 2) { + return false; + } + /* If it has holes, it might get sequentialized */ + if (!HT_IS_WITHOUT_HOLES(Z_ARRVAL_P(arg))) { + return false; + } + /* Immutable => no modification allowed */ + if (GC_FLAGS(Z_ARRVAL_P(arg)) & IS_ARRAY_IMMUTABLE) { + return false; + } + + const zend_op *call_opline = execute_data->prev_execute_data->opline; + const zend_op *next_opline = call_opline + 1; + zval *var = ZEND_CALL_VAR(execute_data->prev_execute_data, next_opline->op1.var); + + /* Must be an assignment to the same array */ + if (next_opline->opcode != ZEND_ASSIGN || next_opline->op2.var != call_opline->result.var || Z_ARRVAL_P(arg) != Z_ARRVAL_P(var)) { + return false; + } + + /* Must set the CV to NULL so we don't destroy the array on assignment */ + ZVAL_NULL(var); + + return true; +} + static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMETERS, int recursive) /* {{{ */ { zval *args = NULL; @@ -3925,22 +3957,35 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET arg = args; src = Z_ARRVAL_P(arg); - /* copy first array */ - array_init_size(return_value, count); - dest = Z_ARRVAL_P(return_value); + bool add_ref = false; + /* copy first array if necessary */ if (HT_IS_PACKED(src)) { - zend_hash_real_init_packed(dest); - ZEND_HASH_FILL_PACKED(dest) { - ZEND_HASH_PACKED_FOREACH_VAL(src, src_entry) { - if (UNEXPECTED(Z_ISREF_P(src_entry) && - Z_REFCOUNT_P(src_entry) == 1)) { - src_entry = Z_REFVAL_P(src_entry); - } - Z_TRY_ADDREF_P(src_entry); - ZEND_HASH_FILL_ADD(src_entry); - } ZEND_HASH_FOREACH_END(); - } ZEND_HASH_FILL_END(); + if (prepare_in_place_array_modify_if_possible(execute_data, arg)) { + /* Make RC 1 such that the array may be modified, add_ref will make sure the refcount gets back to 2 at the end */ + GC_DELREF(src); + add_ref = true; + dest = src; + ZVAL_ARR(return_value, dest); + } else { + array_init_size(return_value, count); + dest = Z_ARRVAL_P(return_value); + + zend_hash_real_init_packed(dest); + ZEND_HASH_FILL_PACKED(dest) { + ZEND_HASH_PACKED_FOREACH_VAL(src, src_entry) { + if (UNEXPECTED(Z_ISREF_P(src_entry) && + Z_REFCOUNT_P(src_entry) == 1)) { + src_entry = Z_REFVAL_P(src_entry); + } + Z_TRY_ADDREF_P(src_entry); + ZEND_HASH_FILL_ADD(src_entry); + } ZEND_HASH_FOREACH_END(); + } ZEND_HASH_FILL_END(); + } } else { + array_init_size(return_value, count); + dest = Z_ARRVAL_P(return_value); + zend_string *string_key; zend_hash_real_init_mixed(dest); ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(src, string_key, src_entry) { @@ -3967,6 +4012,10 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET php_array_merge(dest, Z_ARRVAL_P(arg)); } } + + if (add_ref) { + GC_ADDREF(src); + } } /* }}} */ From 1e8bd23e382c3f11c954cb878837fe6f469b19ef Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Apr 2023 13:40:03 +0200 Subject: [PATCH 02/15] More robust checking, and move hole check to caller to allow more generalisation --- ext/standard/array.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index f2f69584022f5..abcc32fe6ba26 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3866,27 +3866,30 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg) { - ZEND_ASSERT(HT_IS_PACKED(Z_ARRVAL_P(arg))); - /* 2 refs: the CV and the argument */ - if (Z_REFCOUNT_P(arg) != 2) { - return false; - } - /* If it has holes, it might get sequentialized */ - if (!HT_IS_WITHOUT_HOLES(Z_ARRVAL_P(arg))) { + if (Z_REFCOUNT_P(arg) != 2) { // TODO: ook RC1 toelaten maar met refcount caveat return false; } /* Immutable => no modification allowed */ if (GC_FLAGS(Z_ARRVAL_P(arg)) & IS_ARRAY_IMMUTABLE) { return false; } + /* Potentially possible with fake frames during optimization */ + if (UNEXPECTED(!execute_data->prev_execute_data)) { + return false; + } const zend_op *call_opline = execute_data->prev_execute_data->opline; const zend_op *next_opline = call_opline + 1; - zval *var = ZEND_CALL_VAR(execute_data->prev_execute_data, next_opline->op1.var); - /* Must be an assignment to the same array */ - if (next_opline->opcode != ZEND_ASSIGN || next_opline->op2.var != call_opline->result.var || Z_ARRVAL_P(arg) != Z_ARRVAL_P(var)) { + /* Must be an assignment from the result of the call to a CV */ + if (next_opline->opcode != ZEND_ASSIGN || next_opline->op1_type != IS_CV || next_opline->op2.var != call_opline->result.var) { + return false; + } + + /* Must be an assignment to the same array as the input */ + zval *var = ZEND_CALL_VAR(execute_data->prev_execute_data, next_opline->op1.var); + if (Z_TYPE_P(var) != IS_ARRAY || Z_ARRVAL_P(arg) != Z_ARRVAL_P(var)) { return false; } @@ -3960,7 +3963,8 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET bool add_ref = false; /* copy first array if necessary */ if (HT_IS_PACKED(src)) { - if (prepare_in_place_array_modify_if_possible(execute_data, arg)) { + /* Note: If it has holes, it might get sequentialized */ + if (HT_IS_WITHOUT_HOLES(src) && prepare_in_place_array_modify_if_possible(execute_data, arg)) { /* Make RC 1 such that the array may be modified, add_ref will make sure the refcount gets back to 2 at the end */ GC_DELREF(src); add_ref = true; From c873f699b2e12b7920b87b1817335b0268cc611b Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Apr 2023 13:47:25 +0200 Subject: [PATCH 03/15] Allow RC1 input arrays via the same mechanism --- ext/standard/array.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index abcc32fe6ba26..872230a393148 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3864,10 +3864,12 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM } /* }}} */ -static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg) +static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg, bool *add_ref) { - /* 2 refs: the CV and the argument */ - if (Z_REFCOUNT_P(arg) != 2) { // TODO: ook RC1 toelaten maar met refcount caveat + /* 2 refs: the CV and the argument; or 1 ref for a temporary */ + uint32_t refcount = Z_REFCOUNT_P(arg); + ZEND_ASSERT(refcount > 0); + if (refcount > 2) { return false; } /* Immutable => no modification allowed */ @@ -3887,15 +3889,19 @@ static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *e return false; } - /* Must be an assignment to the same array as the input */ - zval *var = ZEND_CALL_VAR(execute_data->prev_execute_data, next_opline->op1.var); - if (Z_TYPE_P(var) != IS_ARRAY || Z_ARRVAL_P(arg) != Z_ARRVAL_P(var)) { - return false; + if (refcount == 2) { + /* Must be an assignment to the same array as the input */ + zval *var = ZEND_CALL_VAR(execute_data->prev_execute_data, next_opline->op1.var); + if (Z_TYPE_P(var) != IS_ARRAY || Z_ARRVAL_P(arg) != Z_ARRVAL_P(var)) { + return false; + } + /* Must set the CV to NULL so we don't destroy the array on assignment */ + ZVAL_NULL(var); + *add_ref = true; + /* Make RC 1 such that the array may be modified, add_ref will make sure the refcount gets back to 2 at the end */ + GC_DELREF(Z_ARRVAL_P(arg)); } - /* Must set the CV to NULL so we don't destroy the array on assignment */ - ZVAL_NULL(var); - return true; } @@ -3964,10 +3970,7 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET /* copy first array if necessary */ if (HT_IS_PACKED(src)) { /* Note: If it has holes, it might get sequentialized */ - if (HT_IS_WITHOUT_HOLES(src) && prepare_in_place_array_modify_if_possible(execute_data, arg)) { - /* Make RC 1 such that the array may be modified, add_ref will make sure the refcount gets back to 2 at the end */ - GC_DELREF(src); - add_ref = true; + if (HT_IS_WITHOUT_HOLES(src) && prepare_in_place_array_modify_if_possible(execute_data, arg, &add_ref)) { dest = src; ZVAL_ARR(return_value, dest); } else { From e1238cf7826d554535018a016fdd9d1a6f24cfad Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Apr 2023 13:54:28 +0200 Subject: [PATCH 04/15] Rename add_ref to update_refcount --- ext/standard/array.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 872230a393148..c84d77fd8e1cb 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3864,7 +3864,7 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM } /* }}} */ -static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg, bool *add_ref) +static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg, bool *update_refcount) { /* 2 refs: the CV and the argument; or 1 ref for a temporary */ uint32_t refcount = Z_REFCOUNT_P(arg); @@ -3897,8 +3897,8 @@ static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *e } /* Must set the CV to NULL so we don't destroy the array on assignment */ ZVAL_NULL(var); - *add_ref = true; - /* Make RC 1 such that the array may be modified, add_ref will make sure the refcount gets back to 2 at the end */ + *update_refcount = true; + /* Make RC 1 such that the array may be modified, update_refcount will make sure the refcount gets back to 2 at the end */ GC_DELREF(Z_ARRVAL_P(arg)); } @@ -3966,11 +3966,11 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET arg = args; src = Z_ARRVAL_P(arg); - bool add_ref = false; + bool update_refcount = false; /* copy first array if necessary */ if (HT_IS_PACKED(src)) { /* Note: If it has holes, it might get sequentialized */ - if (HT_IS_WITHOUT_HOLES(src) && prepare_in_place_array_modify_if_possible(execute_data, arg, &add_ref)) { + if (HT_IS_WITHOUT_HOLES(src) && prepare_in_place_array_modify_if_possible(execute_data, arg, &update_refcount)) { dest = src; ZVAL_ARR(return_value, dest); } else { @@ -4020,7 +4020,7 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET } } - if (add_ref) { + if (update_refcount) { GC_ADDREF(src); } } From 4cd7e17c53b2bd9a4636d8fdc94e427eb8f2a360 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Apr 2023 14:22:36 +0200 Subject: [PATCH 05/15] Handle RC1 properly --- ext/standard/array.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index c84d77fd8e1cb..8a30424c9ccdf 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3864,32 +3864,35 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM } /* }}} */ -static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg, bool *update_refcount) +/* Returns true if it's possible to do an in-place array modification, preventing a costly copy. + * It also modifies the CV to prevent freeing it upon assigning. */ +static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg) { - /* 2 refs: the CV and the argument; or 1 ref for a temporary */ + /* 2 refs: the CV and the argument; or 1 ref for a temporary passed as argument */ uint32_t refcount = Z_REFCOUNT_P(arg); ZEND_ASSERT(refcount > 0); if (refcount > 2) { return false; } - /* Immutable => no modification allowed */ - if (GC_FLAGS(Z_ARRVAL_P(arg)) & IS_ARRAY_IMMUTABLE) { - return false; - } - /* Potentially possible with fake frames during optimization */ - if (UNEXPECTED(!execute_data->prev_execute_data)) { + /* Immutable or persistent => no modification allowed */ + if (GC_FLAGS(Z_ARRVAL_P(arg)) & (IS_ARRAY_IMMUTABLE | IS_ARRAY_PERSISTENT)) { return false; } - const zend_op *call_opline = execute_data->prev_execute_data->opline; - const zend_op *next_opline = call_opline + 1; + if (refcount == 2) { + /* Potentially possible with fake frames during optimization */ + if (UNEXPECTED(!execute_data->prev_execute_data)) { + return false; + } - /* Must be an assignment from the result of the call to a CV */ - if (next_opline->opcode != ZEND_ASSIGN || next_opline->op1_type != IS_CV || next_opline->op2.var != call_opline->result.var) { - return false; - } + const zend_op *call_opline = execute_data->prev_execute_data->opline; + const zend_op *next_opline = call_opline + 1; + + /* Must be an assignment from the result of the call to a CV */ + if (next_opline->opcode != ZEND_ASSIGN || next_opline->op1_type != IS_CV || next_opline->op2.var != call_opline->result.var) { + return false; + } - if (refcount == 2) { /* Must be an assignment to the same array as the input */ zval *var = ZEND_CALL_VAR(execute_data->prev_execute_data, next_opline->op1.var); if (Z_TYPE_P(var) != IS_ARRAY || Z_ARRVAL_P(arg) != Z_ARRVAL_P(var)) { @@ -3897,7 +3900,6 @@ static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *e } /* Must set the CV to NULL so we don't destroy the array on assignment */ ZVAL_NULL(var); - *update_refcount = true; /* Make RC 1 such that the array may be modified, update_refcount will make sure the refcount gets back to 2 at the end */ GC_DELREF(Z_ARRVAL_P(arg)); } @@ -3970,7 +3972,8 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET /* copy first array if necessary */ if (HT_IS_PACKED(src)) { /* Note: If it has holes, it might get sequentialized */ - if (HT_IS_WITHOUT_HOLES(src) && prepare_in_place_array_modify_if_possible(execute_data, arg, &update_refcount)) { + if (HT_IS_WITHOUT_HOLES(src) && prepare_in_place_array_modify_if_possible(execute_data, arg)) { + update_refcount = true; dest = src; ZVAL_ARR(return_value, dest); } else { From 7f6d05a0eb7bf0d59e7ac2a45489ca3d60498517 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Apr 2023 14:22:46 +0200 Subject: [PATCH 06/15] Allow in-place array_unique --- ext/standard/array.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 8a30424c9ccdf..da1d7b8dea396 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -4633,7 +4633,13 @@ PHP_FUNCTION(array_unique) cmp = php_get_data_compare_func_unstable(sort_type, 0); - RETVAL_ARR(zend_array_dup(Z_ARRVAL_P(array))); + bool update_refcount = false; + if (prepare_in_place_array_modify_if_possible(execute_data, array)) { + update_refcount = true; + RETVAL_ARR(Z_ARRVAL_P(array)); + } else { + RETVAL_ARR(zend_array_dup(Z_ARRVAL_P(array))); + } /* create and sort array with pointers to the target_hash buckets */ arTmp = pemalloc((Z_ARRVAL_P(array)->nNumOfElements + 1) * sizeof(struct bucketindex), GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT); @@ -4679,6 +4685,10 @@ PHP_FUNCTION(array_unique) } } pefree(arTmp, GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT); + + if (update_refcount) { + GC_ADDREF(Z_ARRVAL_P(array)); + } } /* }}} */ From 77f43fab31e1a7fa6319171191569826c17fe9dc Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Apr 2023 21:14:39 +0200 Subject: [PATCH 07/15] Fix broken comment --- ext/standard/array.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index da1d7b8dea396..b35b6330281a3 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3865,7 +3865,8 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM /* }}} */ /* Returns true if it's possible to do an in-place array modification, preventing a costly copy. - * It also modifies the CV to prevent freeing it upon assigning. */ + * It also modifies the CV to prevent freeing it upon assigning. + * If this returns true you need to add a ref at the end of the modification for the return value. */ static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *execute_data, const zval *arg) { /* 2 refs: the CV and the argument; or 1 ref for a temporary passed as argument */ @@ -3900,7 +3901,7 @@ static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *e } /* Must set the CV to NULL so we don't destroy the array on assignment */ ZVAL_NULL(var); - /* Make RC 1 such that the array may be modified, update_refcount will make sure the refcount gets back to 2 at the end */ + /* Make RC 1 such that the array may be modified */ GC_DELREF(Z_ARRVAL_P(arg)); } From 08d36abde1866a8ddb5ff9a7a3fde56d06830629 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Apr 2023 21:18:27 +0200 Subject: [PATCH 08/15] Add a helper to avoid zend_array_dup if it can happen in-place --- ext/standard/array.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index b35b6330281a3..be752469b45cb 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3908,6 +3908,17 @@ static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *e return true; } +static bool set_return_value_dup_or_in_place(const zend_execute_data *execute_data, const zval *arg, zval *return_value) +{ + if (prepare_in_place_array_modify_if_possible(execute_data, arg)) { + RETVAL_ARR(Z_ARRVAL_P(arg)); + return true; + } else { + RETVAL_ARR(zend_array_dup(Z_ARRVAL_P(arg))); + return false; + } +} + static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMETERS, int recursive) /* {{{ */ { zval *args = NULL; @@ -4634,13 +4645,7 @@ PHP_FUNCTION(array_unique) cmp = php_get_data_compare_func_unstable(sort_type, 0); - bool update_refcount = false; - if (prepare_in_place_array_modify_if_possible(execute_data, array)) { - update_refcount = true; - RETVAL_ARR(Z_ARRVAL_P(array)); - } else { - RETVAL_ARR(zend_array_dup(Z_ARRVAL_P(array))); - } + bool update_refcount = set_return_value_dup_or_in_place(execute_data, array, return_value); /* create and sort array with pointers to the target_hash buckets */ arTmp = pemalloc((Z_ARRVAL_P(array)->nNumOfElements + 1) * sizeof(struct bucketindex), GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT); From 59619a9c963cdbc6417d3415bbe4636ec7f450b2 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 10 Apr 2023 14:58:24 +0200 Subject: [PATCH 09/15] Handle array_replace --- ext/standard/array.c | 83 +++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index be752469b45cb..a8c209dff09b1 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3825,45 +3825,6 @@ PHPAPI int php_array_replace_recursive(HashTable *dest, HashTable *src) /* {{{ * } /* }}} */ -static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAMETERS, int recursive) /* {{{ */ -{ - zval *args = NULL; - zval *arg; - int argc, i; - HashTable *dest; - - ZEND_PARSE_PARAMETERS_START(1, -1) - Z_PARAM_VARIADIC('+', args, argc) - ZEND_PARSE_PARAMETERS_END(); - - - for (i = 0; i < argc; i++) { - zval *arg = args + i; - - if (Z_TYPE_P(arg) != IS_ARRAY) { - zend_argument_type_error(i + 1, "must be of type array, %s given", zend_zval_value_name(arg)); - RETURN_THROWS(); - } - } - - /* copy first array */ - arg = args; - dest = zend_array_dup(Z_ARRVAL_P(arg)); - ZVAL_ARR(return_value, dest); - if (recursive) { - for (i = 1; i < argc; i++) { - arg = args + i; - php_array_replace_recursive(dest, Z_ARRVAL_P(arg)); - } - } else { - for (i = 1; i < argc; i++) { - arg = args + i; - zend_hash_merge(dest, Z_ARRVAL_P(arg), zval_add_ref, 1); - } - } -} -/* }}} */ - /* Returns true if it's possible to do an in-place array modification, preventing a costly copy. * It also modifies the CV to prevent freeing it upon assigning. * If this returns true you need to add a ref at the end of the modification for the return value. */ @@ -3919,6 +3880,50 @@ static bool set_return_value_dup_or_in_place(const zend_execute_data *execute_da } } +static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAMETERS, int recursive) /* {{{ */ +{ + zval *args = NULL; + zval *arg; + int argc, i; + HashTable *dest; + + ZEND_PARSE_PARAMETERS_START(1, -1) + Z_PARAM_VARIADIC('+', args, argc) + ZEND_PARSE_PARAMETERS_END(); + + + for (i = 0; i < argc; i++) { + zval *arg = args + i; + + if (Z_TYPE_P(arg) != IS_ARRAY) { + zend_argument_type_error(i + 1, "must be of type array, %s given", zend_zval_value_name(arg)); + RETURN_THROWS(); + } + } + + /* copy first array if necessary */ + arg = args; + bool update_refcount = set_return_value_dup_or_in_place(execute_data, arg, return_value); + dest = Z_ARRVAL_P(return_value); + + if (recursive) { + for (i = 1; i < argc; i++) { + arg = args + i; + php_array_replace_recursive(dest, Z_ARRVAL_P(arg)); + } + } else { + for (i = 1; i < argc; i++) { + arg = args + i; + zend_hash_merge(dest, Z_ARRVAL_P(arg), zval_add_ref, 1); + } + } + + if (update_refcount) { + GC_ADDREF(dest); + } +} +/* }}} */ + static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMETERS, int recursive) /* {{{ */ { zval *args = NULL; From ffff1681eb0a70e0cf2e951c34ed2edbf0cf1477 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 10 Apr 2023 15:52:32 +0200 Subject: [PATCH 10/15] Allow array_diff in-place --- ext/standard/array.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index a8c209dff09b1..baeae325ff400 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -5209,6 +5209,7 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_ zend_fcall_info *fci_key = NULL, *fci_data; zend_fcall_info_cache *fci_key_cache = NULL, *fci_data_cache; PHP_ARRAY_CMP_FUNC_VARS; + bool in_place = false; bucket_compare_func_t diff_key_compare_func; bucket_compare_func_t diff_data_compare_func; @@ -5293,6 +5294,8 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_ } else if ((behavior & DIFF_ASSOC) && key_compare_type == DIFF_COMP_KEY_USER) { BG(user_compare_fci) = *fci_key; BG(user_compare_fci_cache) = *fci_key_cache; + } else { + in_place = true; } for (i = 0; i < arr_argc; i++) { @@ -5335,8 +5338,12 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_ } } - /* copy the argument array */ - RETVAL_ARR(zend_array_dup(Z_ARRVAL(args[0]))); + /* copy the argument array if necessary */ + if (in_place) { + in_place = set_return_value_dup_or_in_place(execute_data, &args[0], return_value); + } else { + RETVAL_ARR(zend_array_dup(Z_ARRVAL(args[0]))); + } /* go through the lists and look for values of ptr[0] that are not in the others */ while (Z_TYPE(ptrs[0]->val) != IS_UNDEF) { @@ -5445,6 +5452,10 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_ efree(ptrs); efree(lists); + + if (in_place) { + GC_ADDREF(Z_ARRVAL_P(return_value)); + } } /* }}} */ From 24de130c79cbeb64236c633e1fc53cdc8a560354 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 10 Apr 2023 15:54:53 +0200 Subject: [PATCH 11/15] Allow in-place for array_intersect --- ext/standard/array.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index baeae325ff400..78cd989f0b1fc 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -4824,6 +4824,7 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int zend_fcall_info *fci_key = NULL, *fci_data; zend_fcall_info_cache *fci_key_cache = NULL, *fci_data_cache; PHP_ARRAY_CMP_FUNC_VARS; + bool in_place = false; bucket_compare_func_t intersect_key_compare_func; bucket_compare_func_t intersect_data_compare_func; @@ -4908,6 +4909,8 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int } else if ((behavior & INTERSECT_ASSOC) && key_compare_type == INTERSECT_COMP_KEY_USER) { BG(user_compare_fci) = *fci_key; BG(user_compare_fci_cache) = *fci_key_cache; + } else { + in_place = true; } for (i = 0; i < arr_argc; i++) { @@ -4950,8 +4953,12 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int } } - /* copy the argument array */ - RETVAL_ARR(zend_array_dup(Z_ARRVAL(args[0]))); + /* copy the argument array if necessary */ + if (in_place) { + in_place = set_return_value_dup_or_in_place(execute_data, &args[0], return_value); + } else { + RETVAL_ARR(zend_array_dup(Z_ARRVAL(args[0]))); + } /* go through the lists and look for common values */ while (Z_TYPE(ptrs[0]->val) != IS_UNDEF) { From dc38f00d2f4428f27efb7258b7f810fe5309f287 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 10 Apr 2023 17:05:55 +0200 Subject: [PATCH 12/15] Forgot a refcount --- ext/standard/array.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/standard/array.c b/ext/standard/array.c index 78cd989f0b1fc..5e161ea4cb71d 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -5069,6 +5069,10 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int efree(ptrs); efree(lists); + + if (in_place) { + GC_ADDREF(Z_ARRVAL_P(return_value)); + } } /* }}} */ From c7d3e40ea560632260043ac890b89434aba88627 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 10 Apr 2023 17:06:14 +0200 Subject: [PATCH 13/15] Rename set_return_value_dup_or_in_place to set_return_value_array_dup_or_in_place --- ext/standard/array.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 5e161ea4cb71d..f674713c1e9ee 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3869,7 +3869,7 @@ static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *e return true; } -static bool set_return_value_dup_or_in_place(const zend_execute_data *execute_data, const zval *arg, zval *return_value) +static bool set_return_value_array_dup_or_in_place(const zend_execute_data *execute_data, const zval *arg, zval *return_value) { if (prepare_in_place_array_modify_if_possible(execute_data, arg)) { RETVAL_ARR(Z_ARRVAL_P(arg)); @@ -3903,7 +3903,7 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM /* copy first array if necessary */ arg = args; - bool update_refcount = set_return_value_dup_or_in_place(execute_data, arg, return_value); + bool update_refcount = set_return_value_array_dup_or_in_place(execute_data, arg, return_value); dest = Z_ARRVAL_P(return_value); if (recursive) { @@ -4650,7 +4650,7 @@ PHP_FUNCTION(array_unique) cmp = php_get_data_compare_func_unstable(sort_type, 0); - bool update_refcount = set_return_value_dup_or_in_place(execute_data, array, return_value); + bool update_refcount = set_return_value_array_dup_or_in_place(execute_data, array, return_value); /* create and sort array with pointers to the target_hash buckets */ arTmp = pemalloc((Z_ARRVAL_P(array)->nNumOfElements + 1) * sizeof(struct bucketindex), GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT); @@ -4955,7 +4955,7 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int /* copy the argument array if necessary */ if (in_place) { - in_place = set_return_value_dup_or_in_place(execute_data, &args[0], return_value); + in_place = set_return_value_array_dup_or_in_place(execute_data, &args[0], return_value); } else { RETVAL_ARR(zend_array_dup(Z_ARRVAL(args[0]))); } @@ -5351,7 +5351,7 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_ /* copy the argument array if necessary */ if (in_place) { - in_place = set_return_value_dup_or_in_place(execute_data, &args[0], return_value); + in_place = set_return_value_array_dup_or_in_place(execute_data, &args[0], return_value); } else { RETVAL_ARR(zend_array_dup(Z_ARRVAL(args[0]))); } From 43ecf2c4b372693afb0a2f38173e4e143154415a Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 10 Apr 2023 17:11:52 +0200 Subject: [PATCH 14/15] prev_execute_data is always non-NULL --- ext/standard/array.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index f674713c1e9ee..e3b06ea78a72e 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3842,11 +3842,6 @@ static bool prepare_in_place_array_modify_if_possible(const zend_execute_data *e } if (refcount == 2) { - /* Potentially possible with fake frames during optimization */ - if (UNEXPECTED(!execute_data->prev_execute_data)) { - return false; - } - const zend_op *call_opline = execute_data->prev_execute_data->opline; const zend_op *next_opline = call_opline + 1; From cec2458f566f872627c1e25ea9fc30ba50be0af0 Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 10 Apr 2023 17:34:52 +0200 Subject: [PATCH 15/15] Fix --- ext/standard/array.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/standard/array.c b/ext/standard/array.c index e3b06ea78a72e..22afef7e96832 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -4912,6 +4912,7 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int if (Z_TYPE(args[i]) != IS_ARRAY) { zend_argument_type_error(i + 1, "must be of type array, %s given", zend_zval_value_name(&args[i])); arr_argc = i; /* only free up to i - 1 */ + in_place = false; goto out; } hash = Z_ARRVAL(args[i]); @@ -5308,6 +5309,7 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_ if (Z_TYPE(args[i]) != IS_ARRAY) { zend_argument_type_error(i + 1, "must be of type array, %s given", zend_zval_value_name(&args[i])); arr_argc = i; /* only free up to i - 1 */ + in_place = false; goto out; } hash = Z_ARRVAL(args[i]);