diff --git a/Zend/tests/fibers/gh9735-001.phpt b/Zend/tests/fibers/gh9735-001.phpt new file mode 100644 index 00000000000..327e7432392 --- /dev/null +++ b/Zend/tests/fibers/gh9735-001.phpt @@ -0,0 +1,37 @@ +--TEST-- +Bug GH-9735 001 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECT-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-002.phpt b/Zend/tests/fibers/gh9735-002.phpt new file mode 100644 index 00000000000..9a56c65f0ab --- /dev/null +++ b/Zend/tests/fibers/gh9735-002.phpt @@ -0,0 +1,41 @@ +--TEST-- +Bug GH-9735 002 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECT-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-003.phpt b/Zend/tests/fibers/gh9735-003.phpt new file mode 100644 index 00000000000..03ea973b61d --- /dev/null +++ b/Zend/tests/fibers/gh9735-003.phpt @@ -0,0 +1,40 @@ +--TEST-- +Bug GH-9735 003 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECT-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-004.phpt b/Zend/tests/fibers/gh9735-004.phpt new file mode 100644 index 00000000000..cfcf381d83d --- /dev/null +++ b/Zend/tests/fibers/gh9735-004.phpt @@ -0,0 +1,40 @@ +--TEST-- +Bug GH-9735 004 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECT-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-005.phpt b/Zend/tests/fibers/gh9735-005.phpt new file mode 100644 index 00000000000..c18a42fc370 --- /dev/null +++ b/Zend/tests/fibers/gh9735-005.phpt @@ -0,0 +1,44 @@ +--TEST-- +Bug GH-9735 005 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-006.phpt b/Zend/tests/fibers/gh9735-006.phpt new file mode 100644 index 00000000000..59bb79d2404 --- /dev/null +++ b/Zend/tests/fibers/gh9735-006.phpt @@ -0,0 +1,47 @@ +--TEST-- +Bug GH-9735 006 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-007.phpt b/Zend/tests/fibers/gh9735-007.phpt new file mode 100644 index 00000000000..dbb6a338211 --- /dev/null +++ b/Zend/tests/fibers/gh9735-007.phpt @@ -0,0 +1,47 @@ +--TEST-- +Bug GH-9735 007 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-008.phpt b/Zend/tests/fibers/gh9735-008.phpt new file mode 100644 index 00000000000..ec6f29fb79d --- /dev/null +++ b/Zend/tests/fibers/gh9735-008.phpt @@ -0,0 +1,42 @@ +--TEST-- +Bug GH-9735 008 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-009.phpt b/Zend/tests/fibers/gh9735-009.phpt new file mode 100644 index 00000000000..c471499443b --- /dev/null +++ b/Zend/tests/fibers/gh9735-009.phpt @@ -0,0 +1,42 @@ +--TEST-- +Bug GH-9735 009 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 487dcfc6bc8..de32aab8af9 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4414,6 +4414,71 @@ ZEND_API void zend_cleanup_unfinished_execution(zend_execute_data *execute_data, cleanup_live_vars(execute_data, op_num, catch_op_num); } +ZEND_API HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer) +{ + if (!EX(func) || !ZEND_USER_CODE(EX(func)->common.type)) { + return NULL; + } + + zend_op_array *op_array = &EX(func)->op_array; + + if (!(EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE)) { + uint32_t i, num_cvs = EX(func)->op_array.last_var; + for (i = 0; i < num_cvs; i++) { + zend_get_gc_buffer_add_zval(gc_buffer, EX_VAR_NUM(i)); + } + } + + if (EX_CALL_INFO() & ZEND_CALL_FREE_EXTRA_ARGS) { + zval *zv = EX_VAR_NUM(op_array->last_var + op_array->T); + zval *end = zv + (EX_NUM_ARGS() - op_array->num_args); + while (zv != end) { + zend_get_gc_buffer_add_zval(gc_buffer, zv++); + } + } + + if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) { + zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This)); + } + if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) { + zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func))); + } + if (EX_CALL_INFO() & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { + zval extra_named_params; + ZVAL_ARR(&extra_named_params, EX(extra_named_params)); + zend_get_gc_buffer_add_zval(gc_buffer, &extra_named_params); + } + + if (call) { + /* -1 required because we want the last run opcode, not the next to-be-run one. */ + uint32_t op_num = execute_data->opline - op_array->opcodes - 1; + zend_unfinished_calls_gc(execute_data, call, op_num, gc_buffer); + } + + if (execute_data->opline != op_array->opcodes) { + uint32_t i, op_num = execute_data->opline - op_array->opcodes - 1; + for (i = 0; i < op_array->last_live_range; i++) { + const zend_live_range *range = &op_array->live_range[i]; + if (range->start > op_num) { + break; + } else if (op_num < range->end) { + uint32_t kind = range->var & ZEND_LIVE_MASK; + uint32_t var_num = range->var & ~ZEND_LIVE_MASK; + zval *var = EX_VAR(var_num); + if (kind == ZEND_LIVE_TMPVAR || kind == ZEND_LIVE_LOOP) { + zend_get_gc_buffer_add_zval(gc_buffer, var); + } + } + } + } + + if (EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE) { + return execute_data->symbol_table; + } else { + return NULL; + } +} + #if ZEND_VM_SPEC static void zend_swap_operands(zend_op *op) /* {{{ */ { diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index a9b316b8bdb..7a5837f4a0f 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -380,6 +380,7 @@ ZEND_API void zend_clean_and_cache_symbol_table(zend_array *symbol_table); ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *execute_data); ZEND_API void zend_unfinished_calls_gc(zend_execute_data *execute_data, zend_execute_data *call, uint32_t op_num, zend_get_gc_buffer *buf); ZEND_API void zend_cleanup_unfinished_execution(zend_execute_data *execute_data, uint32_t op_num, uint32_t catch_op_num); +ZEND_API HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer); zval * ZEND_FASTCALL zend_handle_named_arg( zend_execute_data **call_ptr, zend_string *arg_name, diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index a78927bee56..ed85834b81b 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -25,6 +25,8 @@ #include "zend_builtin_functions.h" #include "zend_observer.h" #include "zend_mmap.h" +#include "zend_compile.h" +#include "zend_closures.h" #include "zend_fibers.h" #include "zend_fibers_arginfo.h" @@ -659,9 +661,30 @@ static HashTable *zend_fiber_object_gc(zend_object *object, zval **table, int *n zend_get_gc_buffer_add_zval(buf, &fiber->fci.function_name); zend_get_gc_buffer_add_zval(buf, &fiber->result); + if (fiber->context.status != ZEND_FIBER_STATUS_SUSPENDED) { + zend_get_gc_buffer_use(buf, table, num); + return NULL; + } + + HashTable *lastSymTable = NULL; + zend_execute_data *ex = fiber->execute_data; + for (; ex; ex = ex->prev_execute_data) { + HashTable *symTable = zend_unfinished_execution_gc(ex, ex->call, buf); + if (symTable) { + if (lastSymTable) { + zval *val; + ZEND_HASH_FOREACH_VAL(lastSymTable, val) { + ZEND_ASSERT(Z_TYPE_P(val) == IS_INDIRECT); + zend_get_gc_buffer_add_zval(buf, Z_INDIRECT_P(val)); + } ZEND_HASH_FOREACH_END(); + } + lastSymTable = symTable; + } + } + zend_get_gc_buffer_use(buf, table, num); - return NULL; + return lastSymTable; } ZEND_METHOD(Fiber, __construct) diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c index 48f228b956e..f47f008bfab 100644 --- a/Zend/zend_generators.c +++ b/Zend/zend_generators.c @@ -332,7 +332,7 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int * { zend_generator *generator = (zend_generator*)object; zend_execute_data *execute_data = generator->execute_data; - zend_op_array *op_array; + zend_execute_data *call = NULL; if (!execute_data) { /* If the generator has been closed, it can only hold on to three values: The value, key @@ -352,7 +352,6 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int * return NULL; } - op_array = &EX(func)->op_array; zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); zend_get_gc_buffer_add_zval(gc_buffer, &generator->value); @@ -360,57 +359,15 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int * zend_get_gc_buffer_add_zval(gc_buffer, &generator->retval); zend_get_gc_buffer_add_zval(gc_buffer, &generator->values); - if (!(EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE)) { - uint32_t i, num_cvs = EX(func)->op_array.last_var; - for (i = 0; i < num_cvs; i++) { - zend_get_gc_buffer_add_zval(gc_buffer, EX_VAR_NUM(i)); - } - } - - if (EX_CALL_INFO() & ZEND_CALL_FREE_EXTRA_ARGS) { - zval *zv = EX_VAR_NUM(op_array->last_var + op_array->T); - zval *end = zv + (EX_NUM_ARGS() - op_array->num_args); - while (zv != end) { - zend_get_gc_buffer_add_zval(gc_buffer, zv++); - } - } - - if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) { - zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This)); - } - if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) { - zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func))); - } - if (EX_CALL_INFO() & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { - zval extra_named_params; - ZVAL_ARR(&extra_named_params, EX(extra_named_params)); - zend_get_gc_buffer_add_zval(gc_buffer, &extra_named_params); - } - if (UNEXPECTED(generator->frozen_call_stack)) { /* The frozen stack is linked in reverse order */ - zend_execute_data *call = zend_generator_revert_call_stack(generator->frozen_call_stack); - /* -1 required because we want the last run opcode, not the next to-be-run one. */ - uint32_t op_num = execute_data->opline - op_array->opcodes - 1; - zend_unfinished_calls_gc(execute_data, call, op_num, gc_buffer); - zend_generator_revert_call_stack(call); + call = zend_generator_revert_call_stack(generator->frozen_call_stack); } - if (execute_data->opline != op_array->opcodes) { - uint32_t i, op_num = execute_data->opline - op_array->opcodes - 1; - for (i = 0; i < op_array->last_live_range; i++) { - const zend_live_range *range = &op_array->live_range[i]; - if (range->start > op_num) { - break; - } else if (op_num < range->end) { - uint32_t kind = range->var & ZEND_LIVE_MASK; - uint32_t var_num = range->var & ~ZEND_LIVE_MASK; - zval *var = EX_VAR(var_num); - if (kind == ZEND_LIVE_TMPVAR || kind == ZEND_LIVE_LOOP) { - zend_get_gc_buffer_add_zval(gc_buffer, var); - } - } - } + zend_unfinished_execution_gc(execute_data, call, gc_buffer); + + if (UNEXPECTED(generator->frozen_call_stack)) { + zend_generator_revert_call_stack(call); } if (generator->node.parent) { diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 9498bd3718c..eae03e4491f 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -166,6 +166,8 @@ static zend_always_inline void zend_hash_real_init_mixed_ex(HashTable *ht) void *data; uint32_t nSize = ht->nTableSize; + ZEND_ASSERT(HT_SIZE_TO_MASK(nSize)); + if (UNEXPECTED(GC_FLAGS(ht) & IS_ARRAY_PERSISTENT)) { data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), 1); } else if (EXPECTED(nSize == HT_MIN_SIZE)) { @@ -338,6 +340,8 @@ ZEND_API void ZEND_FASTCALL zend_hash_packed_to_hash(HashTable *ht) uint32_t i; uint32_t nSize = ht->nTableSize; + ZEND_ASSERT(HT_SIZE_TO_MASK(nSize)); + HT_ASSERT_RC1(ht); HT_FLAGS(ht) &= ~HASH_FLAG_PACKED; new_data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); @@ -380,7 +384,11 @@ ZEND_API void ZEND_FASTCALL zend_hash_to_packed(HashTable *ht) ZEND_API void ZEND_FASTCALL zend_hash_extend(HashTable *ht, uint32_t nSize, bool packed) { HT_ASSERT_RC1(ht); + if (nSize == 0) return; + + ZEND_ASSERT(HT_SIZE_TO_MASK(nSize)); + if (UNEXPECTED(HT_FLAGS(ht) & HASH_FLAG_UNINITIALIZED)) { if (nSize > ht->nTableSize) { ht->nTableSize = zend_hash_check_size(nSize); @@ -1227,6 +1235,8 @@ static void ZEND_FASTCALL zend_hash_do_resize(HashTable *ht) uint32_t nSize = ht->nTableSize + ht->nTableSize; Bucket *old_buckets = ht->arData; + ZEND_ASSERT(HT_SIZE_TO_MASK(nSize)); + ht->nTableSize = nSize; new_data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); ht->nTableMask = HT_SIZE_TO_MASK(ht->nTableSize); diff --git a/Zend/zend_types.h b/Zend/zend_types.h index df64541749d..e6505d8b62a 100644 --- a/Zend/zend_types.h +++ b/Zend/zend_types.h @@ -411,8 +411,15 @@ struct _zend_array { #define HT_MIN_MASK ((uint32_t) -2) #define HT_MIN_SIZE 8 +/* HT_MAX_SIZE is chosen to satisfy the following constraints: + * - HT_SIZE_TO_MASK(HT_MAX_SIZE) != 0 + * - HT_SIZE_EX(HT_MAX_SIZE, HT_SIZE_TO_MASK(HT_MAX_SIZE)) does not overflow or + * wrapparound, and is <= the addressable space size + * - HT_MAX_SIZE must be a power of two: + * (nTableSize