diff --git a/Zend/Optimizer/zend_dump.c b/Zend/Optimizer/zend_dump.c index b788b652979de..5b61280d48056 100644 --- a/Zend/Optimizer/zend_dump.c +++ b/Zend/Optimizer/zend_dump.c @@ -122,6 +122,10 @@ static void zend_dump_unused_op(const zend_op *opline, znode_op op, uint32_t fla if (op.num != (uint32_t)-1) { fprintf(stderr, " try-catch(%u)", op.num); } + } else if (ZEND_VM_OP_LOOP_END == (flags & ZEND_VM_OP_MASK)) { + if (opline->extended_value & ZEND_FREE_ON_RETURN) { + fprintf(stderr, " loop-end(+%u)", op.num); + } } else if (ZEND_VM_OP_THIS == (flags & ZEND_VM_OP_MASK)) { fprintf(stderr, " THIS"); } else if (ZEND_VM_OP_NEXT == (flags & ZEND_VM_OP_MASK)) { diff --git a/Zend/tests/gc_050.phpt b/Zend/tests/gc_050.phpt index 858be7cbebd5f..0bedc7220fd43 100644 --- a/Zend/tests/gc_050.phpt +++ b/Zend/tests/gc_050.phpt @@ -1,37 +1,40 @@ --TEST-- -GC 050: Destructor are never called twice +GC 050: Try/finally in foreach should create separate live ranges --FILE-- v = 1; + } + return new stdClass; } -class WithDestructor -{ - public function __destruct() - { - echo "d\n"; +for ($i = 0; $i < 100000; $i++) { + // Create cyclic garbage to trigger GC + $a = new stdClass; + $b = new stdClass; + $a->r = $b; + $b->r = $a; - G::$v = $this; - } + $r = f($i % 2 + 1); } - -$o = new WithDestructor(); -$weakO = \WeakReference::create($o); -echo "---\n"; -unset($o); -echo "---\n"; -var_dump($weakO->get() !== null); // verify if kept allocated -G::$v = null; -echo "---\n"; -var_dump($weakO->get() !== null); // verify if released +echo "OK\n"; ?> --EXPECT-- ---- -d ---- -bool(true) ---- -bool(false) +OK diff --git a/Zend/tests/gc_051.phpt b/Zend/tests/gc_051.phpt new file mode 100644 index 0000000000000..575a25a108a15 --- /dev/null +++ b/Zend/tests/gc_051.phpt @@ -0,0 +1,29 @@ +--TEST-- +GC 048: FE_FREE should mark variable as UNDEF to prevent use-after-free during GC +--FILE-- +ref = $b; + $b->ref = $a; + + $result = test_foreach_early_return("x"); +} + +echo "OK\n"; +?> +--EXPECT-- +OK diff --git a/Zend/tests/gc_052.phpt b/Zend/tests/gc_052.phpt new file mode 100644 index 0000000000000..dd15c56bcbf54 --- /dev/null +++ b/Zend/tests/gc_052.phpt @@ -0,0 +1,36 @@ +--TEST-- +GC 049: Multiple early returns from foreach should create separate live ranges +--FILE-- +r = $b; + $b->r = $a; + + $r = f($i % 3 + 1); +} +echo "OK\n"; +?> +--EXPECT-- +OK diff --git a/Zend/tests/gc_053.phpt b/Zend/tests/gc_053.phpt new file mode 100644 index 0000000000000..858be7cbebd5f --- /dev/null +++ b/Zend/tests/gc_053.phpt @@ -0,0 +1,37 @@ +--TEST-- +GC 050: Destructor are never called twice +--FILE-- +get() !== null); // verify if kept allocated +G::$v = null; +echo "---\n"; +var_dump($weakO->get() !== null); // verify if released +?> +--EXPECT-- +--- +d +--- +bool(true) +--- +bool(false) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index c2d4e57c02db6..ea5f55cf6ef78 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4673,20 +4673,6 @@ static void cleanup_unfinished_calls(zend_execute_data *execute_data, uint32_t o } /* }}} */ -static const zend_live_range *find_live_range(const zend_op_array *op_array, uint32_t op_num, uint32_t var_num) /* {{{ */ -{ - int i; - for (i = 0; i < op_array->last_live_range; i++) { - const zend_live_range *range = &op_array->live_range[i]; - if (op_num >= range->start && op_num < range->end - && var_num == (range->var & ~ZEND_LIVE_MASK)) { - return range; - } - } - return NULL; -} -/* }}} */ - static void cleanup_live_vars(zend_execute_data *execute_data, uint32_t op_num, uint32_t catch_op_num) /* {{{ */ { int i; @@ -4702,6 +4688,16 @@ static void cleanup_live_vars(zend_execute_data *execute_data, uint32_t op_num, uint32_t var_num = range->var & ~ZEND_LIVE_MASK; zval *var = EX_VAR(var_num); + /* Handle the split range for loop vars */ + if (catch_op_num) { + zend_op *final_op = EX(func)->op_array.opcodes + range->end; + if (final_op->extended_value & ZEND_FREE_ON_RETURN && (final_op->opcode == ZEND_FE_FREE || final_op->opcode == ZEND_FREE)) { + if (catch_op_num < range->end + final_op->op2.num) { + continue; + } + } + } + if (kind == ZEND_LIVE_TMPVAR) { zval_ptr_dtor_nogc(var); } else if (kind == ZEND_LIVE_NEW) { diff --git a/Zend/zend_opcode.c b/Zend/zend_opcode.c index f32ae13e06793..7a364dfccbcdd 100644 --- a/Zend/zend_opcode.c +++ b/Zend/zend_opcode.c @@ -981,6 +981,35 @@ static void zend_calc_live_ranges( /* OP_DATA is really part of the previous opcode. */ last_use[var_num] = opnum - (opline->opcode == ZEND_OP_DATA); } + } else if ((opline->opcode == ZEND_FREE || opline->opcode == ZEND_FE_FREE) && opline->extended_value & ZEND_FREE_ON_RETURN) { + int jump_offset = 1; + while (((opline + jump_offset)->opcode == ZEND_FREE || (opline + jump_offset)->opcode == ZEND_FE_FREE) + && (opline + jump_offset)->extended_value & ZEND_FREE_ON_RETURN) { + ++jump_offset; + } + // loop var frees directly precede the jump (or return) operand, except that ZEND_VERIFY_RETURN_TYPE may happen first. + if ((opline + jump_offset)->opcode == ZEND_VERIFY_RETURN_TYPE) { + ++jump_offset; + } + /* FREE with ZEND_FREE_ON_RETURN immediately followed by RETURN frees + * the loop variable on early return. We need to split the live range + * so GC doesn't access the freed variable after this FREE. */ + uint32_t opnum_last_use = last_use[var_num]; + zend_op *opline_last_use = op_array->opcodes + opnum_last_use; + ZEND_ASSERT(opline_last_use->opcode == opline->opcode); // any ZEND_FREE_ON_RETURN must be followed by a FREE without + if (opnum + jump_offset + 1 != opnum_last_use) { + emit_live_range_raw(op_array, var_num, opline->opcode == ZEND_FE_FREE ? ZEND_LIVE_LOOP : ZEND_LIVE_TMPVAR, + opnum + jump_offset + 1, opnum_last_use); + } + + /* Update last_use so next range includes this FREE */ + last_use[var_num] = opnum; + + /* Store opline offset to loop end */ + opline->op2.opline_num = opnum_last_use - opnum; + if (opline_last_use->extended_value & ZEND_FREE_ON_RETURN) { + opline->op2.opline_num += opline_last_use->op2.opline_num; + } } } if (opline->op2_type & (IS_TMP_VAR|IS_VAR)) { diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 039d9679848ee..1f06eab120d39 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -3193,7 +3193,7 @@ ZEND_VM_COLD_CONST_HANDLER(47, ZEND_JMPNZ_EX, CONST|TMPVAR|CV, JMP_ADDR) ZEND_VM_JMP(opline); } -ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, ANY) +ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, LOOP_END) { USE_OPLINE @@ -3202,7 +3202,7 @@ ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, ANY) ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION(); } -ZEND_VM_HOT_HANDLER(127, ZEND_FE_FREE, TMPVAR, ANY) +ZEND_VM_HOT_HANDLER(127, ZEND_FE_FREE, TMPVAR, LOOP_END) { zval *var; USE_OPLINE @@ -8140,24 +8140,11 @@ ZEND_VM_HANDLER(149, ZEND_HANDLE_EXCEPTION, ANY, ANY) && throw_op->extended_value & ZEND_FREE_ON_RETURN) { /* exceptions thrown because of loop var destruction on return/break/... * are logically thrown at the end of the foreach loop, so adjust the - * throw_op_num. + * throw_op_num to the final loop variable FREE. */ - const zend_live_range *range = find_live_range( - &EX(func)->op_array, throw_op_num, throw_op->op1.var); - /* free op1 of the corresponding RETURN */ - for (i = throw_op_num; i < range->end; i++) { - if (EX(func)->op_array.opcodes[i].opcode == ZEND_FREE - || EX(func)->op_array.opcodes[i].opcode == ZEND_FE_FREE) { - /* pass */ - } else { - if (EX(func)->op_array.opcodes[i].opcode == ZEND_RETURN - && (EX(func)->op_array.opcodes[i].op1_type & (IS_VAR|IS_TMP_VAR))) { - zval_ptr_dtor(EX_VAR(EX(func)->op_array.opcodes[i].op1.var)); - } - break; - } - } - throw_op_num = range->end; + uint32_t new_throw_op_num = throw_op_num + throw_op->op2.opline_num; + cleanup_live_vars(execute_data, throw_op_num, new_throw_op_num); + throw_op_num = new_throw_op_num; } /* Find the innermost try/catch/finally the exception was thrown in */ diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index d6ee850839aec..fdef3e3a1b74e 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -3265,24 +3265,11 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_HANDLE_EXCEPTION_SPEC_HANDLER( && throw_op->extended_value & ZEND_FREE_ON_RETURN) { /* exceptions thrown because of loop var destruction on return/break/... * are logically thrown at the end of the foreach loop, so adjust the - * throw_op_num. + * throw_op_num to the final loop variable FREE. */ - const zend_live_range *range = find_live_range( - &EX(func)->op_array, throw_op_num, throw_op->op1.var); - /* free op1 of the corresponding RETURN */ - for (i = throw_op_num; i < range->end; i++) { - if (EX(func)->op_array.opcodes[i].opcode == ZEND_FREE - || EX(func)->op_array.opcodes[i].opcode == ZEND_FE_FREE) { - /* pass */ - } else { - if (EX(func)->op_array.opcodes[i].opcode == ZEND_RETURN - && (EX(func)->op_array.opcodes[i].op1_type & (IS_VAR|IS_TMP_VAR))) { - zval_ptr_dtor(EX_VAR(EX(func)->op_array.opcodes[i].op1.var)); - } - break; - } - } - throw_op_num = range->end; + uint32_t new_throw_op_num = throw_op_num + throw_op->op2.opline_num; + cleanup_live_vars(execute_data, throw_op_num, new_throw_op_num); + throw_op_num = new_throw_op_num; } /* Find the innermost try/catch/finally the exception was thrown in */ diff --git a/Zend/zend_vm_gen.php b/Zend/zend_vm_gen.php index 8c178aba04ce1..5f1a44efae3a9 100755 --- a/Zend/zend_vm_gen.php +++ b/Zend/zend_vm_gen.php @@ -63,7 +63,7 @@ "ZEND_VM_OP_NUM" => 0x10, "ZEND_VM_OP_JMP_ADDR" => 0x20, "ZEND_VM_OP_TRY_CATCH" => 0x30, - // unused 0x40 + "ZEND_VM_OP_LOOP_END" => 0x40, "ZEND_VM_OP_THIS" => 0x50, "ZEND_VM_OP_NEXT" => 0x60, "ZEND_VM_OP_CLASS_FETCH" => 0x70, @@ -111,6 +111,7 @@ "NUM" => ZEND_VM_OP_NUM, "JMP_ADDR" => ZEND_VM_OP_JMP_ADDR, "TRY_CATCH" => ZEND_VM_OP_TRY_CATCH, + "LOOP_END" => ZEND_VM_OP_LOOP_END, "THIS" => ZEND_VM_OP_THIS, "NEXT" => ZEND_VM_OP_NEXT, "CLASS_FETCH" => ZEND_VM_OP_CLASS_FETCH, diff --git a/Zend/zend_vm_opcodes.c b/Zend/zend_vm_opcodes.c index 202dfd3f734f3..2d57da5d06f15 100644 --- a/Zend/zend_vm_opcodes.c +++ b/Zend/zend_vm_opcodes.c @@ -306,7 +306,7 @@ static uint32_t zend_vm_opcodes_flags[210] = { 0x00001301, 0x0100a173, 0x01040300, - 0x00000005, + 0x00004005, 0x00186703, 0x00106703, 0x08000007, @@ -363,7 +363,7 @@ static uint32_t zend_vm_opcodes_flags[210] = { 0x0000a103, 0x00002003, 0x03000001, - 0x00000005, + 0x00004005, 0x01000700, 0x00000000, 0x00000000, diff --git a/Zend/zend_vm_opcodes.h b/Zend/zend_vm_opcodes.h index d472b5b9660f5..9e56910c14455 100644 --- a/Zend/zend_vm_opcodes.h +++ b/Zend/zend_vm_opcodes.h @@ -48,6 +48,7 @@ #define ZEND_VM_OP_NUM 0x00000010 #define ZEND_VM_OP_JMP_ADDR 0x00000020 #define ZEND_VM_OP_TRY_CATCH 0x00000030 +#define ZEND_VM_OP_LOOP_END 0x00000040 #define ZEND_VM_OP_THIS 0x00000050 #define ZEND_VM_OP_NEXT 0x00000060 #define ZEND_VM_OP_CLASS_FETCH 0x00000070 diff --git a/ext/opcache/tests/opt/gh11245_2.phpt b/ext/opcache/tests/opt/gh11245_2.phpt index f42da12c52743..cd5b0bd363b62 100644 --- a/ext/opcache/tests/opt/gh11245_2.phpt +++ b/ext/opcache/tests/opt/gh11245_2.phpt @@ -28,9 +28,9 @@ $_main: 0000 T1 = PRE_INC_STATIC_PROP string("prop") string("X") 0001 T2 = ISSET_ISEMPTY_CV (empty) CV0($xx) 0002 JMPZ T2 0005 -0003 FREE T1 +0003 FREE T1 loop-end(+2) 0004 RETURN null 0005 FREE T1 0006 RETURN int(1) LIVE RANGES: - 1: 0001 - 0005 (tmp/var) + 1: 0001 - 0003 (tmp/var)