Skip to content

Commit 3f7dadf

Browse files
committed
Fix readonly+clone JIT issues
Closes GH-10748
1 parent ff71c9e commit 3f7dadf

8 files changed

+182
-25
lines changed

Zend/tests/readonly_props/readonly_clone_error1.phpt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
--TEST--
22
Readonly property cannot be reset twice during cloning
3-
--SKIPIF--
4-
<?php
5-
if (function_exists('opcache_get_status') && (opcache_get_status()["jit"]["enabled"] ?? false)) {
6-
die('skip Not yet implemented for JIT');
7-
}
8-
?>
93
--FILE--
104
<?php
115

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
Readonly property cannot be op-assigned twice during cloning
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly int $bar
9+
) {}
10+
11+
public function __clone()
12+
{
13+
$this->bar += 2;
14+
var_dump($this);
15+
$this->bar += 3;
16+
}
17+
}
18+
19+
$foo = new Foo(1);
20+
21+
try {
22+
clone $foo;
23+
} catch (Error $exception) {
24+
echo $exception->getMessage() . "\n";
25+
}
26+
27+
try {
28+
clone $foo;
29+
} catch (Error $exception) {
30+
echo $exception->getMessage() . "\n";
31+
}
32+
33+
?>
34+
--EXPECT--
35+
object(Foo)#2 (1) {
36+
["bar"]=>
37+
int(3)
38+
}
39+
Cannot modify readonly property Foo::$bar
40+
object(Foo)#2 (1) {
41+
["bar"]=>
42+
int(3)
43+
}
44+
Cannot modify readonly property Foo::$bar
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
--TEST--
2+
Readonly property clone indirect variation for JIT
3+
--FILE--
4+
<?php
5+
6+
trait CloneSetOnceTrait {
7+
public function __clone() {
8+
$this->prop[] = 1;
9+
}
10+
}
11+
12+
trait CloneSetTwiceTrait {
13+
public function __clone() {
14+
$this->prop[] = 1;
15+
$this->prop[] = 2;
16+
}
17+
}
18+
19+
class TestSetOnce {
20+
use CloneSetOnceTrait;
21+
public readonly array $prop;
22+
public function __construct() {
23+
$this->prop = [];
24+
}
25+
}
26+
27+
class TestSetTwice {
28+
use CloneSetTwiceTrait;
29+
public readonly array $prop;
30+
public function __construct() {
31+
$this->prop = [];
32+
}
33+
}
34+
35+
try {
36+
var_dump(clone (new TestSetOnce()));
37+
} catch (Error $e) {
38+
echo $e->getMessage(), "\n";
39+
}
40+
41+
try {
42+
var_dump(clone (new TestSetOnce()));
43+
} catch (Error $e) {
44+
echo $e->getMessage(), "\n";
45+
}
46+
47+
try {
48+
var_dump(clone (new TestSetTwice()));
49+
} catch (Error $e) {
50+
echo $e->getMessage(), "\n";
51+
}
52+
53+
try {
54+
var_dump(clone (new TestSetTwice()));
55+
} catch (Error $e) {
56+
echo $e->getMessage(), "\n";
57+
}
58+
59+
?>
60+
--EXPECT--
61+
object(TestSetOnce)#2 (1) {
62+
["prop"]=>
63+
array(1) {
64+
[0]=>
65+
int(1)
66+
}
67+
}
68+
object(TestSetOnce)#1 (1) {
69+
["prop"]=>
70+
array(1) {
71+
[0]=>
72+
int(1)
73+
}
74+
}
75+
Cannot modify readonly property TestSetTwice::$prop
76+
Cannot modify readonly property TestSetTwice::$prop

Zend/tests/readonly_props/readonly_clone_success1.phpt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
--TEST--
22
Readonly property can be reset once during cloning
3-
--SKIPIF--
4-
<?php
5-
if (function_exists('opcache_get_status') && (opcache_get_status()["jit"]["enabled"] ?? false)) {
6-
die('skip Not yet implemented for JIT');
7-
}
8-
?>
93
--FILE--
104
<?php
115

Zend/tests/readonly_props/readonly_clone_success3.phpt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
--TEST--
22
__clone() can indirectly modify unlocked readonly properties
3-
--SKIPIF--
4-
<?php
5-
if (function_exists('opcache_get_status') && (opcache_get_status()["jit"]["enabled"] ?? false)) {
6-
die('skip Not yet implemented for JIT');
7-
}
8-
?>
93
--FILE--
104
<?php
115

ext/opcache/jit/zend_jit_arm64.dasc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12242,6 +12242,17 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
1224212242
| SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX, TMP1w, TMP2
1224312243
| b >9
1224412244
|2:
12245+
| ldr REG0w, [FCARG1x, #offsetof(zval, u2.extra)]
12246+
| TST_32_WITH_CONST REG0w, IS_PROP_REINITABLE, TMP1w
12247+
| beq >6
12248+
| and REG0w, REG0w, #(~IS_PROP_REINITABLE)
12249+
| str REG0w, [FCARG1x, #offsetof(zval, u2.extra)]
12250+
if (flags) {
12251+
| b >3
12252+
} else {
12253+
| b >4
12254+
}
12255+
|6:
1224512256
| mov FCARG1x, FCARG2x
1224612257
| SET_EX_OPLINE opline, REG0
1224712258
| EXT_CALL zend_readonly_property_modification_error, REG0
@@ -12295,12 +12306,20 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
1229512306
| b >9
1229612307
|.cold_code
1229712308
|4:
12309+
| ldr REG0w, [FCARG1x, #(prop_info->offset + offsetof(zval, u2.extra))]
12310+
| TST_32_WITH_CONST REG0w, IS_PROP_REINITABLE, TMP1w
12311+
| beq >6
12312+
| and REG0w, REG0w, #(~IS_PROP_REINITABLE)
12313+
| str REG0w, [FCARG1x, #(prop_info->offset + offsetof(zval, u2.extra))]
12314+
| b >4
12315+
|6:
1229812316
| LOAD_ADDR FCARG1x, prop_info
1229912317
| SET_EX_OPLINE opline, REG0
1230012318
| EXT_CALL zend_readonly_property_modification_error, REG0
1230112319
| SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR, TMP1w, TMP2
1230212320
| b >9
1230312321
|.code
12322+
|4:
1230412323
}
1230512324
if (opline->opcode == ZEND_FETCH_OBJ_W
1230612325
&& (opline->extended_value & ZEND_FETCH_OBJ_FLAGS)

ext/opcache/jit/zend_jit_helpers.c

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2513,7 +2513,7 @@ static void ZEND_FASTCALL zend_jit_assign_to_typed_prop(zval *property_val, zend
25132513
value = &EG(uninitialized_zval);
25142514
}
25152515

2516-
if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) {
2516+
if (UNEXPECTED((info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE))) {
25172517
zend_readonly_property_modification_error(info);
25182518
if (result) {
25192519
ZVAL_UNDEF(result);
@@ -2532,6 +2532,8 @@ static void ZEND_FASTCALL zend_jit_assign_to_typed_prop(zval *property_val, zend
25322532
return;
25332533
}
25342534

2535+
Z_PROP_FLAG_P(property_val) &= ~IS_PROP_REINITABLE;
2536+
25352537
value = zend_assign_to_variable(property_val, &tmp, IS_TMP_VAR, EX_USES_STRICT_TYPES());
25362538
if (result) {
25372539
ZVAL_COPY_DEREF(result, value);
@@ -2570,7 +2572,7 @@ static void ZEND_FASTCALL zend_jit_assign_op_to_typed_prop(zval *zptr, zend_prop
25702572
zend_execute_data *execute_data = EG(current_execute_data);
25712573
zval z_copy;
25722574

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

25862588
binary_op(&z_copy, zptr, value);
25872589
if (EXPECTED(zend_verify_property_type(prop_info, &z_copy, EX_USES_STRICT_TYPES()))) {
2590+
Z_PROP_FLAG_P(zptr) &= ~IS_PROP_REINITABLE;
25882591
zval_ptr_dtor(zptr);
25892592
ZVAL_COPY_VALUE(zptr, &z_copy);
25902593
} else {
@@ -2664,7 +2667,7 @@ static void ZEND_FASTCALL zend_jit_inc_typed_prop(zval *var_ptr, zend_property_i
26642667
{
26652668
ZEND_ASSERT(Z_TYPE_P(var_ptr) != IS_UNDEF);
26662669

2667-
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY))) {
2670+
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(var_ptr) & IS_PROP_REINITABLE))) {
26682671
zend_readonly_property_modification_error(prop_info);
26692672
return;
26702673
}
@@ -2681,11 +2684,14 @@ static void ZEND_FASTCALL zend_jit_inc_typed_prop(zval *var_ptr, zend_property_i
26812684
if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
26822685
zend_long val = _zend_jit_throw_inc_prop_error(prop_info);
26832686
ZVAL_LONG(var_ptr, val);
2687+
} else {
2688+
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
26842689
}
26852690
} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
26862691
zval_ptr_dtor(var_ptr);
26872692
ZVAL_COPY_VALUE(var_ptr, &tmp);
26882693
} else {
2694+
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
26892695
zval_ptr_dtor(&tmp);
26902696
}
26912697
}
@@ -2694,7 +2700,7 @@ static void ZEND_FASTCALL zend_jit_dec_typed_prop(zval *var_ptr, zend_property_i
26942700
{
26952701
ZEND_ASSERT(Z_TYPE_P(var_ptr) != IS_UNDEF);
26962702

2697-
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY))) {
2703+
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(var_ptr) & IS_PROP_REINITABLE))) {
26982704
zend_readonly_property_modification_error(prop_info);
26992705
return;
27002706
}
@@ -2711,11 +2717,14 @@ static void ZEND_FASTCALL zend_jit_dec_typed_prop(zval *var_ptr, zend_property_i
27112717
if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
27122718
zend_long val = _zend_jit_throw_dec_prop_error(prop_info);
27132719
ZVAL_LONG(var_ptr, val);
2720+
} else {
2721+
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
27142722
}
27152723
} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
27162724
zval_ptr_dtor(var_ptr);
27172725
ZVAL_COPY_VALUE(var_ptr, &tmp);
27182726
} else {
2727+
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
27192728
zval_ptr_dtor(&tmp);
27202729
}
27212730
}
@@ -2738,7 +2747,7 @@ static void ZEND_FASTCALL zend_jit_post_inc_typed_prop(zval *var_ptr, zend_prope
27382747
{
27392748
ZEND_ASSERT(Z_TYPE_P(var_ptr) != IS_UNDEF);
27402749

2741-
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY))) {
2750+
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(var_ptr) & IS_PROP_REINITABLE))) {
27422751
zend_readonly_property_modification_error(prop_info);
27432752
if (result) {
27442753
ZVAL_UNDEF(result);
@@ -2757,19 +2766,23 @@ static void ZEND_FASTCALL zend_jit_post_inc_typed_prop(zval *var_ptr, zend_prope
27572766
if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
27582767
zend_long val = _zend_jit_throw_inc_prop_error(prop_info);
27592768
ZVAL_LONG(var_ptr, val);
2769+
} else {
2770+
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
27602771
}
27612772
} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
27622773
zval_ptr_dtor(var_ptr);
27632774
ZVAL_COPY_VALUE(var_ptr, result);
27642775
ZVAL_UNDEF(result);
2776+
} else {
2777+
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
27652778
}
27662779
}
27672780

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

2772-
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY))) {
2785+
if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(var_ptr) & IS_PROP_REINITABLE))) {
27732786
zend_readonly_property_modification_error(prop_info);
27742787
if (result) {
27752788
ZVAL_UNDEF(result);
@@ -2788,11 +2801,15 @@ static void ZEND_FASTCALL zend_jit_post_dec_typed_prop(zval *var_ptr, zend_prope
27882801
if (!(ZEND_TYPE_FULL_MASK(prop_info->type) & MAY_BE_DOUBLE)) {
27892802
zend_long val = _zend_jit_throw_dec_prop_error(prop_info);
27902803
ZVAL_LONG(var_ptr, val);
2804+
} else {
2805+
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
27912806
}
27922807
} else if (UNEXPECTED(!zend_verify_property_type(prop_info, var_ptr, EX_USES_STRICT_TYPES()))) {
27932808
zval_ptr_dtor(var_ptr);
27942809
ZVAL_COPY_VALUE(var_ptr, result);
27952810
ZVAL_UNDEF(result);
2811+
} else {
2812+
Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_REINITABLE;
27962813
}
27972814
}
27982815

ext/opcache/jit/zend_jit_x86.dasc

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12983,6 +12983,17 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
1298312983
| SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX
1298412984
| jmp >9
1298512985
|2:
12986+
| mov eax, dword [FCARG1a + offsetof(zval, u2.extra)]
12987+
| test eax, IS_PROP_REINITABLE
12988+
| jz >6
12989+
| and eax, ~IS_PROP_REINITABLE
12990+
| mov dword [FCARG1a + offsetof(zval, u2.extra)], eax
12991+
if (flags) {
12992+
| jmp >3
12993+
} else {
12994+
| jmp >4
12995+
}
12996+
|6:
1298612997
| mov FCARG1a, FCARG2a
1298712998
| SET_EX_OPLINE opline, r0
1298812999
| EXT_CALL zend_readonly_property_modification_error, r0
@@ -13034,7 +13045,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
1303413045
if (opline->opcode == ZEND_FETCH_OBJ_W && (prop_info->flags & ZEND_ACC_READONLY)) {
1303513046
if (!type_loaded) {
1303613047
type_loaded = 1;
13037-
| mov edx, dword [FCARG1a + prop_info->offset + 8]
13048+
| mov edx, dword [FCARG1a + prop_info->offset + offsetof(zval, u1.type_info)]
1303813049
}
1303913050
| IF_NOT_TYPE dl, IS_OBJECT, >4
1304013051
| GET_ZVAL_PTR r0, prop_addr
@@ -13044,12 +13055,20 @@ static int zend_jit_fetch_obj(dasm_State **Dst,
1304413055
| jmp >9
1304513056
|.cold_code
1304613057
|4:
13058+
| mov eax, dword [FCARG1a + prop_info->offset + offsetof(zval, u2.extra)]
13059+
| test eax, IS_PROP_REINITABLE
13060+
| jz >6
13061+
| and eax, ~IS_PROP_REINITABLE
13062+
| mov dword [FCARG1a + prop_info->offset + offsetof(zval, u2.extra)], eax
13063+
| jmp >4
13064+
|6:
1304713065
| LOAD_ADDR FCARG1a, prop_info
1304813066
| SET_EX_OPLINE opline, r0
1304913067
| EXT_CALL zend_readonly_property_modification_error, r0
1305013068
| SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR
1305113069
| jmp >9
1305213070
|.code
13071+
|4:
1305313072
}
1305413073
if (opline->opcode == ZEND_FETCH_OBJ_W
1305513074
&& (opline->extended_value & ZEND_FETCH_OBJ_FLAGS)

0 commit comments

Comments
 (0)