From 94563cedbe5310440211fa3fb1f69b9da61374d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Schr=C3=B6der?= Date: Fri, 4 Jun 2021 15:11:18 +0200 Subject: [PATCH 1/6] Export fiber switch observer function. --- Zend/zend_observer.c | 2 +- Zend/zend_observer.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Zend/zend_observer.c b/Zend/zend_observer.c index 53a28707ecc57..7b5703ae627cd 100644 --- a/Zend/zend_observer.c +++ b/Zend/zend_observer.c @@ -264,7 +264,7 @@ ZEND_API void zend_observer_fiber_switch_register(zend_observer_fiber_switch_han zend_llist_add_element(&zend_observer_fiber_switch, &handler); } -void zend_observer_fiber_switch_notify(zend_fiber *from, zend_fiber *to) +ZEND_API void ZEND_FASTCALL zend_observer_fiber_switch_notify(zend_fiber *from, zend_fiber *to) { zend_llist_element *element; zend_observer_fiber_switch_handler callback; diff --git a/Zend/zend_observer.h b/Zend/zend_observer.h index 6ee73f84a4e2e..9145f4f8620fe 100644 --- a/Zend/zend_observer.h +++ b/Zend/zend_observer.h @@ -81,7 +81,7 @@ void zend_observer_error_notify(int type, zend_string *error_filename, uint32_t typedef void (*zend_observer_fiber_switch_handler)(zend_fiber *from, zend_fiber *to); ZEND_API void zend_observer_fiber_switch_register(zend_observer_fiber_switch_handler handler); -void zend_observer_fiber_switch_notify(zend_fiber *from, zend_fiber *to); +ZEND_API void ZEND_FASTCALL zend_observer_fiber_switch_notify(zend_fiber *from, zend_fiber *to); END_EXTERN_C() From 16e3120ea419b8ffbb6e027c414f3912ac6cbdfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Schr=C3=B6der?= Date: Fri, 4 Jun 2021 18:25:03 +0200 Subject: [PATCH 2/6] Refactored fiber internals. --- Zend/zend_execute_API.c | 1 + Zend/zend_fibers.c | 102 ++++++++++----------- Zend/zend_fibers.h | 56 ++++++----- Zend/zend_globals.h | 7 +- Zend/zend_observer.c | 2 +- Zend/zend_observer.h | 4 +- ext/reflection/php_reflection.c | 6 +- ext/zend_test/test.c | 42 ++++----- ext/zend_test/tests/observer_fiber_01.phpt | 8 +- ext/zend_test/tests/observer_fiber_02.phpt | 8 +- ext/zend_test/tests/observer_fiber_03.phpt | 16 ++-- ext/zend_test/tests/observer_fiber_04.phpt | 12 +-- ext/zend_test/tests/observer_fiber_05.phpt | 12 +-- ext/zend_test/tests/observer_fiber_06.phpt | 8 +- 14 files changed, 142 insertions(+), 142 deletions(-) diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index b66345c1c3397..bcd6863dfe7e1 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -380,6 +380,7 @@ void shutdown_executor(void) /* {{{ */ zend_objects_store_free_object_storage(&EG(objects_store), fast_shutdown); zend_weakrefs_shutdown(); + zend_fiber_shutdown(); zend_try { zend_llist_apply(&zend_extensions, (llist_apply_func_t) zend_extension_deactivator); diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 0db3ad7e4f2b0..9be34f93a951c 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -200,23 +200,21 @@ static void zend_fiber_stack_free(zend_fiber_stack *stack) static ZEND_NORETURN void zend_fiber_trampoline(transfer_t transfer) { - zend_fiber_context *context = transfer.data; + zend_fiber_context *context = EG(current_fiber); + + ((zend_fiber_context *) transfer.data)->handle = transfer.context; #ifdef __SANITIZE_ADDRESS__ __sanitizer_finish_switch_fiber(NULL, &context->stack.prior_pointer, &context->stack.prior_size); #endif - context->caller = transfer.context; - - context->function(context); - - context->self = NULL; + zend_fiber_context *to = context->function(context); #ifdef __SANITIZE_ADDRESS__ __sanitizer_start_switch_fiber(NULL, context->stack.prior_pointer, context->stack.prior_size); #endif - jump_fcontext(context->caller, NULL); + zend_fiber_switch_context(context, to); abort(); } @@ -230,10 +228,10 @@ ZEND_API bool zend_fiber_init_context(zend_fiber_context *context, zend_fiber_co // Stack grows down, calculate the top of the stack. make_fcontext then shifts pointer to lower 16-byte boundary. void *stack = (void *) ((uintptr_t) context->stack.pointer + context->stack.size); - context->self = make_fcontext(stack, context->stack.size, zend_fiber_trampoline); - ZEND_ASSERT(context->self != NULL && "make_fcontext() never returns NULL"); + context->handle = make_fcontext(stack, context->stack.size, zend_fiber_trampoline); + ZEND_ASSERT(context->handle != NULL && "make_fcontext() never returns NULL"); + context->function = coroutine; - context->caller = NULL; return true; } @@ -243,40 +241,26 @@ ZEND_API void zend_fiber_destroy_context(zend_fiber_context *context) zend_fiber_stack_free(&context->stack); } -ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) +ZEND_API void zend_fiber_switch_context(zend_fiber_context *from, zend_fiber_context *to) { - ZEND_ASSERT(to && to->self && to->stack.pointer && "Invalid fiber context"); + ZEND_ASSERT(to && to->handle && "Invalid fiber context"); + ZEND_ASSERT(from && "From context must be present"); #ifdef __SANITIZE_ADDRESS__ void *fake_stack; __sanitizer_start_switch_fiber(&fake_stack, to->stack.pointer, to->stack.size); #endif - transfer_t transfer = jump_fcontext(to->self, to); - -#ifdef __SANITIZE_ADDRESS__ - __sanitizer_finish_switch_fiber(fake_stack, &to->stack.prior_pointer, &to->stack.prior_size); -#endif + EG(current_fiber) = to; - to->self = transfer.context; -} - -ZEND_API void zend_fiber_suspend_context(zend_fiber_context *current) -{ - ZEND_ASSERT(current && current->caller && current->stack.pointer && "Invalid fiber context"); - -#ifdef __SANITIZE_ADDRESS__ - void *fake_stack; - __sanitizer_start_switch_fiber(&fake_stack, current->stack.prior_pointer, current->stack.prior_size); -#endif + transfer_t transfer = jump_fcontext(to->handle, from); + ((zend_fiber_context *) transfer.data)->handle = transfer.context; - transfer_t transfer = jump_fcontext(current->caller, NULL); + EG(current_fiber) = from; #ifdef __SANITIZE_ADDRESS__ - __sanitizer_finish_switch_fiber(fake_stack, ¤t->stack.prior_pointer, ¤t->stack.prior_size); + __sanitizer_finish_switch_fiber(fake_stack, &to->stack.prior_pointer, &to->stack.prior_size); #endif - - current->caller = transfer.context; } static void zend_fiber_suspend_from(zend_fiber *fiber) @@ -288,16 +272,18 @@ static void zend_fiber_suspend_from(zend_fiber *fiber) uint32_t jit_trace_num; JMP_BUF *bailout; + ZEND_ASSERT(fiber->caller && "Fiber has no caller"); + ZEND_FIBER_BACKUP_EG(stack, stack_page_size, execute_data, error_reporting, jit_trace_num, bailout); - zend_fiber_suspend_context(&fiber->context); + zend_fiber_switch_context(zend_fiber_get_context(fiber), zend_fiber_get_context(fiber->caller)); ZEND_FIBER_RESTORE_EG(stack, stack_page_size, execute_data, error_reporting, jit_trace_num, bailout); } static void zend_fiber_switch_to(zend_fiber *fiber) { - zend_fiber *previous; + zend_fiber_context *context = zend_fiber_get_context(fiber); zend_vm_stack stack; size_t stack_page_size; zend_execute_data *execute_data; @@ -305,21 +291,19 @@ static void zend_fiber_switch_to(zend_fiber *fiber) uint32_t jit_trace_num; JMP_BUF *bailout; - previous = EG(current_fiber); - - zend_observer_fiber_switch_notify(previous, fiber); + zend_observer_fiber_switch_notify(EG(current_fiber), context); ZEND_FIBER_BACKUP_EG(stack, stack_page_size, execute_data, error_reporting, jit_trace_num, bailout); - EG(current_fiber) = fiber; + fiber->caller = zend_fiber_from_context(EG(current_fiber)); - zend_fiber_switch_context(&fiber->context); + zend_fiber_switch_context(EG(current_fiber), context); - EG(current_fiber) = previous; + fiber->caller = NULL; ZEND_FIBER_RESTORE_EG(stack, stack_page_size, execute_data, error_reporting, jit_trace_num, bailout); - zend_observer_fiber_switch_notify(fiber, previous); + zend_observer_fiber_switch_notify(context, EG(current_fiber)); if (UNEXPECTED(fiber->flags & ZEND_FIBER_FLAG_BAILOUT)) { // zend_bailout() was called in the fiber, so call it again in the previous fiber or {main}. @@ -339,9 +323,9 @@ static zend_always_inline zend_vm_stack zend_fiber_vm_stack_alloc(size_t size) return page; } -static void ZEND_STACK_ALIGNED zend_fiber_execute(zend_fiber_context *context) +static ZEND_STACK_ALIGNED zend_fiber_context *zend_fiber_execute(zend_fiber_context *context) { - zend_fiber *fiber = EG(current_fiber); + zend_fiber *fiber = zend_fiber_from_context(EG(current_fiber)); ZEND_ASSERT(fiber); zend_long error_reporting = INI_INT("error_reporting"); @@ -396,6 +380,8 @@ static void ZEND_STACK_ALIGNED zend_fiber_execute(zend_fiber_context *context) zend_vm_stack_destroy(); fiber->execute_data = NULL; fiber->stack_bottom = NULL; + + return zend_fiber_get_context(fiber->caller); } static zend_object *zend_fiber_object_create(zend_class_entry *ce) @@ -454,7 +440,7 @@ static void zend_fiber_object_free(zend_object *object) zval_ptr_dtor(&fiber->value); - zend_fiber_destroy_context(&fiber->context); + zend_fiber_destroy_context(zend_fiber_get_context(fiber)); zend_object_std_dtor(&fiber->std); } @@ -494,7 +480,7 @@ ZEND_API void zend_fiber_start(zend_fiber *fiber, zval *params, uint32_t param_c fiber->fci.param_count = param_count; fiber->fci.named_params = named_params; - if (!zend_fiber_init_context(&fiber->context, zend_fiber_execute, EG(fiber_stack_size))) { + if (!zend_fiber_init_context(zend_fiber_get_context(fiber), zend_fiber_execute, EG(fiber_stack_size))) { RETURN_THROWS(); } @@ -524,13 +510,13 @@ ZEND_METHOD(Fiber, start) ZEND_API void zend_fiber_suspend(zval *value, zval *return_value) { - zend_fiber *fiber = EG(current_fiber); - - if (UNEXPECTED(!fiber)) { + if (UNEXPECTED(EG(current_fiber) == EG(main_fiber))) { zend_throw_error(zend_ce_fiber_error, "Cannot suspend outside of a fiber"); RETURN_THROWS(); } + zend_fiber *fiber = zend_fiber_from_context(EG(current_fiber)); + if (UNEXPECTED(fiber->flags & ZEND_FIBER_FLAG_DESTROYED)) { zend_throw_error(zend_ce_fiber_error, "Cannot suspend in a force-closed fiber"); RETURN_THROWS(); @@ -734,17 +720,13 @@ ZEND_METHOD(Fiber, getReturn) ZEND_METHOD(Fiber, this) { - zend_fiber *fiber; - ZEND_PARSE_PARAMETERS_NONE(); - fiber = EG(current_fiber); - - if (!fiber) { + if (EG(current_fiber) == EG(main_fiber)) { RETURN_NULL(); } - RETURN_OBJ_COPY(&fiber->std); + RETURN_OBJ_COPY(&zend_fiber_from_context(EG(current_fiber))->std); } ZEND_METHOD(FiberError, __construct) @@ -775,5 +757,15 @@ void zend_register_fiber_ce(void) void zend_fiber_init(void) { - EG(current_fiber) = NULL; + zend_fiber_context *context = ecalloc(1, sizeof(zend_fiber_context)); + + context->status = ZEND_FIBER_STATUS_RUNNING; + + EG(main_fiber) = context; + EG(current_fiber) = context; +} + +void zend_fiber_shutdown(void) +{ + efree(EG(main_fiber)); } diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index b26e22e9a888a..31b1d12000f04 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -40,13 +40,10 @@ typedef enum { void zend_register_fiber_ce(void); void zend_fiber_init(void); +void zend_fiber_shutdown(void); extern ZEND_API zend_class_entry *zend_ce_fiber; -typedef struct _zend_fiber_context zend_fiber_context; - -typedef void (*zend_fiber_coroutine)(zend_fiber_context *context); - typedef struct _zend_fiber_stack { void *pointer; size_t size; @@ -61,30 +58,36 @@ typedef struct _zend_fiber_stack { #endif } zend_fiber_stack; -typedef struct _zend_fiber_context { - void *self; - void *caller; - zend_fiber_coroutine function; - zend_fiber_stack stack; -} zend_fiber_context; +typedef struct _zend_fiber zend_fiber; +typedef struct _zend_fiber_context zend_fiber_context; + +typedef zend_fiber_context *(*zend_fiber_coroutine)(zend_fiber_context *context); -typedef struct _zend_fiber { - /* Fiber PHP object handle. */ +#define ZEND_FIBER_CONTEXT_FIELDS \ + void *handle; \ + zend_fiber_coroutine function; \ + zend_fiber_stack stack; \ + zend_fiber_status status; \ + zend_uchar flags + +struct _zend_fiber_context { + ZEND_FIBER_CONTEXT_FIELDS; +}; + +struct _zend_fiber { + /* PHP object handle. */ zend_object std; - /* Status of the fiber, one of the zend_fiber_status values. */ - zend_fiber_status status; + /* Fiber that resumed us. */ + zend_fiber *caller; - /* Flags of the fiber, bit field of the zend_fiber_flag values. */ - zend_uchar flags; + /* Fiber context fields (embedded to avoid memory allocation). */ + ZEND_FIBER_CONTEXT_FIELDS; /* Callback and info / cache to be used when fiber is started. */ zend_fcall_info fci; zend_fcall_info_cache fci_cache; - /* Context of this fiber, will be initialized during call to Fiber::start(). */ - zend_fiber_context context; - /* Current Zend VM execute data being run by the fiber. */ zend_execute_data *execute_data; @@ -96,7 +99,7 @@ typedef struct _zend_fiber { /* Storage for temporaries and fiber return value. */ zval value; -} zend_fiber; +}; /* These functions create and manipulate a Fiber object, allowing any internal function to start, resume, or suspend a fiber. */ ZEND_API zend_fiber *zend_fiber_create(const zend_fcall_info *fci, const zend_fcall_info_cache *fci_cache); @@ -108,8 +111,7 @@ ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_ /* These functions may be used to create custom fibers (coroutines) using the bundled fiber switching context. */ ZEND_API zend_bool zend_fiber_init_context(zend_fiber_context *context, zend_fiber_coroutine coroutine, size_t stack_size); ZEND_API void zend_fiber_destroy_context(zend_fiber_context *context); -ZEND_API void zend_fiber_switch_context(zend_fiber_context *to); -ZEND_API void zend_fiber_suspend_context(zend_fiber_context *current); +ZEND_API void zend_fiber_switch_context(zend_fiber_context *from, zend_fiber_context *to); #define ZEND_FIBER_GUARD_PAGES 1 @@ -119,4 +121,14 @@ ZEND_API void zend_fiber_suspend_context(zend_fiber_context *current); END_EXTERN_C() +static zend_always_inline zend_fiber *zend_fiber_from_context(zend_fiber_context *context) +{ + return (zend_fiber *)(((char *) context) - XtOffsetOf(zend_fiber, handle)); +} + +static zend_always_inline zend_fiber_context *zend_fiber_get_context(zend_fiber *fiber) +{ + return (zend_fiber_context *) &fiber->handle; +} + #endif diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h index cb627377ae629..b2e7e03cca3d2 100644 --- a/Zend/zend_globals.h +++ b/Zend/zend_globals.h @@ -61,8 +61,7 @@ END_EXTERN_C() typedef struct _zend_vm_stack *zend_vm_stack; typedef struct _zend_ini_entry zend_ini_entry; -typedef struct _zend_fiber zend_fiber; - +typedef struct _zend_fiber_context zend_fiber_context; struct _zend_compiler_globals { zend_stack loop_var_stack; @@ -250,8 +249,8 @@ struct _zend_executor_globals { zend_get_gc_buffer get_gc_buffer; - /* Active fiber, NULL when in main thread. */ - zend_fiber *current_fiber; + zend_fiber_context *main_fiber; + zend_fiber_context *current_fiber; /* Default fiber C stack size. */ zend_long fiber_stack_size; diff --git a/Zend/zend_observer.c b/Zend/zend_observer.c index 7b5703ae627cd..cb528d9616f98 100644 --- a/Zend/zend_observer.c +++ b/Zend/zend_observer.c @@ -264,7 +264,7 @@ ZEND_API void zend_observer_fiber_switch_register(zend_observer_fiber_switch_han zend_llist_add_element(&zend_observer_fiber_switch, &handler); } -ZEND_API void ZEND_FASTCALL zend_observer_fiber_switch_notify(zend_fiber *from, zend_fiber *to) +ZEND_API void ZEND_FASTCALL zend_observer_fiber_switch_notify(zend_fiber_context *from, zend_fiber_context *to) { zend_llist_element *element; zend_observer_fiber_switch_handler callback; diff --git a/Zend/zend_observer.h b/Zend/zend_observer.h index 9145f4f8620fe..09d71daef3ac5 100644 --- a/Zend/zend_observer.h +++ b/Zend/zend_observer.h @@ -78,10 +78,10 @@ typedef void (*zend_observer_error_cb)(int type, zend_string *error_filename, ui ZEND_API void zend_observer_error_register(zend_observer_error_cb callback); void zend_observer_error_notify(int type, zend_string *error_filename, uint32_t error_lineno, zend_string *message); -typedef void (*zend_observer_fiber_switch_handler)(zend_fiber *from, zend_fiber *to); +typedef void (*zend_observer_fiber_switch_handler)(zend_fiber_context *from, zend_fiber_context *to); ZEND_API void zend_observer_fiber_switch_register(zend_observer_fiber_switch_handler handler); -ZEND_API void ZEND_FASTCALL zend_observer_fiber_switch_notify(zend_fiber *from, zend_fiber *to); +ZEND_API void ZEND_FASTCALL zend_observer_fiber_switch_notify(zend_fiber_context *from, zend_fiber_context *to); END_EXTERN_C() diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 56e00800d428a..7e4a26ad8eaa7 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -6895,7 +6895,7 @@ ZEND_METHOD(ReflectionFiber, getTrace) prev_execute_data = fiber->stack_bottom->prev_execute_data; fiber->stack_bottom->prev_execute_data = NULL; - if (EG(current_fiber) != fiber) { + if (EG(current_fiber) != zend_fiber_get_context(fiber)) { // No need to replace current execute data if within the current fiber. EG(current_execute_data) = fiber->execute_data; } @@ -6915,7 +6915,7 @@ ZEND_METHOD(ReflectionFiber, getExecutingLine) REFLECTION_CHECK_VALID_FIBER(fiber); - if (EG(current_fiber) == fiber) { + if (EG(current_fiber) == zend_fiber_get_context(fiber)) { prev_execute_data = execute_data->prev_execute_data; } else { prev_execute_data = fiber->execute_data->prev_execute_data; @@ -6933,7 +6933,7 @@ ZEND_METHOD(ReflectionFiber, getExecutingFile) REFLECTION_CHECK_VALID_FIBER(fiber); - if (EG(current_fiber) == fiber) { + if (EG(current_fiber) == zend_fiber_get_context(fiber)) { prev_execute_data = execute_data->prev_execute_data; } else { prev_execute_data = fiber->execute_data->prev_execute_data; diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 7422f25bbb39c..42507db11e9cd 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -411,44 +411,40 @@ static void observer_set_user_opcode_handler(const char *opcode_names, user_opco } } -static void fiber_address_observer(zend_fiber *from, zend_fiber *to) +static void fiber_address_observer(zend_fiber_context *from, zend_fiber_context *to) { if (ZT_G(observer_fiber_switch)) { php_printf("\n", from, to); } } -static void fiber_enter_observer(zend_fiber *from, zend_fiber *to) +static void fiber_enter_observer(zend_fiber_context *from, zend_fiber_context *to) { if (ZT_G(observer_fiber_switch)) { - if (to) { - if (to->status == ZEND_FIBER_STATUS_INIT) { - php_printf("\n", to); - } else if (to->status == ZEND_FIBER_STATUS_RUNNING && (!from || from->status == ZEND_FIBER_STATUS_RUNNING)) { - if (to->flags & ZEND_FIBER_FLAG_DESTROYED) { - php_printf("\n", to); - } else if (to->status != ZEND_FIBER_STATUS_DEAD) { - php_printf("\n", to); - } + if (to->status == ZEND_FIBER_STATUS_INIT) { + php_printf("\n", to); + } else if (to->status == ZEND_FIBER_STATUS_RUNNING && from->status == ZEND_FIBER_STATUS_RUNNING) { + if (to->flags & ZEND_FIBER_FLAG_DESTROYED) { + php_printf("\n", to); + } else if (to->status != ZEND_FIBER_STATUS_DEAD) { + php_printf("\n", to); } } } } -static void fiber_suspend_observer(zend_fiber *from, zend_fiber *to) +static void fiber_suspend_observer(zend_fiber_context *from, zend_fiber_context *to) { if (ZT_G(observer_fiber_switch)) { - if (from) { - if (from->status == ZEND_FIBER_STATUS_SUSPENDED) { - php_printf("\n", from); - } else if (from->status == ZEND_FIBER_STATUS_DEAD) { - if (from->flags & ZEND_FIBER_FLAG_THREW) { - php_printf("\n", from); - } else if (from->flags & ZEND_FIBER_FLAG_DESTROYED) { - php_printf("\n", from); - } else { - php_printf("\n", from); - } + if (from->status == ZEND_FIBER_STATUS_SUSPENDED) { + php_printf("\n", from); + } else if (from->status == ZEND_FIBER_STATUS_DEAD) { + if (from->flags & ZEND_FIBER_FLAG_THREW) { + php_printf("\n", from); + } else if (from->flags & ZEND_FIBER_FLAG_DESTROYED) { + php_printf("\n", from); + } else { + php_printf("\n", from); } } } diff --git a/ext/zend_test/tests/observer_fiber_01.phpt b/ext/zend_test/tests/observer_fiber_01.phpt index 6b15d814a524b..5ee2aa0e0c6a1 100644 --- a/ext/zend_test/tests/observer_fiber_01.phpt +++ b/ext/zend_test/tests/observer_fiber_01.phpt @@ -18,12 +18,12 @@ $fiber->resume(); ?> --EXPECTF-- - + - + - + - + diff --git a/ext/zend_test/tests/observer_fiber_02.phpt b/ext/zend_test/tests/observer_fiber_02.phpt index 212d9e6ad8bc0..5f023d8ce1656 100644 --- a/ext/zend_test/tests/observer_fiber_02.phpt +++ b/ext/zend_test/tests/observer_fiber_02.phpt @@ -17,12 +17,12 @@ $fiber->start(); ?> --EXPECTF-- - + - + - + - + diff --git a/ext/zend_test/tests/observer_fiber_03.phpt b/ext/zend_test/tests/observer_fiber_03.phpt index 4f3de09ae4eee..bc1a5093ddd6e 100644 --- a/ext/zend_test/tests/observer_fiber_03.phpt +++ b/ext/zend_test/tests/observer_fiber_03.phpt @@ -40,12 +40,12 @@ $fiber->resume(); ?> --EXPECTF-- - + - + - + int(1) @@ -53,9 +53,9 @@ int(1) - + - + int(2) @@ -63,9 +63,9 @@ int(2) int(3) - + - + int(4) @@ -73,5 +73,5 @@ int(4) int(5) - + diff --git a/ext/zend_test/tests/observer_fiber_04.phpt b/ext/zend_test/tests/observer_fiber_04.phpt index 489a95dbc4f14..92ca237ae3167 100644 --- a/ext/zend_test/tests/observer_fiber_04.phpt +++ b/ext/zend_test/tests/observer_fiber_04.phpt @@ -27,25 +27,25 @@ $fiber->resume(); ?> --EXPECTF-- - + - + - + - + - + - + diff --git a/ext/zend_test/tests/observer_fiber_05.phpt b/ext/zend_test/tests/observer_fiber_05.phpt index 9cb0ab0254f78..6e246e79d5175 100644 --- a/ext/zend_test/tests/observer_fiber_05.phpt +++ b/ext/zend_test/tests/observer_fiber_05.phpt @@ -26,25 +26,25 @@ $fiber->resume(); ?> --EXPECTF-- - + - + - + - + - + - + diff --git a/ext/zend_test/tests/observer_fiber_06.phpt b/ext/zend_test/tests/observer_fiber_06.phpt index 2b3f2ec1a9640..aae1bc9fa1cb3 100644 --- a/ext/zend_test/tests/observer_fiber_06.phpt +++ b/ext/zend_test/tests/observer_fiber_06.phpt @@ -23,12 +23,12 @@ try { ?> --EXPECTF-- - + - + - + - + From ab67660dedeb491db5b94c02e22242b5eba5819f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Schr=C3=B6der?= Date: Fri, 4 Jun 2021 18:59:47 +0200 Subject: [PATCH 3/6] Simplified VM state capture / restore and made it reusable. --- Zend/zend_fibers.c | 48 ++++++---------------------------------------- Zend/zend_fibers.h | 33 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 9be34f93a951c..786cef81efbdb 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -78,28 +78,6 @@ extern transfer_t jump_fcontext(fcontext_t to, void *vp); #define ZEND_FIBER_DEFAULT_PAGE_SIZE 4096 -#define ZEND_FIBER_BACKUP_EG(stack, stack_page_size, execute_data, error_reporting, trace_num, bailout) do { \ - stack = EG(vm_stack); \ - stack->top = EG(vm_stack_top); \ - stack->end = EG(vm_stack_end); \ - stack_page_size = EG(vm_stack_page_size); \ - execute_data = EG(current_execute_data); \ - error_reporting = EG(error_reporting); \ - trace_num = EG(jit_trace_num); \ - bailout = EG(bailout); \ -} while (0) - -#define ZEND_FIBER_RESTORE_EG(stack, stack_page_size, execute_data, error_reporting, trace_num, bailout) do { \ - EG(vm_stack) = stack; \ - EG(vm_stack_top) = stack->top; \ - EG(vm_stack_end) = stack->end; \ - EG(vm_stack_page_size) = stack_page_size; \ - EG(current_execute_data) = execute_data; \ - EG(error_reporting) = error_reporting; \ - EG(jit_trace_num) = trace_num; \ - EG(bailout) = bailout; \ -} while (0) - static size_t zend_fiber_get_page_size(void) { static size_t page_size = 0; @@ -265,44 +243,30 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_context *from, zend_fiber_con static void zend_fiber_suspend_from(zend_fiber *fiber) { - zend_vm_stack stack; - size_t stack_page_size; - zend_execute_data *execute_data; - int error_reporting; - uint32_t jit_trace_num; - JMP_BUF *bailout; + zend_fiber_vm_state state; ZEND_ASSERT(fiber->caller && "Fiber has no caller"); - ZEND_FIBER_BACKUP_EG(stack, stack_page_size, execute_data, error_reporting, jit_trace_num, bailout); - + zend_fiber_capture_vm_state(&state); zend_fiber_switch_context(zend_fiber_get_context(fiber), zend_fiber_get_context(fiber->caller)); - - ZEND_FIBER_RESTORE_EG(stack, stack_page_size, execute_data, error_reporting, jit_trace_num, bailout); + zend_fiber_restore_vm_state(&state); } static void zend_fiber_switch_to(zend_fiber *fiber) { zend_fiber_context *context = zend_fiber_get_context(fiber); - zend_vm_stack stack; - size_t stack_page_size; - zend_execute_data *execute_data; - int error_reporting; - uint32_t jit_trace_num; - JMP_BUF *bailout; + zend_fiber_vm_state state; zend_observer_fiber_switch_notify(EG(current_fiber), context); - ZEND_FIBER_BACKUP_EG(stack, stack_page_size, execute_data, error_reporting, jit_trace_num, bailout); - fiber->caller = zend_fiber_from_context(EG(current_fiber)); + zend_fiber_capture_vm_state(&state); zend_fiber_switch_context(EG(current_fiber), context); + zend_fiber_restore_vm_state(&state); fiber->caller = NULL; - ZEND_FIBER_RESTORE_EG(stack, stack_page_size, execute_data, error_reporting, jit_trace_num, bailout); - zend_observer_fiber_switch_notify(context, EG(current_fiber)); if (UNEXPECTED(fiber->flags & ZEND_FIBER_FLAG_BAILOUT)) { diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index 31b1d12000f04..248ea9a3c74b4 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -74,6 +74,15 @@ struct _zend_fiber_context { ZEND_FIBER_CONTEXT_FIELDS; }; +typedef struct _zend_fiber_vm_state { + zend_vm_stack vm_stack; + size_t vm_stack_page_size; + zend_execute_data *current_execute_data; + int error_reporting; + uint32_t jit_trace_num; + JMP_BUF *bailout; +} zend_fiber_vm_state; + struct _zend_fiber { /* PHP object handle. */ zend_object std; @@ -131,4 +140,28 @@ static zend_always_inline zend_fiber_context *zend_fiber_get_context(zend_fiber return (zend_fiber_context *) &fiber->handle; } +static zend_always_inline void zend_fiber_capture_vm_state(zend_fiber_vm_state *state) +{ + state->vm_stack = EG(vm_stack); + state->vm_stack->top = EG(vm_stack_top); + state->vm_stack->end = EG(vm_stack_end); + state->vm_stack_page_size = EG(vm_stack_page_size); + state->current_execute_data = EG(current_execute_data); + state->error_reporting = EG(error_reporting); + state->jit_trace_num = EG(jit_trace_num); + state->bailout = EG(bailout); +} + +static zend_always_inline void zend_fiber_restore_vm_state(zend_fiber_vm_state *state) +{ + EG(vm_stack) = state->vm_stack; + EG(vm_stack_top) = state->vm_stack->top; + EG(vm_stack_end) = state->vm_stack->end; + EG(vm_stack_page_size) = state->vm_stack_page_size; + EG(current_execute_data) = state->current_execute_data; + EG(error_reporting) = state->error_reporting; + EG(jit_trace_num) = state->jit_trace_num; + EG(bailout) = state->bailout; +} + #endif From b2d82ff4ae4e5eea648a1a28f04427a079068470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Schr=C3=B6der?= Date: Fri, 4 Jun 2021 19:13:27 +0200 Subject: [PATCH 4/6] Further simplified fiber context switching. --- Zend/zend_fibers.c | 10 ++++++---- Zend/zend_fibers.h | 18 +++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 786cef81efbdb..05b91e085b1af 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -192,7 +192,7 @@ static ZEND_NORETURN void zend_fiber_trampoline(transfer_t transfer) __sanitizer_start_switch_fiber(NULL, context->stack.prior_pointer, context->stack.prior_size); #endif - zend_fiber_switch_context(context, to); + zend_fiber_switch_context(to); abort(); } @@ -219,8 +219,10 @@ ZEND_API void zend_fiber_destroy_context(zend_fiber_context *context) zend_fiber_stack_free(&context->stack); } -ZEND_API void zend_fiber_switch_context(zend_fiber_context *from, zend_fiber_context *to) +ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) { + zend_fiber_context *from = EG(current_fiber); + ZEND_ASSERT(to && to->handle && "Invalid fiber context"); ZEND_ASSERT(from && "From context must be present"); @@ -248,7 +250,7 @@ static void zend_fiber_suspend_from(zend_fiber *fiber) ZEND_ASSERT(fiber->caller && "Fiber has no caller"); zend_fiber_capture_vm_state(&state); - zend_fiber_switch_context(zend_fiber_get_context(fiber), zend_fiber_get_context(fiber->caller)); + zend_fiber_switch_context(zend_fiber_get_context(fiber->caller)); zend_fiber_restore_vm_state(&state); } @@ -262,7 +264,7 @@ static void zend_fiber_switch_to(zend_fiber *fiber) fiber->caller = zend_fiber_from_context(EG(current_fiber)); zend_fiber_capture_vm_state(&state); - zend_fiber_switch_context(EG(current_fiber), context); + zend_fiber_switch_context(context); zend_fiber_restore_vm_state(&state); fiber->caller = NULL; diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index 248ea9a3c74b4..db109d2906ae2 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -23,6 +23,11 @@ #include "zend_API.h" #include "zend_types.h" +#define ZEND_FIBER_GUARD_PAGES 1 + +#define ZEND_FIBER_DEFAULT_C_STACK_SIZE (4096 * (((sizeof(void *)) < 8) ? 256 : 512)) +#define ZEND_FIBER_VM_STACK_SIZE (1024 * sizeof(zval)) + BEGIN_EXTERN_C() typedef enum { @@ -44,6 +49,7 @@ void zend_fiber_shutdown(void); extern ZEND_API zend_class_entry *zend_ce_fiber; +/* Encapsulates the fiber C stack with extension for debugging tools. */ typedef struct _zend_fiber_stack { void *pointer; size_t size; @@ -61,8 +67,10 @@ typedef struct _zend_fiber_stack { typedef struct _zend_fiber zend_fiber; typedef struct _zend_fiber_context zend_fiber_context; +/* Fiber function is passed the fiber context and must return another context to switch to after it ran to completion. */ typedef zend_fiber_context *(*zend_fiber_coroutine)(zend_fiber_context *context); +/* Defined as a macro to allow anonymous embedding. */ #define ZEND_FIBER_CONTEXT_FIELDS \ void *handle; \ zend_fiber_coroutine function; \ @@ -70,10 +78,12 @@ typedef zend_fiber_context *(*zend_fiber_coroutine)(zend_fiber_context *context) zend_fiber_status status; \ zend_uchar flags +/* Standalone context (used primarily as pointer type). */ struct _zend_fiber_context { ZEND_FIBER_CONTEXT_FIELDS; }; +/* Zend VM that needs to be captured / restored during context switch. */ typedef struct _zend_fiber_vm_state { zend_vm_stack vm_stack; size_t vm_stack_page_size; @@ -120,13 +130,7 @@ ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_ /* These functions may be used to create custom fibers (coroutines) using the bundled fiber switching context. */ ZEND_API zend_bool zend_fiber_init_context(zend_fiber_context *context, zend_fiber_coroutine coroutine, size_t stack_size); ZEND_API void zend_fiber_destroy_context(zend_fiber_context *context); -ZEND_API void zend_fiber_switch_context(zend_fiber_context *from, zend_fiber_context *to); - -#define ZEND_FIBER_GUARD_PAGES 1 - -#define ZEND_FIBER_DEFAULT_C_STACK_SIZE (4096 * (((sizeof(void *)) < 8) ? 256 : 512)) - -#define ZEND_FIBER_VM_STACK_SIZE (1024 * sizeof(zval)) +ZEND_API void zend_fiber_switch_context(zend_fiber_context *to); END_EXTERN_C() From 85148407c8efdc58520b6c9d4af1790c296412d3 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Fri, 4 Jun 2021 13:47:58 -0500 Subject: [PATCH 5/6] Fix ASAN --- Zend/zend_fibers.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 05b91e085b1af..c486bc12513c8 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -150,6 +150,11 @@ static bool zend_fiber_stack_allocate(zend_fiber_stack *stack, size_t size) stack->valgrind = VALGRIND_STACK_REGISTER(base, base + stack->size); #endif +#ifdef __SANITIZE_ADDRESS__ + stack->prior_pointer = stack->pointer; + stack->prior_size = stack->size; +#endif + return true; } @@ -179,19 +184,16 @@ static void zend_fiber_stack_free(zend_fiber_stack *stack) static ZEND_NORETURN void zend_fiber_trampoline(transfer_t transfer) { zend_fiber_context *context = EG(current_fiber); + zend_fiber_context *from = transfer.data; - ((zend_fiber_context *) transfer.data)->handle = transfer.context; + from->handle = transfer.context; #ifdef __SANITIZE_ADDRESS__ - __sanitizer_finish_switch_fiber(NULL, &context->stack.prior_pointer, &context->stack.prior_size); + __sanitizer_finish_switch_fiber(NULL, &from->stack.prior_pointer, &from->stack.prior_size); #endif zend_fiber_context *to = context->function(context); -#ifdef __SANITIZE_ADDRESS__ - __sanitizer_start_switch_fiber(NULL, context->stack.prior_pointer, context->stack.prior_size); -#endif - zend_fiber_switch_context(to); abort(); @@ -227,8 +229,11 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) ZEND_ASSERT(from && "From context must be present"); #ifdef __SANITIZE_ADDRESS__ - void *fake_stack; - __sanitizer_start_switch_fiber(&fake_stack, to->stack.pointer, to->stack.size); + void *fake_stack = NULL; + __sanitizer_start_switch_fiber( + from->status != ZEND_FIBER_STATUS_DEAD ? &fake_stack : NULL, + to->stack.prior_pointer, + to->stack.prior_size); #endif EG(current_fiber) = to; @@ -291,8 +296,7 @@ static zend_always_inline zend_vm_stack zend_fiber_vm_stack_alloc(size_t size) static ZEND_STACK_ALIGNED zend_fiber_context *zend_fiber_execute(zend_fiber_context *context) { - zend_fiber *fiber = zend_fiber_from_context(EG(current_fiber)); - ZEND_ASSERT(fiber); + zend_fiber *fiber = zend_fiber_from_context(context); zend_long error_reporting = INI_INT("error_reporting"); if (!error_reporting && !INI_STR("error_reporting")) { From 2adc8529992f329435f232570973d0c96d5c65b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Schr=C3=B6der?= Date: Fri, 4 Jun 2021 22:25:35 +0200 Subject: [PATCH 6/6] Adjusted a few comment. Assert from and to context are not the same. --- Zend/zend_fibers.c | 9 +++++---- Zend/zend_fibers.h | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index c486bc12513c8..9178b248f202b 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -192,10 +192,10 @@ static ZEND_NORETURN void zend_fiber_trampoline(transfer_t transfer) __sanitizer_finish_switch_fiber(NULL, &from->stack.prior_pointer, &from->stack.prior_size); #endif - zend_fiber_context *to = context->function(context); - - zend_fiber_switch_context(to); + /* Final context switch, the fiber must not be resumed afterwards! */ + zend_fiber_switch_context(context->function(context)); + /* Abort here because we are in an inconsistent program state. */ abort(); } @@ -226,7 +226,8 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) zend_fiber_context *from = EG(current_fiber); ZEND_ASSERT(to && to->handle && "Invalid fiber context"); - ZEND_ASSERT(from && "From context must be present"); + ZEND_ASSERT(from && "From fiber context must be present"); + ZEND_ASSERT(to != from && "Cannot switch into the running fiber context"); #ifdef __SANITIZE_ADDRESS__ void *fake_stack = NULL; diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index db109d2906ae2..84148f0431886 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -67,7 +67,7 @@ typedef struct _zend_fiber_stack { typedef struct _zend_fiber zend_fiber; typedef struct _zend_fiber_context zend_fiber_context; -/* Fiber function is passed the fiber context and must return another context to switch to after it ran to completion. */ +/* Coroutine functions must return a fiber context to switch to after they are finished. */ typedef zend_fiber_context *(*zend_fiber_coroutine)(zend_fiber_context *context); /* Defined as a macro to allow anonymous embedding. */ @@ -83,7 +83,7 @@ struct _zend_fiber_context { ZEND_FIBER_CONTEXT_FIELDS; }; -/* Zend VM that needs to be captured / restored during context switch. */ +/* Zend VM state that needs to be captured / restored during fiber context switch. */ typedef struct _zend_fiber_vm_state { zend_vm_stack vm_stack; size_t vm_stack_page_size;