diff --git a/Zend/tests/gh14983_1.phpt b/Zend/tests/gh14983_1.phpt new file mode 100644 index 0000000000000..04bfde761dc8a --- /dev/null +++ b/Zend/tests/gh14983_1.phpt @@ -0,0 +1,42 @@ +--TEST-- +GH-14983: Shutdown disregards guards active on bailout +--SKIPIF-- + +--FILE-- +{$name}; + } +} + +global $a; +$a = new A; + +function shutdown() { + global $a; + global $loop; + $loop = false; + var_dump($a->foo); +} + +register_shutdown_function('shutdown'); + +$a->foo; + +?> +--EXPECTF-- +Fatal error: Maximum execution time of 1 second exceeded in %s on line %d +A::__get + +Warning: Undefined property: A::$foo in %s on line %d +NULL diff --git a/Zend/tests/gh14983_2.phpt b/Zend/tests/gh14983_2.phpt new file mode 100644 index 0000000000000..59d4b6256a1c1 --- /dev/null +++ b/Zend/tests/gh14983_2.phpt @@ -0,0 +1,32 @@ +--TEST-- +GH-14983: Guards from fibers are separated from guards from main context +--FILE-- +bar; +}); + +$value = $fiber->start(); +echo "Suspended\n"; +$foo->bar; +$fiber->resume('test'); + +?> +--EXPECT-- +__set(bar) +Suspended +__set(bar) +Resuming diff --git a/Zend/zend.c b/Zend/zend.c index ef5ea8cebc78b..751f1570764ff 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -1240,6 +1240,7 @@ ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32 CG(in_compilation) = 0; CG(memoize_mode) = 0; EG(current_execute_data) = NULL; + EG(guard_context) = ++EG(guard_context_counter); LONGJMP(*EG(bailout), FAILURE); } /* }}} */ diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index f1f5bfc84516f..1f52651a05a80 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -200,6 +200,9 @@ void init_executor(void) /* {{{ */ EG(filename_override) = NULL; EG(lineno_override) = -1; + EG(guard_context) = 0; + EG(guard_context_counter) = 0; + zend_max_execution_timer_init(); zend_fiber_init(); zend_weakrefs_init(); diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 273647692d824..35e24be90723f 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -116,6 +116,7 @@ typedef struct _zend_fiber_vm_state { void *stack_base; void *stack_limit; #endif + uint32_t guard_context; } zend_fiber_vm_state; static zend_always_inline void zend_fiber_capture_vm_state(zend_fiber_vm_state *state) @@ -133,6 +134,7 @@ static zend_always_inline void zend_fiber_capture_vm_state(zend_fiber_vm_state * state->stack_base = EG(stack_base); state->stack_limit = EG(stack_limit); #endif + state->guard_context = EG(guard_context); } static zend_always_inline void zend_fiber_restore_vm_state(zend_fiber_vm_state *state) @@ -150,6 +152,7 @@ static zend_always_inline void zend_fiber_restore_vm_state(zend_fiber_vm_state * EG(stack_base) = state->stack_base; EG(stack_limit) = state->stack_limit; #endif + EG(guard_context) = state->guard_context; } #ifdef ZEND_FIBER_UCONTEXT @@ -446,6 +449,7 @@ ZEND_API zend_result zend_fiber_init_context(zend_fiber_context *context, void * context->kind = kind; context->function = coroutine; + context->guard_context = ++EG(guard_context_counter); // Set status in case memory has not been zeroed. context->status = ZEND_FIBER_STATUS_INIT; @@ -500,6 +504,7 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_transfer *transfer) transfer->context = from; EG(current_fiber_context) = to; + EG(guard_context) = to->guard_context; #ifdef __SANITIZE_ADDRESS__ void *fake_stack = NULL; diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index 9442019bfa259..d8a2ffa7f958d 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -95,6 +95,8 @@ struct _zend_fiber_context { /* Observer state */ zend_execute_data *top_observed_frame; + uint32_t guard_context; + /* Reserved for extensions */ void *reserved[ZEND_MAX_RESERVED_RESOURCES]; }; diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h index 3aa967b8dd6bb..509d8f53b8832 100644 --- a/Zend/zend_globals.h +++ b/Zend/zend_globals.h @@ -295,6 +295,13 @@ struct _zend_executor_globals { zend_string *filename_override; zend_long lineno_override; + /* Guards are context dependent. I.e. if __get() is being called for an object + * within a fiber, the guard will _not_ skip __get() in the main context. To + * achieve this, we offset the guard string hash by the guard context. + * Additionally, bailout will discard the current guards in the same way. */ + uint32_t guard_context; + uint32_t guard_context_counter; + #ifdef ZEND_CHECK_STACK_LIMIT zend_call_stack call_stack; zend_long max_allowed_stack_size; diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index e354e2e69a417..6ff66cbd429ec 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -565,6 +565,12 @@ ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *membe zval *zv; uint32_t *ptr; + if (EXPECTED(EG(guard_context) == 0)) { + member = zend_string_copy(member); + } else { + member = zend_string_init(ZSTR_VAL(member), ZSTR_LEN(member), false); + ZSTR_H(member) = zend_string_hash_val(member) + EG(guard_context); + } ZEND_ASSERT(zobj->ce->ce_flags & ZEND_ACC_USE_GUARDS); zv = zend_get_guard_value(zobj); @@ -572,11 +578,14 @@ ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *membe zend_string *str = Z_STR_P(zv); if (EXPECTED(str == member) || /* str and member don't necessarily have a pre-calculated hash value here */ - EXPECTED(zend_string_equal_content(str, member))) { + (EXPECTED(ZSTR_HASH(str) == ZSTR_HASH(member) + && zend_string_equal_content(str, member)))) { + zend_string_release(member); return &Z_GUARD_P(zv); } else if (EXPECTED(Z_GUARD_P(zv) == 0)) { zval_ptr_dtor_str(zv); - ZVAL_STR_COPY(zv, member); + /* Transfer ownership. */ + ZVAL_STR(zv, member); return &Z_GUARD_P(zv); } else { ALLOC_HASHTABLE(guards); @@ -592,18 +601,22 @@ ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *membe ZEND_ASSERT(guards != NULL); zv = zend_hash_find(guards, member); if (zv != NULL) { + zend_string_release(member); return (uint32_t*)(((uintptr_t)Z_PTR_P(zv)) & ~1); } } else { ZEND_ASSERT(Z_TYPE_P(zv) == IS_UNDEF); ZVAL_STR_COPY(zv, member); Z_GUARD_P(zv) &= ~ZEND_GUARD_PROPERTY_MASK; + zend_string_release(member); return &Z_GUARD_P(zv); } /* we have to allocate uint32_t separately because ht->arData may be reallocated */ ptr = (uint32_t*)emalloc(sizeof(uint32_t)); *ptr = 0; - return (uint32_t*)zend_hash_add_new_ptr(guards, member, ptr); + uint32_t *result = (uint32_t*)zend_hash_add_new_ptr(guards, member, ptr); + zend_string_release(member); + return result; } /* }}} */