From 314ab47e55845862be62683e331aa247025d073d Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 26 May 2020 14:06:36 +0200 Subject: Fix zend_assign_to_typed_ref() implementation There was some confusion going on here regarding the original value vs the copied value. I've dropped the needs_copy variable, because this code is not inlined, so it would always be true anyway. What we need to do is perform a move-assignment of the copied value (in which case we don't care about performing the assignment before destroying garbage), and destroying the original value for the VAR/TMP cases. This is a bit complicated by the fact that references are passed in via a separate ref variable, so we can't just ptr_dtor the original variable. --- Zend/zend_execute.c | 60 +++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 32 deletions(-) (limited to 'Zend/zend_execute.c') diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index ef4c829ce6..cf28635df9 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -3168,45 +3168,41 @@ ZEND_API zend_bool ZEND_FASTCALL zend_verify_ref_assignable_zval(zend_reference return 1; } -ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *value, zend_uchar value_type, zend_bool strict, zend_refcounted *ref) -{ - zend_bool need_copy = ZEND_CONST_COND(value_type & (IS_CONST|IS_CV), 1) || - ((value_type & IS_VAR) && UNEXPECTED(ref) && GC_REFCOUNT(ref) > 1); - zend_bool ret; - zval tmp; - - if (need_copy) { - ZVAL_COPY(&tmp, value); - value = &tmp; - } - ret = zend_verify_ref_assignable_zval(Z_REF_P(variable_ptr), value, strict); - if (need_copy) { - Z_TRY_DELREF_P(value); - } - if (!ret) { - if (value_type & (IS_VAR|IS_TMP_VAR)) { - zval_ptr_dtor(value); +static zend_always_inline void i_zval_ptr_dtor_noref(zval *zval_ptr) { + if (Z_REFCOUNTED_P(zval_ptr)) { + zend_refcounted *ref = Z_COUNTED_P(zval_ptr); + ZEND_ASSERT(Z_TYPE_P(zval_ptr) != IS_REFERENCE); + if (!GC_DELREF(ref)) { + rc_dtor_func(ref); + } else if (UNEXPECTED(GC_MAY_LEAK(ref))) { + gc_possible_root(ref); } - return Z_REFVAL_P(variable_ptr); } +} +ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, zend_uchar value_type, zend_bool strict, zend_refcounted *ref) +{ + zend_bool ret; + zval value; + ZVAL_COPY(&value, orig_value); + ret = zend_verify_ref_assignable_zval(Z_REF_P(variable_ptr), &value, strict); variable_ptr = Z_REFVAL_P(variable_ptr); - if (EXPECTED(Z_REFCOUNTED_P(variable_ptr))) { - zend_refcounted *garbage = Z_COUNTED_P(variable_ptr); - - zend_copy_to_variable(variable_ptr, value, value_type, ref); - if (GC_DELREF(garbage) == 0) { - rc_dtor_func(garbage); - } else { /* we need to split */ - /* optimized version of GC_ZVAL_CHECK_POSSIBLE_ROOT(variable_ptr) */ - if (UNEXPECTED(GC_MAY_LEAK(garbage))) { - gc_possible_root(garbage); + if (EXPECTED(ret)) { + i_zval_ptr_dtor_noref(variable_ptr); + ZVAL_COPY_VALUE(variable_ptr, &value); + } else { + zval_ptr_dtor_nogc(&value); + } + if (value_type & (IS_VAR|IS_TMP_VAR)) { + if (UNEXPECTED(ref)) { + if (UNEXPECTED(GC_DELREF(ref) == 0)) { + zval_ptr_dtor(orig_value); + efree_size(ref, sizeof(zend_reference)); } + } else { + i_zval_ptr_dtor_noref(orig_value); } - return variable_ptr; } - - zend_copy_to_variable(variable_ptr, value, value_type, ref); return variable_ptr; } -- cgit v1.2.1