Skip to content

Commit 314ab47

Browse files
committed
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.
1 parent fb370ec commit 314ab47

File tree

2 files changed

+54
-32
lines changed

2 files changed

+54
-32
lines changed
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
Assigning stringable object to static string property
3+
--FILE--
4+
5+
6+
class Test1 {
7+
public static $ref;
8+
}
9+
class Test2 {
10+
public string $str = "str";
11+
}
12+
class Test3 {
13+
public function __toString() {
14+
$x = "foo";
15+
return $x . "bar";
16+
}
17+
}
18+
19+
$test2 = new Test2;
20+
Test1::$ref =& $test2->str;
21+
Test1::$ref = new Test3;
22+
var_dump(Test1::$ref);
23+
24+
?>
25+
--EXPECT--
26+
string(6) "foobar"

Zend/zend_execute.c

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3168,45 +3168,41 @@ ZEND_API zend_bool ZEND_FASTCALL zend_verify_ref_assignable_zval(zend_reference
31683168
return 1;
31693169
}
31703170

3171-
ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *value, zend_uchar value_type, zend_bool strict, zend_refcounted *ref)
3172-
{
3173-
zend_bool need_copy = ZEND_CONST_COND(value_type & (IS_CONST|IS_CV), 1) ||
3174-
((value_type & IS_VAR) && UNEXPECTED(ref) && GC_REFCOUNT(ref) > 1);
3175-
zend_bool ret;
3176-
zval tmp;
3177-
3178-
if (need_copy) {
3179-
ZVAL_COPY(&tmp, value);
3180-
value = &tmp;
3181-
}
3182-
ret = zend_verify_ref_assignable_zval(Z_REF_P(variable_ptr), value, strict);
3183-
if (need_copy) {
3184-
Z_TRY_DELREF_P(value);
3185-
}
3186-
if (!ret) {
3187-
if (value_type & (IS_VAR|IS_TMP_VAR)) {
3188-
zval_ptr_dtor(value);
3171+
static zend_always_inline void i_zval_ptr_dtor_noref(zval *zval_ptr) {
3172+
if (Z_REFCOUNTED_P(zval_ptr)) {
3173+
zend_refcounted *ref = Z_COUNTED_P(zval_ptr);
3174+
ZEND_ASSERT(Z_TYPE_P(zval_ptr) != IS_REFERENCE);
3175+
if (!GC_DELREF(ref)) {
3176+
rc_dtor_func(ref);
3177+
} else if (UNEXPECTED(GC_MAY_LEAK(ref))) {
3178+
gc_possible_root(ref);
31893179
}
3190-
return Z_REFVAL_P(variable_ptr);
31913180
}
3181+
}
31923182

3183+
ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, zend_uchar value_type, zend_bool strict, zend_refcounted *ref)
3184+
{
3185+
zend_bool ret;
3186+
zval value;
3187+
ZVAL_COPY(&value, orig_value);
3188+
ret = zend_verify_ref_assignable_zval(Z_REF_P(variable_ptr), &value, strict);
31933189
variable_ptr = Z_REFVAL_P(variable_ptr);
3194-
if (EXPECTED(Z_REFCOUNTED_P(variable_ptr))) {
3195-
zend_refcounted *garbage = Z_COUNTED_P(variable_ptr);
3196-
3197-
zend_copy_to_variable(variable_ptr, value, value_type, ref);
3198-
if (GC_DELREF(garbage) == 0) {
3199-
rc_dtor_func(garbage);
3200-
} else { /* we need to split */
3201-
/* optimized version of GC_ZVAL_CHECK_POSSIBLE_ROOT(variable_ptr) */
3202-
if (UNEXPECTED(GC_MAY_LEAK(garbage))) {
3203-
gc_possible_root(garbage);
3190+
if (EXPECTED(ret)) {
3191+
i_zval_ptr_dtor_noref(variable_ptr);
3192+
ZVAL_COPY_VALUE(variable_ptr, &value);
3193+
} else {
3194+
zval_ptr_dtor_nogc(&value);
3195+
}
3196+
if (value_type & (IS_VAR|IS_TMP_VAR)) {
3197+
if (UNEXPECTED(ref)) {
3198+
if (UNEXPECTED(GC_DELREF(ref) == 0)) {
3199+
zval_ptr_dtor(orig_value);
3200+
efree_size(ref, sizeof(zend_reference));
32043201
}
3202+
} else {
3203+
i_zval_ptr_dtor_noref(orig_value);
32053204
}
3206-
return variable_ptr;
32073205
}
3208-
3209-
zend_copy_to_variable(variable_ptr, value, value_type, ref);
32103206
return variable_ptr;
32113207
}
32123208

0 commit comments

Comments
 (0)