Skip to content

Fix readonly+clone JIT issues #10748

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix readonly+clone JIT issues #10748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee15caf
Fix readonly+clone JIT issues
iluuu1994 Mar 1, 2023
19b723b
Simplify assembly by reusing hot path
iluuu1994 Mar 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions Zend/tests/readonly_props/readonly_clone_error4.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
--TEST--
Readonly property cannot be op-assigned twice during cloning
--FILE--
<?php

class Foo {
public function __construct(
public readonly int $bar
) {}

public function __clone()
{
$this->bar += 2;
var_dump($this);
$this->bar += 3;
}
}

$foo = new Foo(1);

try {
clone $foo;
} catch (Error $exception) {
echo $exception->getMessage() . "\n";
}

try {
clone $foo;
} catch (Error $exception) {
echo $exception->getMessage() . "\n";
}

?>
--EXPECT--
object(Foo)#2 (1) {
["bar"]=>
int(3)
}
Cannot modify readonly property Foo::$bar
object(Foo)#2 (1) {
["bar"]=>
int(3)
}
Cannot modify readonly property Foo::$bar
76 changes: 76 additions & 0 deletions Zend/tests/readonly_props/readonly_clone_error5.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
--TEST--
Readonly property clone indirect variation for JIT
--FILE--
<?php

trait CloneSetOnceTrait {
public function __clone() {
$this->prop[] = 1;
}
}

trait CloneSetTwiceTrait {
public function __clone() {
$this->prop[] = 1;
$this->prop[] = 2;
}
}

class TestSetOnce {
use CloneSetOnceTrait;
public readonly array $prop;
public function __construct() {
$this->prop = [];
}
}

class TestSetTwice {
use CloneSetTwiceTrait;
public readonly array $prop;
public function __construct() {
$this->prop = [];
}
}

try {
var_dump(clone (new TestSetOnce()));
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(clone (new TestSetOnce()));
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(clone (new TestSetTwice()));
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(clone (new TestSetTwice()));
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

?>
--EXPECT--
object(TestSetOnce)#2 (1) {
["prop"]=>
array(1) {
[0]=>
int(1)
}
}
object(TestSetOnce)#1 (1) {
["prop"]=>
array(1) {
[0]=>
int(1)
}
}
Cannot modify readonly property TestSetTwice::$prop
Cannot modify readonly property TestSetTwice::$prop
19 changes: 19 additions & 0 deletions ext/opcache/jit/zend_jit_arm64.dasc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12242,6 +12242,17 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
| SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX, TMP1w, TMP2
| b >9
|2:
| ldr REG0w, [FCARG1x, #offsetof(zval, u2.extra)]
| TST_32_WITH_CONST REG0w, IS_PROP_REINITABLE, TMP1w
| beq >6
| and REG0w, REG0w, #(~IS_PROP_REINITABLE)
| str REG0w, [FCARG1x, #offsetof(zval, u2.extra)]
if (flags) {
| b >3
} else {
| b >4
}
|6:
| mov FCARG1x, FCARG2x
| SET_EX_OPLINE opline, REG0
| EXT_CALL zend_readonly_property_modification_error, REG0
Expand Down Expand Up @@ -12295,12 +12306,20 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
| b >9
|.cold_code
|4:
| ldr REG0w, [FCARG1x, #(prop_info->offset + offsetof(zval, u2.extra))]
| TST_32_WITH_CONST REG0w, IS_PROP_REINITABLE, TMP1w
| beq >6
| and REG0w, REG0w, #(~IS_PROP_REINITABLE)
| str REG0w, [FCARG1x, #(prop_info->offset + offsetof(zval, u2.extra))]
| b >4
|6:
| LOAD_ADDR FCARG1x, prop_info
| SET_EX_OPLINE opline, REG0
| EXT_CALL zend_readonly_property_modification_error, REG0
| SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR, TMP1w, TMP2
| b >9
|.code
|4:
}
if (opline->opcode == ZEND_FETCH_OBJ_W
&& (opline->extended_value & ZEND_FETCH_OBJ_FLAGS)
Expand Down
29 changes: 23 additions & 6 deletions ext/opcache/jit/zend_jit_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2513,7 +2513,7 @@ static void ZEND_FASTCALL zend_jit_assign_to_typed_prop(zval *property_val, zend
value = &EG(uninitialized_zval);
}

if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) {
if (UNEXPECTED((info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE))) {
zend_readonly_property_modification_error(info);
if (result) {
ZVAL_UNDEF(result);
Expand All @@ -2532,6 +2532,8 @@ static void ZEND_FASTCALL zend_jit_assign_to_typed_prop(zval *property_val, zend
return;
}

Z_PROP_FLAG_P(property_val) &= ~IS_PROP_REINITABLE;

value = zend_assign_to_variable(property_val, &tmp, IS_TMP_VAR, EX_USES_STRICT_TYPES());
if (result) {
ZVAL_COPY_DEREF(result, value);
Expand Down Expand Up @@ -2570,7 +2572,7 @@ static void ZEND_FASTCALL zend_jit_assign_op_to_typed_prop(zval *zptr, zend_prop
zend_execute_data *execute_data = EG(current_execute_data);
zval z_copy;

if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) {
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(zptr) & IS_PROP_REINITABLE))) {
zend_readonly_property_modification_error(prop_info);
return;
}
Expand All @@ -2585,6 +2587,7 @@ static void ZEND_FASTCALL zend_jit_assign_op_to_typed_prop(zval *zptr, zend_prop

binary_op(&z_copy, zptr, value);
if (EXPECTED(zend_verify_property_type(prop_info, &z_copy, EX_USES_STRICT_TYPES()))) {
Z_PROP_FLAG_P(zptr) &= ~IS_PROP_REINITABLE;
zval_ptr_dtor(zptr);
ZVAL_COPY_VALUE(zptr, &z_copy);
} else {
Expand Down Expand Up @@ -2664,7 +2667,7 @@ static void ZEND_FASTCALL zend_jit_inc_typed_prop(zval *var_ptr, zend_property_i
{
ZEND_ASSERT(Z_TYPE_P(var_ptr) != IS_UNDEF);

if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY))) {
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(var_ptr) & IS_PROP_REINITABLE))) {
zend_readonly_property_modification_error(prop_info);
return;
}
Expand All @@ -2681,11 +2684,14 @@ static void ZEND_FASTCALL zend_jit_inc_typed_prop(zval *var_ptr, zend_property_i
if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
zend_long val = _zend_jit_throw_inc_prop_error(prop_info);
ZVAL_LONG(var_ptr, val);
} else {
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
}
} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
zval_ptr_dtor(var_ptr);
ZVAL_COPY_VALUE(var_ptr, &tmp);
} else {
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
zval_ptr_dtor(&tmp);
}
}
Expand All @@ -2694,7 +2700,7 @@ static void ZEND_FASTCALL zend_jit_dec_typed_prop(zval *var_ptr, zend_property_i
{
ZEND_ASSERT(Z_TYPE_P(var_ptr) != IS_UNDEF);

if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY))) {
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(var_ptr) & IS_PROP_REINITABLE))) {
zend_readonly_property_modification_error(prop_info);
return;
}
Expand All @@ -2711,11 +2717,14 @@ static void ZEND_FASTCALL zend_jit_dec_typed_prop(zval *var_ptr, zend_property_i
if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
zend_long val = _zend_jit_throw_dec_prop_error(prop_info);
ZVAL_LONG(var_ptr, val);
} else {
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
}
} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
zval_ptr_dtor(var_ptr);
ZVAL_COPY_VALUE(var_ptr, &tmp);
} else {
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
zval_ptr_dtor(&tmp);
}
}
Expand All @@ -2738,7 +2747,7 @@ static void ZEND_FASTCALL zend_jit_post_inc_typed_prop(zval *var_ptr, zend_prope
{
ZEND_ASSERT(Z_TYPE_P(var_ptr) != IS_UNDEF);

if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY))) {
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(var_ptr) & IS_PROP_REINITABLE))) {
zend_readonly_property_modification_error(prop_info);
if (result) {
ZVAL_UNDEF(result);
Expand All @@ -2757,19 +2766,23 @@ static void ZEND_FASTCALL zend_jit_post_inc_typed_prop(zval *var_ptr, zend_prope
if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
zend_long val = _zend_jit_throw_inc_prop_error(prop_info);
ZVAL_LONG(var_ptr, val);
} else {
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
}
} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
zval_ptr_dtor(var_ptr);
ZVAL_COPY_VALUE(var_ptr, result);
ZVAL_UNDEF(result);
} else {
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
}
}

static void ZEND_FASTCALL zend_jit_post_dec_typed_prop(zval *var_ptr, zend_property_info *prop_info, zval *result)
{
ZEND_ASSERT(Z_TYPE_P(var_ptr) != IS_UNDEF);

if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY))) {
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(var_ptr) & IS_PROP_REINITABLE))) {
zend_readonly_property_modification_error(prop_info);
if (result) {
ZVAL_UNDEF(result);
Expand All @@ -2788,11 +2801,15 @@ static void ZEND_FASTCALL zend_jit_post_dec_typed_prop(zval *var_ptr, zend_prope
if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
zend_long val = _zend_jit_throw_dec_prop_error(prop_info);
ZVAL_LONG(var_ptr, val);
} else {
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
}
} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
zval_ptr_dtor(var_ptr);
ZVAL_COPY_VALUE(var_ptr, result);
ZVAL_UNDEF(result);
} else {
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
}
}

Expand Down
21 changes: 20 additions & 1 deletion ext/opcache/jit/zend_jit_x86.dasc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12983,6 +12983,17 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
| SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX
| jmp >9
|2:
| mov eax, dword [FCARG1a + offsetof(zval, u2.extra)]
| test eax, IS_PROP_REINITABLE
| jz >6
| and eax, ~IS_PROP_REINITABLE
| mov dword [FCARG1a + offsetof(zval, u2.extra)], eax
if (flags) {
| jmp >3
} else {
| jmp >4
}
|6:
| mov FCARG1a, FCARG2a
| SET_EX_OPLINE opline, r0
| EXT_CALL zend_readonly_property_modification_error, r0
Expand Down Expand Up @@ -13034,7 +13045,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
if (opline->opcode == ZEND_FETCH_OBJ_W && (prop_info->flags & ZEND_ACC_READONLY)) {
if (!type_loaded) {
type_loaded = 1;
| mov edx, dword [FCARG1a + prop_info->offset + 8]
| mov edx, dword [FCARG1a + prop_info->offset + offsetof(zval, u1.type_info)]
}
| IF_NOT_TYPE dl, IS_OBJECT, >4
| GET_ZVAL_PTR r0, prop_addr
Expand All @@ -13044,12 +13055,20 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
| jmp >9
|.cold_code
|4:
| mov eax, dword [FCARG1a + prop_info->offset + offsetof(zval, u2.extra)]
| test eax, IS_PROP_REINITABLE
| jz >6
| and eax, ~IS_PROP_REINITABLE
| mov dword [FCARG1a + prop_info->offset + offsetof(zval, u2.extra)], eax
| jmp >4
|6:
| LOAD_ADDR FCARG1a, prop_info
| SET_EX_OPLINE opline, r0
| EXT_CALL zend_readonly_property_modification_error, r0
| SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR
| jmp >9
|.code
|4:
}
if (opline->opcode == ZEND_FETCH_OBJ_W
&& (opline->extended_value & ZEND_FETCH_OBJ_FLAGS)
Expand Down