Skip to content

Commit c878380

Browse files
bwoebibwoebi
committed
Merge branch 'PHP-8.4' of github.com:php/php-src into PHP-8.5
* 'PHP-8.4' of github.com:php/php-src: Split the live-ranges of loop variables again (php#20865)
2 parents 3280368 + 27ed48c commit c878380

File tree

14 files changed

+193
-82
lines changed

14 files changed

+193
-82
lines changed

‎NEWS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ PHP NEWS
1515
. Fix OSS-Fuzz #474613951 (Leaked parent property default value). (ilutov)
1616
. Fixed bug GH-20895 (ReflectionProperty does not return the PHPDoc of a
1717
property if it contains an attribute with a Closure). (timwolla)
18+
. Fixed bug GH-20766 (Use-after-free in FE_FREE with GC interaction). (Bob)
1819

1920
- Date:
2021
. Update timelib to 2022.16. (Derick)

‎Zend/Optimizer/zend_dump.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ static void zend_dump_unused_op(const zend_op *opline, znode_op op, uint32_t fla
136136
if (op.num != (uint32_t)-1) {
137137
fprintf(stderr, " try-catch(%u)", op.num);
138138
}
139+
} else if (ZEND_VM_OP_LOOP_END == (flags & ZEND_VM_OP_MASK)) {
140+
if (opline->extended_value & ZEND_FREE_ON_RETURN) {
141+
fprintf(stderr, " loop-end(+%u)", op.num);
142+
}
139143
} else if (ZEND_VM_OP_THIS == (flags & ZEND_VM_OP_MASK)) {
140144
fprintf(stderr, " THIS");
141145
} else if (ZEND_VM_OP_NEXT == (flags & ZEND_VM_OP_MASK)) {

‎Zend/tests/gc/gc_050.phpt‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,40 @@
11
--TEST--
2-
GC 050: Destructor are never called twice
2+
GC 050: Try/finally in foreach should create separate live ranges
33
--FILE--
44
<?php
55

6-
class G
7-
{
8-
public static $v;
6+
function f(int $n): object {
7+
try {
8+
foreach ((array) $n as $v) {
9+
if ($n === 1) {
10+
try {
11+
$a = new stdClass;
12+
return $a;
13+
} finally {
14+
return $ret = $a;
15+
}
16+
}
17+
if ($n === 2) {
18+
$b = new stdClass;
19+
return $ret = $b;
20+
}
21+
}
22+
} finally {
23+
$ret->v = 1;
24+
}
25+
return new stdClass;
926
}
1027

11-
class WithDestructor
12-
{
13-
public function __destruct()
14-
{
15-
echo "d\n";
28+
for ($i = 0; $i < 100000; $i++) {
29+
// Create cyclic garbage to trigger GC
30+
$a = new stdClass;
31+
$b = new stdClass;
32+
$a->r = $b;
33+
$b->r = $a;
1634

17-
G::$v = $this;
18-
}
35+
$r = f($i % 2 + 1);
1936
}
20-
21-
$o = new WithDestructor();
22-
$weakO = \WeakReference::create($o);
23-
echo "---\n";
24-
unset($o);
25-
echo "---\n";
26-
var_dump($weakO->get() !== null); // verify if kept allocated
27-
G::$v = null;
28-
echo "---\n";
29-
var_dump($weakO->get() !== null); // verify if released
37+
echo "OK\n";
3038
?>
3139
--EXPECT--
32-
---
33-
d
34-
---
35-
bool(true)
36-
---
37-
bool(false)
40+
OK

‎Zend/tests/gc_051.phpt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GC 048: FE_FREE should mark variable as UNDEF to prevent use-after-free during GC
3+
--FILE--
4+
<?php
5+
// FE_FREE frees the iterator but doesn't set zval to UNDEF
6+
// When GC runs during RETURN, zend_gc_remove_root_tmpvars() may access freed memory
7+
8+
function test_foreach_early_return(string $s): object {
9+
foreach ((array) $s as $v) {
10+
$obj = new stdClass;
11+
// in the early return, the VAR for the cast result is still live
12+
return $obj; // the return may trigger GC
13+
}
14+
}
15+
16+
for ($i = 0; $i < 100000; $i++) {
17+
// create cyclic garbage to fill GC buffer
18+
$a = new stdClass;
19+
$b = new stdClass;
20+
$a->ref = $b;
21+
$b->ref = $a;
22+
23+
$result = test_foreach_early_return("x");
24+
}
25+
26+
echo "OK\n";
27+
?>
28+
--EXPECT--
29+
OK

‎Zend/tests/gc_052.phpt‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
GC 049: Multiple early returns from foreach should create separate live ranges
3+
--FILE--
4+
<?php
5+
6+
function f(int $n): object {
7+
foreach ((array) $n as $v) {
8+
if ($n === 1) {
9+
$a = new stdClass;
10+
return $a;
11+
}
12+
if ($n === 2) {
13+
$b = new stdClass;
14+
return $b;
15+
}
16+
if ($n === 3) {
17+
$c = new stdClass;
18+
return $c;
19+
}
20+
}
21+
return new stdClass;
22+
}
23+
24+
for ($i = 0; $i < 100000; $i++) {
25+
// Create cyclic garbage to trigger GC
26+
$a = new stdClass;
27+
$b = new stdClass;
28+
$a->r = $b;
29+
$b->r = $a;
30+
31+
$r = f($i % 3 + 1);
32+
}
33+
echo "OK\n";
34+
?>
35+
--EXPECT--
36+
OK

‎Zend/tests/gc_053.phpt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
GC 050: Destructor are never called twice
3+
--FILE--
4+
<?php
5+
6+
class G
7+
{
8+
public static $v;
9+
}
10+
11+
class WithDestructor
12+
{
13+
public function __destruct()
14+
{
15+
echo "d\n";
16+
17+
G::$v = $this;
18+
}
19+
}
20+
21+
$o = new WithDestructor();
22+
$weakO = \WeakReference::create($o);
23+
echo "---\n";
24+
unset($o);
25+
echo "---\n";
26+
var_dump($weakO->get() !== null); // verify if kept allocated
27+
G::$v = null;
28+
echo "---\n";
29+
var_dump($weakO->get() !== null); // verify if released
30+
?>
31+
--EXPECT--
32+
---
33+
d
34+
---
35+
bool(true)
36+
---
37+
bool(false)

‎Zend/zend_execute.c‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4881,20 +4881,6 @@ static void cleanup_unfinished_calls(zend_execute_data *execute_data, uint32_t o
48814881
}
48824882
/* }}} */
48834883

4884-
static const zend_live_range *find_live_range(const zend_op_array *op_array, uint32_t op_num, uint32_t var_num) /* {{{ */
4885-
{
4886-
int i;
4887-
for (i = 0; i < op_array->last_live_range; i++) {
4888-
const zend_live_range *range = &op_array->live_range[i];
4889-
if (op_num >= range->start && op_num < range->end
4890-
&& var_num == (range->var & ~ZEND_LIVE_MASK)) {
4891-
return range;
4892-
}
4893-
}
4894-
return NULL;
4895-
}
4896-
/* }}} */
4897-
48984884
static void cleanup_live_vars(zend_execute_data *execute_data, uint32_t op_num, uint32_t catch_op_num) /* {{{ */
48994885
{
49004886
int i;
@@ -4910,6 +4896,16 @@ static void cleanup_live_vars(zend_execute_data *execute_data, uint32_t op_num,
49104896
uint32_t var_num = range->var & ~ZEND_LIVE_MASK;
49114897
zval *var = EX_VAR(var_num);
49124898

4899+
/* Handle the split range for loop vars */
4900+
if (catch_op_num) {
4901+
zend_op *final_op = EX(func)->op_array.opcodes + range->end;
4902+
if (final_op->extended_value & ZEND_FREE_ON_RETURN && (final_op->opcode == ZEND_FE_FREE || final_op->opcode == ZEND_FREE)) {
4903+
if (catch_op_num < range->end + final_op->op2.num) {
4904+
continue;
4905+
}
4906+
}
4907+
}
4908+
49134909
if (kind == ZEND_LIVE_TMPVAR) {
49144910
zval_ptr_dtor_nogc(var);
49154911
} else if (kind == ZEND_LIVE_NEW) {

‎Zend/zend_opcode.c‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,35 @@ static void zend_calc_live_ranges(
987987
/* OP_DATA is really part of the previous opcode. */
988988
last_use[var_num] = opnum - (opline->opcode == ZEND_OP_DATA);
989989
}
990+
} else if ((opline->opcode == ZEND_FREE || opline->opcode == ZEND_FE_FREE) && opline->extended_value & ZEND_FREE_ON_RETURN) {
991+
int jump_offset = 1;
992+
while (((opline + jump_offset)->opcode == ZEND_FREE || (opline + jump_offset)->opcode == ZEND_FE_FREE)
993+
&& (opline + jump_offset)->extended_value & ZEND_FREE_ON_RETURN) {
994+
++jump_offset;
995+
}
996+
// loop var frees directly precede the jump (or return) operand, except that ZEND_VERIFY_RETURN_TYPE may happen first.
997+
if ((opline + jump_offset)->opcode == ZEND_VERIFY_RETURN_TYPE) {
998+
++jump_offset;
999+
}
1000+
/* FREE with ZEND_FREE_ON_RETURN immediately followed by RETURN frees
1001+
* the loop variable on early return. We need to split the live range
1002+
* so GC doesn't access the freed variable after this FREE. */
1003+
uint32_t opnum_last_use = last_use[var_num];
1004+
zend_op *opline_last_use = op_array->opcodes + opnum_last_use;
1005+
ZEND_ASSERT(opline_last_use->opcode == opline->opcode); // any ZEND_FREE_ON_RETURN must be followed by a FREE without
1006+
if (opnum + jump_offset + 1 != opnum_last_use) {
1007+
emit_live_range_raw(op_array, var_num, opline->opcode == ZEND_FE_FREE ? ZEND_LIVE_LOOP : ZEND_LIVE_TMPVAR,
1008+
opnum + jump_offset + 1, opnum_last_use);
1009+
}
1010+
1011+
/* Update last_use so next range includes this FREE */
1012+
last_use[var_num] = opnum;
1013+
1014+
/* Store opline offset to loop end */
1015+
opline->op2.opline_num = opnum_last_use - opnum;
1016+
if (opline_last_use->extended_value & ZEND_FREE_ON_RETURN) {
1017+
opline->op2.opline_num += opline_last_use->op2.opline_num;
1018+
}
9901019
}
9911020
}
9921021
if (opline->op2_type & (IS_TMP_VAR|IS_VAR)) {

‎Zend/zend_vm_def.h‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3246,7 +3246,7 @@ ZEND_VM_COLD_CONST_HANDLER(47, ZEND_JMPNZ_EX, CONST|TMPVAR|CV, JMP_ADDR)
32463246
ZEND_VM_JMP(opline);
32473247
}
32483248

3249-
ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, ANY)
3249+
ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, LOOP_END)
32503250
{
32513251
USE_OPLINE
32523252

@@ -3255,7 +3255,7 @@ ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, ANY)
32553255
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
32563256
}
32573257

3258-
ZEND_VM_HOT_HANDLER(127, ZEND_FE_FREE, TMPVAR, ANY)
3258+
ZEND_VM_HOT_HANDLER(127, ZEND_FE_FREE, TMPVAR, LOOP_END)
32593259
{
32603260
zval *var;
32613261
USE_OPLINE
@@ -8185,24 +8185,11 @@ ZEND_VM_HANDLER(149, ZEND_HANDLE_EXCEPTION, ANY, ANY)
81858185
&& throw_op->extended_value & ZEND_FREE_ON_RETURN) {
81868186
/* exceptions thrown because of loop var destruction on return/break/...
81878187
* are logically thrown at the end of the foreach loop, so adjust the
8188-
* throw_op_num.
8188+
* throw_op_num to the final loop variable FREE.
81898189
*/
8190-
const zend_live_range *range = find_live_range(
8191-
&EX(func)->op_array, throw_op_num, throw_op->op1.var);
8192-
/* free op1 of the corresponding RETURN */
8193-
for (i = throw_op_num; i < range->end; i++) {
8194-
if (EX(func)->op_array.opcodes[i].opcode == ZEND_FREE
8195-
|| EX(func)->op_array.opcodes[i].opcode == ZEND_FE_FREE) {
8196-
/* pass */
8197-
} else {
8198-
if (EX(func)->op_array.opcodes[i].opcode == ZEND_RETURN
8199-
&& (EX(func)->op_array.opcodes[i].op1_type & (IS_VAR|IS_TMP_VAR))) {
8200-
zval_ptr_dtor(EX_VAR(EX(func)->op_array.opcodes[i].op1.var));
8201-
}
8202-
break;
8203-
}
8204-
}
8205-
throw_op_num = range->end;
8190+
uint32_t new_throw_op_num = throw_op_num + throw_op->op2.opline_num;
8191+
cleanup_live_vars(execute_data, throw_op_num, new_throw_op_num);
8192+
throw_op_num = new_throw_op_num;
82068193
}
82078194

82088195
/* Find the innermost try/catch/finally the exception was thrown in */

‎Zend/zend_vm_execute.h‎

Lines changed: 4 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)