From 35c5a7cba8c0668335450e0ba1ffe65646b271b7 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 8 Apr 2022 10:31:49 -0600 Subject: [PATCH 1/5] Make vm_interrupt and timed_out atomic This is done by adding a new zend_atomic_bool type. The type definition is only available for compiler alignment and size info; it should be treated as opaque and only the zend_atomic_bool_* family of functions should be used. Note that directly using atomic_bool is complicated. All C++ compilers stdlibs that I checked typedef atomic_bool to std::atomic, which can't be used in an extern "C" section, and there's at least one usage of this in core, and probably more outside of it. So, instead use platform specific functions, preferring compiler intrinsics. --- Zend/zend_atomic.c | 40 ++++++++ Zend/zend_atomic.h | 171 +++++++++++++++++++++++++++++++ Zend/zend_execute.c | 4 +- Zend/zend_execute_API.c | 29 +++--- Zend/zend_globals.h | 5 +- Zend/zend_vm_def.h | 4 +- Zend/zend_vm_execute.h | 4 +- configure.ac | 2 +- ext/opcache/jit/zend_jit_trace.c | 2 +- ext/pcntl/pcntl.c | 2 +- sapi/phpdbg/phpdbg_prompt.c | 2 +- win32/build/config.w32 | 2 +- win32/signal.c | 4 +- 13 files changed, 241 insertions(+), 30 deletions(-) create mode 100644 Zend/zend_atomic.c create mode 100644 Zend/zend_atomic.h diff --git a/Zend/zend_atomic.c b/Zend/zend_atomic.c new file mode 100644 index 000000000000..22d98fcb4134 --- /dev/null +++ b/Zend/zend_atomic.c @@ -0,0 +1,40 @@ +/* + +----------------------------------------------------------------------+ + | This source file is subject to version 3.01 of the PHP license, | + | that is bundled with this package in the file LICENSE, and is | + | available through the world-wide-web at the following url: | + | https://www.php.net/license/3_01.txt | + | If you did not receive a copy of the PHP license and are unable to | + | obtain it through the world-wide-web, please send a note to | + | license@php.net so we can mail you a copy immediately. | + +----------------------------------------------------------------------+ + | Authors: Levi Morrison | + +----------------------------------------------------------------------+ + */ + +#include "zend_atomic.h" + +/* This file contains the non-inline copy of atomic functions. This is useful + * for extensions written in languages such as Rust. C and C++ compilers are + * probably going to inline these functions, but in the case they don't, this + * is also where the code will go. + */ + +/* Defined for FFI users; everyone else use ZEND_ATOMIC_BOOL_INIT. + * This is NOT ATOMIC as it is meant for initialization. + */ +ZEND_API void zend_atomic_bool_init(zend_atomic_bool *obj, bool desired) { + ZEND_ATOMIC_BOOL_INIT(obj, desired); +} + +extern inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, + bool desired); +extern inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, + bool desired); + +#if ZEND_WIN32 || HAVE_SYNC_ATOMICS +/* On these platforms it is non-const due to underlying APIs. */ +extern inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj); +#else +extern inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj); +#endif diff --git a/Zend/zend_atomic.h b/Zend/zend_atomic.h new file mode 100644 index 000000000000..37397b66295e --- /dev/null +++ b/Zend/zend_atomic.h @@ -0,0 +1,171 @@ +/* + +----------------------------------------------------------------------+ + | This source file is subject to version 3.01 of the PHP license, | + | that is bundled with this package in the file LICENSE, and is | + | available through the world-wide-web at the following url: | + | https://www.php.net/license/3_01.txt | + | If you did not receive a copy of the PHP license and are unable to | + | obtain it through the world-wide-web, please send a note to | + | license@php.net so we can mail you a copy immediately. | + +----------------------------------------------------------------------+ + | Authors: Levi Morrison | + +----------------------------------------------------------------------+ + */ + +#ifndef ZEND_ATOMIC_H +#define ZEND_ATOMIC_H + +#include "zend_portability.h" + +#include + +#ifndef __has_feature +#define __has_feature(x) 0 +#endif + +#define ZEND_GCC_PREREQ(x, y) \ + ((__GNUC__ == (x) && __GNUC_MINOR__ >= (y)) || (__GNUC__ > (x))) + +/* Builtins are used to avoid library linkage */ +#if __has_feature(c_atomic) +#define HAVE_C11_ATOMICS 1 +#elif ZEND_GCC_PREREQ(4, 7) +#define HAVE_GNUC_ATOMICS 1 +#elif defined(__GNUC__) +#define HAVE_SYNC_ATOMICS 1 +#elif !defined(ZEND_WIN32) +#define HAVE_NO_ATOMICS 1 +#endif + +#undef ZEND_GCC_PREREQ + +/* Treat zend_atomic_* types as opaque. They have definitions only for size + * and alignment purposes. + */ + +#if ZEND_WIN32 || HAVE_SYNC_ATOMICS +typedef struct zend_atomic_bool_s { + volatile char value; +} zend_atomic_bool; +#elif HAVE_C11_ATOMICS +typedef struct zend_atomic_bool_s { + _Atomic(bool) value; +} zend_atomic_bool; +#else +typedef struct zend_atomic_bool_s { + volatile bool value; +} zend_atomic_bool; +#endif + +BEGIN_EXTERN_C() + +#if ZEND_WIN32 + +#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) + +inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { + return InterlockedExchange8(&obj->value, desired); +} + +/* On this platform it is non-const due to Iterlocked API*/ +inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj) { + /* Or'ing with false won't change the value. */ + return InterlockedOr8(&obj->value, false); +} + +inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { + (void)InterlockedExchange8(&obj->value, desired); +} + +#elif HAVE_C11_ATOMICS + +#define ZEND_ATOMIC_BOOL_INIT(obj, desired) __c11_atomic_init(&(obj)->value, (desired)) + +inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { + return __c11_atomic_exchange(&obj->value, desired, __ATOMIC_SEQ_CST); +} + +inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) { + return __c11_atomic_load(&obj->value, __ATOMIC_SEQ_CST); +} + +inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { + __c11_atomic_store(&obj->value, desired, __ATOMIC_SEQ_CST); +} + +#elif HAVE_GNUC_ATOMICS + +#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) + +inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { + bool prev = false; + __atomic_exchange(&obj->value, &desired, &prev, __ATOMIC_SEQ_CST); + return prev; +} + +inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) { + bool prev = false; + __atomic_load(&obj->value, &prev, __ATOMIC_SEQ_CST); + return prev; +} + +inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { + __atomic_store(&obj->value, &desired, __ATOMIC_SEQ_CST); +} + +#elif HAVE_SYNC_ATOMICS + +#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) + +inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { + bool prev = __sync_lock_test_and_set(&obj->value, desired); + + /* __sync_lock_test_and_set only does an acquire barrier, so sync + * immediately after. + */ + __sync_synchronize(); + return prev; +} + +inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj) { + /* Or'ing false won't change the value */ + return __sync_fetch_and_or(&obj->value, false); +} + +inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { + __sync_synchronize(); + obj->value = desired; + __sync_synchronize(); +} + +#elif HAVE_NO_ATOMICS + +#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) + +/* Yes, these are not guaranteed to be atomic. Understand that previously + * atomics were never used, so the fact they are sometimes used is an + * improvement. As more platforms support C11 atomics, or as we add support + * for more platforms through intrinsics/asm, this should be used less and + * less until it can be removed. + * At the time of this writing, all platforms in CI avoided this fallback. + */ + +inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { + obj->value = desired; +} + +inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) { + return obj->value; +} + +inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { + bool prev = obj->value; + obj->value = true; + return prev; +} + +#endif + +END_EXTERN_C() + +#endif diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 436a5a2fd2a9..9f3b15cafe2b 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -3726,13 +3726,13 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec /* }}} */ #define ZEND_VM_INTERRUPT_CHECK() do { \ - if (UNEXPECTED(EG(vm_interrupt))) { \ + if (UNEXPECTED(zend_atomic_bool_load(&EG(vm_interrupt)))) { \ ZEND_VM_INTERRUPT(); \ } \ } while (0) #define ZEND_VM_LOOP_INTERRUPT_CHECK() do { \ - if (UNEXPECTED(EG(vm_interrupt))) { \ + if (UNEXPECTED(zend_atomic_bool_load(&EG(vm_interrupt)))) { \ ZEND_VM_LOOP_INTERRUPT(); \ } \ } while (0) diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index 2a26e71847db..a57cf3897036 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -168,8 +168,8 @@ void init_executor(void) /* {{{ */ zend_objects_store_init(&EG(objects_store), 1024); EG(full_tables_cleanup) = 0; - EG(vm_interrupt) = 0; - EG(timed_out) = 0; + ZEND_ATOMIC_BOOL_INIT(&EG(vm_interrupt), false); + ZEND_ATOMIC_BOOL_INIT(&EG(timed_out), false); EG(exception) = NULL; EG(prev_exception) = NULL; @@ -960,9 +960,8 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_ /* This flag is regularly checked while running user functions, but not internal * So see whether interrupt flag was set while the function was running... */ - if (EG(vm_interrupt)) { - EG(vm_interrupt) = 0; - if (EG(timed_out)) { + if (zend_atomic_bool_exchange(&EG(vm_interrupt), false)) { + if (zend_atomic_bool_load(&EG(timed_out))) { zend_timeout(); } else if (zend_interrupt_function) { zend_interrupt_function(EG(current_execute_data)); @@ -1330,14 +1329,14 @@ ZEND_API ZEND_NORETURN void ZEND_FASTCALL zend_timeout(void) /* {{{ */ timer is not restarted properly, it could hang in the shutdown function. */ if (EG(hard_timeout) > 0) { - EG(timed_out) = 0; + zend_atomic_bool_store(&EG(timed_out), false); zend_set_timeout_ex(EG(hard_timeout), 1); /* XXX Abused, introduce an additional flag if the value needs to be kept. */ EG(hard_timeout) = 0; } # endif #else - EG(timed_out) = 0; + zend_atomic_bool_store(&EG(timed_out), false); zend_set_timeout_ex(0, 1); #endif @@ -1349,7 +1348,7 @@ ZEND_API ZEND_NORETURN void ZEND_FASTCALL zend_timeout(void) /* {{{ */ static void zend_timeout_handler(int dummy) /* {{{ */ { #ifndef ZTS - if (EG(timed_out)) { + if (zend_atomic_bool_load(&EG(timed_out))) { /* Die on hard timeout */ const char *error_filename = NULL; uint32_t error_lineno = 0; @@ -1384,8 +1383,8 @@ static void zend_timeout_handler(int dummy) /* {{{ */ zend_on_timeout(EG(timeout_seconds)); } - EG(timed_out) = 1; - EG(vm_interrupt) = 1; + zend_atomic_bool_store(&EG(timed_out), true); + zend_atomic_bool_store(&EG(vm_interrupt), true); #ifndef ZTS if (EG(hard_timeout) > 0) { @@ -1409,8 +1408,8 @@ VOID CALLBACK tq_timer_cb(PVOID arg, BOOLEAN timed_out) } eg = (zend_executor_globals *)arg; - eg->timed_out = 1; - eg->vm_interrupt = 1; + zend_atomic_bool_store(&eg->timed_out, true); + zend_atomic_bool_store(&eg->vm_interrupt, true); } #endif @@ -1496,7 +1495,7 @@ void zend_set_timeout(zend_long seconds, bool reset_signals) /* {{{ */ EG(timeout_seconds) = seconds; zend_set_timeout_ex(seconds, reset_signals); - EG(timed_out) = 0; + zend_atomic_bool_store(&EG(timed_out), false); } /* }}} */ @@ -1505,7 +1504,7 @@ void zend_unset_timeout(void) /* {{{ */ #ifdef ZEND_WIN32 if (NULL != tq_timer) { if (!DeleteTimerQueueTimer(NULL, tq_timer, INVALID_HANDLE_VALUE)) { - EG(timed_out) = 0; + zend_atomic_bool_store(&EG(timed_out), false); tq_timer = NULL; zend_error_noreturn(E_ERROR, "Could not delete queued timer"); return; @@ -1525,7 +1524,7 @@ void zend_unset_timeout(void) /* {{{ */ # endif } #endif - EG(timed_out) = 0; + zend_atomic_bool_store(&EG(timed_out), false); } /* }}} */ diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h index ccee57b3fb1b..17469fab0c11 100644 --- a/Zend/zend_globals.h +++ b/Zend/zend_globals.h @@ -25,6 +25,7 @@ #include "zend_globals_macros.h" +#include "zend_atomic.h" #include "zend_stack.h" #include "zend_ptr_stack.h" #include "zend_hash.h" @@ -189,8 +190,8 @@ struct _zend_executor_globals { /* for extended information support */ bool no_extensions; - bool vm_interrupt; - bool timed_out; + zend_atomic_bool vm_interrupt; + zend_atomic_bool timed_out; zend_long hard_timeout; #ifdef ZEND_WIN32 diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index bbba2ca0dfc9..e9b146824abe 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -9892,9 +9892,9 @@ ZEND_VM_DEFINE_OP(137, ZEND_OP_DATA); ZEND_VM_HELPER(zend_interrupt_helper, ANY, ANY) { - EG(vm_interrupt) = 0; + zend_atomic_bool_store(&EG(vm_interrupt), false); SAVE_OPLINE(); - if (EG(timed_out)) { + if (zend_atomic_bool_load(&EG(timed_out))) { zend_timeout(); } else if (zend_interrupt_function) { zend_interrupt_function(execute_data); diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 1b2778547006..839c3086f9e3 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -3537,9 +3537,9 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_JMP_FORWARD_SPEC_H static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_interrupt_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS) { - EG(vm_interrupt) = 0; + zend_atomic_bool_store(&EG(vm_interrupt), false); SAVE_OPLINE(); - if (EG(timed_out)) { + if (zend_atomic_bool_load(&EG(timed_out))) { zend_timeout(); } else if (zend_interrupt_function) { zend_interrupt_function(execute_data); diff --git a/configure.ac b/configure.ac index 29dd3b0437a4..985d5f210188 100644 --- a/configure.ac +++ b/configure.ac @@ -1639,7 +1639,7 @@ PHP_ADD_SOURCES(Zend, \ zend_closures.c zend_weakrefs.c zend_float.c zend_string.c zend_signal.c zend_generators.c \ zend_virtual_cwd.c zend_ast.c zend_objects.c zend_object_handlers.c zend_objects_API.c \ zend_default_classes.c zend_inheritance.c zend_smart_str.c zend_cpuinfo.c zend_gdb.c \ - zend_observer.c zend_system_id.c zend_enum.c zend_fibers.c \ + zend_observer.c zend_system_id.c zend_enum.c zend_fibers.c zend_atomic.c \ Optimizer/zend_optimizer.c \ Optimizer/pass1.c \ Optimizer/pass3.c \ diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index b45846896afc..559c1b64daa6 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -8052,7 +8052,7 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num, zend_jit_registers_buf EX(opline) = opline; } - if (EG(vm_interrupt) || JIT_G(tracing)) { + if (zend_atomic_bool_load(&EG(vm_interrupt)) || JIT_G(tracing)) { return 1; /* Lock-free check if the side trace was already JIT-ed or blacklist-ed in another process */ } else if (t->exit_info[exit_num].flags & (ZEND_JIT_EXIT_JITED|ZEND_JIT_EXIT_BLACKLISTED)) { diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 8931ba92af53..91760e997c30 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -1383,7 +1383,7 @@ static void pcntl_signal_handler(int signo) PCNTL_G(tail) = psig; PCNTL_G(pending_signals) = 1; if (PCNTL_G(async_signals)) { - EG(vm_interrupt) = 1; + zend_atomic_bool_store(&EG(vm_interrupt), true); } } diff --git a/sapi/phpdbg/phpdbg_prompt.c b/sapi/phpdbg/phpdbg_prompt.c index dfdac95cae58..6126ea8e0734 100644 --- a/sapi/phpdbg/phpdbg_prompt.c +++ b/sapi/phpdbg/phpdbg_prompt.c @@ -1663,7 +1663,7 @@ void phpdbg_execute_ex(zend_execute_data *execute_data) /* {{{ */ } #ifdef ZEND_WIN32 - if (EG(timed_out)) { + if (zend_atomic_bool_load(&EG(timed_out))) { zend_timeout(); } #endif diff --git a/win32/build/config.w32 b/win32/build/config.w32 index 9f281390c43a..233bc2c95734 100644 --- a/win32/build/config.w32 +++ b/win32/build/config.w32 @@ -238,7 +238,7 @@ ADD_SOURCES("Zend", "zend_language_parser.c zend_language_scanner.c \ zend_default_classes.c zend_execute.c zend_strtod.c zend_gc.c zend_closures.c zend_weakrefs.c \ zend_float.c zend_string.c zend_generators.c zend_virtual_cwd.c zend_ast.c \ zend_inheritance.c zend_smart_str.c zend_cpuinfo.c zend_observer.c zend_system_id.c \ - zend_enum.c zend_fibers.c"); + zend_enum.c zend_fibers.c zend_atomic.c"); ADD_SOURCES("Zend\\Optimizer", "zend_optimizer.c pass1.c pass3.c optimize_func_calls.c block_pass.c optimize_temp_vars_5.c nop_removal.c compact_literals.c zend_cfg.c zend_dfg.c dfa_pass.c zend_ssa.c zend_inference.c zend_func_info.c zend_call_graph.c zend_dump.c escape_analysis.c compact_vars.c dce.c sccp.c scdf.c"); var FIBER_ASSEMBLER = X64 ? PATH_PROG('ML64') : PATH_PROG('ML'); diff --git a/win32/signal.c b/win32/signal.c index 088a0044283f..da51d934b746 100644 --- a/win32/signal.c +++ b/win32/signal.c @@ -22,7 +22,7 @@ /* true globals; only used from main thread and from kernel callback */ static zval ctrl_handler; static DWORD ctrl_evt = (DWORD)-1; -static bool *vm_interrupt_flag = NULL; +static zend_atomic_bool *vm_interrupt_flag = NULL; static void (*orig_interrupt_function)(zend_execute_data *execute_data); @@ -77,7 +77,7 @@ static BOOL WINAPI php_win32_signal_system_ctrl_handler(DWORD evt) return FALSE; } - (void)InterlockedExchange8(vm_interrupt_flag, 1); + zend_atomic_bool_store(vm_interrupt_flag, true); ctrl_evt = evt; From e752c79aabcc4d639314ca5cbd4a47a2cddc82dd Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 20 May 2022 13:45:09 -0600 Subject: [PATCH 2/5] Avoid extern inline I think the code is worse for doing this, but it was requested during code review and I don't care to die on this hill. --- Zend/zend_atomic.c | 19 ++++++++----- Zend/zend_atomic.h | 46 +++++++++++++++++++++----------- Zend/zend_execute.c | 4 +-- Zend/zend_execute_API.c | 24 ++++++++--------- Zend/zend_vm_def.h | 4 +-- Zend/zend_vm_execute.h | 4 +-- ext/opcache/jit/zend_jit_trace.c | 2 +- ext/pcntl/pcntl.c | 2 +- sapi/phpdbg/phpdbg_prompt.c | 2 +- win32/signal.c | 2 +- 10 files changed, 65 insertions(+), 44 deletions(-) diff --git a/Zend/zend_atomic.c b/Zend/zend_atomic.c index 22d98fcb4134..b3d2d44b47cb 100644 --- a/Zend/zend_atomic.c +++ b/Zend/zend_atomic.c @@ -27,14 +27,21 @@ ZEND_API void zend_atomic_bool_init(zend_atomic_bool *obj, bool desired) { ZEND_ATOMIC_BOOL_INIT(obj, desired); } -extern inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, - bool desired); -extern inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, - bool desired); +ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { + return zend_atomic_bool_exchange_ex(obj, desired); +} + +ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { + return zend_atomic_bool_store_ex(obj, desired); +} #if ZEND_WIN32 || HAVE_SYNC_ATOMICS /* On these platforms it is non-const due to underlying APIs. */ -extern inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj); +ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj) { + return zend_atomic_bool_load_ex(obj); +} #else -extern inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj); +ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) { + return zend_atomic_bool_load_ex(obj); +} #endif diff --git a/Zend/zend_atomic.h b/Zend/zend_atomic.h index 37397b66295e..8a231e43901f 100644 --- a/Zend/zend_atomic.h +++ b/Zend/zend_atomic.h @@ -63,17 +63,17 @@ BEGIN_EXTERN_C() #define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) -inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { +static zend_always_inline bool zend_atomic_bool_exchange_ex(zend_atomic_bool *obj, bool desired) { return InterlockedExchange8(&obj->value, desired); } /* On this platform it is non-const due to Iterlocked API*/ -inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj) { +static zend_always_inline bool zend_atomic_bool_load_ex(zend_atomic_bool *obj) { /* Or'ing with false won't change the value. */ return InterlockedOr8(&obj->value, false); } -inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { +static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, bool desired) { (void)InterlockedExchange8(&obj->value, desired); } @@ -81,15 +81,15 @@ inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) #define ZEND_ATOMIC_BOOL_INIT(obj, desired) __c11_atomic_init(&(obj)->value, (desired)) -inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { +static zend_always_inline bool zend_atomic_bool_exchange_ex(zend_atomic_bool *obj, bool desired) { return __c11_atomic_exchange(&obj->value, desired, __ATOMIC_SEQ_CST); } -inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) { +static zend_always_inline bool zend_atomic_bool_load_ex(const zend_atomic_bool *obj) { return __c11_atomic_load(&obj->value, __ATOMIC_SEQ_CST); } -inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { +static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, bool desired) { __c11_atomic_store(&obj->value, desired, __ATOMIC_SEQ_CST); } @@ -97,19 +97,19 @@ inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) #define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) -inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { +static zend_always_inline bool zend_atomic_bool_exchange_ex(zend_atomic_bool *obj, bool desired) { bool prev = false; __atomic_exchange(&obj->value, &desired, &prev, __ATOMIC_SEQ_CST); return prev; } -inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) { +static zend_always_inline bool zend_atomic_bool_load_ex(const zend_atomic_bool *obj) { bool prev = false; __atomic_load(&obj->value, &prev, __ATOMIC_SEQ_CST); return prev; } -inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { +static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, bool desired) { __atomic_store(&obj->value, &desired, __ATOMIC_SEQ_CST); } @@ -117,7 +117,7 @@ inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) #define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) -inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { +static zend_always_inline bool zend_atomic_bool_exchange_ex(zend_atomic_bool *obj, bool desired) { bool prev = __sync_lock_test_and_set(&obj->value, desired); /* __sync_lock_test_and_set only does an acquire barrier, so sync @@ -127,12 +127,12 @@ inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desir return prev; } -inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj) { +static zend_always_inline bool zend_atomic_bool_load_ex(zend_atomic_bool *obj) { /* Or'ing false won't change the value */ return __sync_fetch_and_or(&obj->value, false); } -inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { +static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, bool desired) { __sync_synchronize(); obj->value = desired; __sync_synchronize(); @@ -147,18 +147,21 @@ inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) * improvement. As more platforms support C11 atomics, or as we add support * for more platforms through intrinsics/asm, this should be used less and * less until it can be removed. - * At the time of this writing, all platforms in CI avoided this fallback. + * At the time of this writing, all platforms in CI avoided this fallback, + * so we emit an error. If there ends up being some platform that needs it, + * we can remove the error or add support for whatever platform that is. */ +#error No atomics support detected. Please open an issue with platform deatils. -inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { +static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, bool desired) { obj->value = desired; } -inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) { +static zend_always_inline bool zend_atomic_bool_load_ex(const zend_atomic_bool *obj) { return obj->value; } -inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { +static zend_always_inline bool zend_atomic_bool_exchange_ex(zend_atomic_bool *obj, bool desired) { bool prev = obj->value; obj->value = true; return prev; @@ -166,6 +169,17 @@ inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desir #endif +ZEND_API void zend_atomic_bool_init(zend_atomic_bool *obj, bool desired); +ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired); +ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired); + +#if ZEND_WIN32 || HAVE_SYNC_ATOMICS +/* On these platforms it is non-const due to underlying APIs. */ +ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj); +#else +ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj); +#endif + END_EXTERN_C() #endif diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 9f3b15cafe2b..7b55fa5e8ac2 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -3726,13 +3726,13 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec /* }}} */ #define ZEND_VM_INTERRUPT_CHECK() do { \ - if (UNEXPECTED(zend_atomic_bool_load(&EG(vm_interrupt)))) { \ + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { \ ZEND_VM_INTERRUPT(); \ } \ } while (0) #define ZEND_VM_LOOP_INTERRUPT_CHECK() do { \ - if (UNEXPECTED(zend_atomic_bool_load(&EG(vm_interrupt)))) { \ + if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { \ ZEND_VM_LOOP_INTERRUPT(); \ } \ } while (0) diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index a57cf3897036..99a1de0e0457 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -960,8 +960,8 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_ /* This flag is regularly checked while running user functions, but not internal * So see whether interrupt flag was set while the function was running... */ - if (zend_atomic_bool_exchange(&EG(vm_interrupt), false)) { - if (zend_atomic_bool_load(&EG(timed_out))) { + if (zend_atomic_bool_exchange_ex(&EG(vm_interrupt), false)) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { zend_timeout(); } else if (zend_interrupt_function) { zend_interrupt_function(EG(current_execute_data)); @@ -1329,14 +1329,14 @@ ZEND_API ZEND_NORETURN void ZEND_FASTCALL zend_timeout(void) /* {{{ */ timer is not restarted properly, it could hang in the shutdown function. */ if (EG(hard_timeout) > 0) { - zend_atomic_bool_store(&EG(timed_out), false); + zend_atomic_bool_store_ex(&EG(timed_out), false); zend_set_timeout_ex(EG(hard_timeout), 1); /* XXX Abused, introduce an additional flag if the value needs to be kept. */ EG(hard_timeout) = 0; } # endif #else - zend_atomic_bool_store(&EG(timed_out), false); + zend_atomic_bool_store_ex(&EG(timed_out), false); zend_set_timeout_ex(0, 1); #endif @@ -1348,7 +1348,7 @@ ZEND_API ZEND_NORETURN void ZEND_FASTCALL zend_timeout(void) /* {{{ */ static void zend_timeout_handler(int dummy) /* {{{ */ { #ifndef ZTS - if (zend_atomic_bool_load(&EG(timed_out))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { /* Die on hard timeout */ const char *error_filename = NULL; uint32_t error_lineno = 0; @@ -1383,8 +1383,8 @@ static void zend_timeout_handler(int dummy) /* {{{ */ zend_on_timeout(EG(timeout_seconds)); } - zend_atomic_bool_store(&EG(timed_out), true); - zend_atomic_bool_store(&EG(vm_interrupt), true); + zend_atomic_bool_store_ex(&EG(timed_out), true); + zend_atomic_bool_store_ex(&EG(vm_interrupt), true); #ifndef ZTS if (EG(hard_timeout) > 0) { @@ -1408,8 +1408,8 @@ VOID CALLBACK tq_timer_cb(PVOID arg, BOOLEAN timed_out) } eg = (zend_executor_globals *)arg; - zend_atomic_bool_store(&eg->timed_out, true); - zend_atomic_bool_store(&eg->vm_interrupt, true); + zend_atomic_bool_store_ex(&eg->timed_out, true); + zend_atomic_bool_store_ex(&eg->vm_interrupt, true); } #endif @@ -1495,7 +1495,7 @@ void zend_set_timeout(zend_long seconds, bool reset_signals) /* {{{ */ EG(timeout_seconds) = seconds; zend_set_timeout_ex(seconds, reset_signals); - zend_atomic_bool_store(&EG(timed_out), false); + zend_atomic_bool_store_ex(&EG(timed_out), false); } /* }}} */ @@ -1504,7 +1504,7 @@ void zend_unset_timeout(void) /* {{{ */ #ifdef ZEND_WIN32 if (NULL != tq_timer) { if (!DeleteTimerQueueTimer(NULL, tq_timer, INVALID_HANDLE_VALUE)) { - zend_atomic_bool_store(&EG(timed_out), false); + zend_atomic_bool_store_ex(&EG(timed_out), false); tq_timer = NULL; zend_error_noreturn(E_ERROR, "Could not delete queued timer"); return; @@ -1524,7 +1524,7 @@ void zend_unset_timeout(void) /* {{{ */ # endif } #endif - zend_atomic_bool_store(&EG(timed_out), false); + zend_atomic_bool_store_ex(&EG(timed_out), false); } /* }}} */ diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index e9b146824abe..0dc7f4d5f4d9 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -9892,9 +9892,9 @@ ZEND_VM_DEFINE_OP(137, ZEND_OP_DATA); ZEND_VM_HELPER(zend_interrupt_helper, ANY, ANY) { - zend_atomic_bool_store(&EG(vm_interrupt), false); + zend_atomic_bool_store_ex(&EG(vm_interrupt), false); SAVE_OPLINE(); - if (zend_atomic_bool_load(&EG(timed_out))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { zend_timeout(); } else if (zend_interrupt_function) { zend_interrupt_function(execute_data); diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 839c3086f9e3..79796ee1a39c 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -3537,9 +3537,9 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_JMP_FORWARD_SPEC_H static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_interrupt_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS) { - zend_atomic_bool_store(&EG(vm_interrupt), false); + zend_atomic_bool_store_ex(&EG(vm_interrupt), false); SAVE_OPLINE(); - if (zend_atomic_bool_load(&EG(timed_out))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { zend_timeout(); } else if (zend_interrupt_function) { zend_interrupt_function(execute_data); diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index 559c1b64daa6..149d4298a7a0 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -8052,7 +8052,7 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num, zend_jit_registers_buf EX(opline) = opline; } - if (zend_atomic_bool_load(&EG(vm_interrupt)) || JIT_G(tracing)) { + if (zend_atomic_bool_load_ex(&EG(vm_interrupt)) || JIT_G(tracing)) { return 1; /* Lock-free check if the side trace was already JIT-ed or blacklist-ed in another process */ } else if (t->exit_info[exit_num].flags & (ZEND_JIT_EXIT_JITED|ZEND_JIT_EXIT_BLACKLISTED)) { diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 91760e997c30..468a89641164 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -1383,7 +1383,7 @@ static void pcntl_signal_handler(int signo) PCNTL_G(tail) = psig; PCNTL_G(pending_signals) = 1; if (PCNTL_G(async_signals)) { - zend_atomic_bool_store(&EG(vm_interrupt), true); + zend_atomic_bool_store_ex(&EG(vm_interrupt), true); } } diff --git a/sapi/phpdbg/phpdbg_prompt.c b/sapi/phpdbg/phpdbg_prompt.c index 6126ea8e0734..154eaf0ee749 100644 --- a/sapi/phpdbg/phpdbg_prompt.c +++ b/sapi/phpdbg/phpdbg_prompt.c @@ -1663,7 +1663,7 @@ void phpdbg_execute_ex(zend_execute_data *execute_data) /* {{{ */ } #ifdef ZEND_WIN32 - if (zend_atomic_bool_load(&EG(timed_out))) { + if (zend_atomic_bool_load_ex(&EG(timed_out))) { zend_timeout(); } #endif diff --git a/win32/signal.c b/win32/signal.c index da51d934b746..88f975ab644b 100644 --- a/win32/signal.c +++ b/win32/signal.c @@ -77,7 +77,7 @@ static BOOL WINAPI php_win32_signal_system_ctrl_handler(DWORD evt) return FALSE; } - zend_atomic_bool_store(vm_interrupt_flag, true); + zend_atomic_bool_store_ex(vm_interrupt_flag, true); ctrl_evt = evt; From 25405add1f78f5c9b0a7195abdf9e47f607ab842 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 20 May 2022 14:01:40 -0600 Subject: [PATCH 3/5] Fix 'void' function returning a value --- Zend/zend_atomic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/zend_atomic.c b/Zend/zend_atomic.c index b3d2d44b47cb..7b16f5e48513 100644 --- a/Zend/zend_atomic.c +++ b/Zend/zend_atomic.c @@ -32,7 +32,7 @@ ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) { } ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) { - return zend_atomic_bool_store_ex(obj, desired); + zend_atomic_bool_store_ex(obj, desired); } #if ZEND_WIN32 || HAVE_SYNC_ATOMICS From d79c09dd2dd014383693f6426e501bc174747615 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 25 May 2022 10:41:24 -0600 Subject: [PATCH 4/5] [ci skip] Clean up a bit twose pointed out that zend_portability.h already defines the __has_feature no-op. --- Zend/zend_atomic.h | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/Zend/zend_atomic.h b/Zend/zend_atomic.h index 8a231e43901f..29ad28b25262 100644 --- a/Zend/zend_atomic.h +++ b/Zend/zend_atomic.h @@ -19,10 +19,6 @@ #include -#ifndef __has_feature -#define __has_feature(x) 0 -#endif - #define ZEND_GCC_PREREQ(x, y) \ ((__GNUC__ == (x) && __GNUC_MINOR__ >= (y)) || (__GNUC__ > (x))) @@ -140,18 +136,13 @@ static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, #elif HAVE_NO_ATOMICS -#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) +#error No atomics support detected. Please open an issue with platform details. -/* Yes, these are not guaranteed to be atomic. Understand that previously - * atomics were never used, so the fact they are sometimes used is an - * improvement. As more platforms support C11 atomics, or as we add support - * for more platforms through intrinsics/asm, this should be used less and - * less until it can be removed. - * At the time of this writing, all platforms in CI avoided this fallback, - * so we emit an error. If there ends up being some platform that needs it, - * we can remove the error or add support for whatever platform that is. +/* If there is a platform without atomics support, either: + * 1. Add support for it. + * 2. Remove the #error above and use the below non-atomic versions. */ -#error No atomics support detected. Please open an issue with platform deatils. +#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, bool desired) { obj->value = desired; From 2a5d58a96d3ae20b811a7986e5355ee8aaf6b1d2 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 1 Jun 2022 09:39:52 -0600 Subject: [PATCH 5/5] [ci skip] Convert `#error` to `#warning` --- Zend/zend_atomic.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Zend/zend_atomic.h b/Zend/zend_atomic.h index 29ad28b25262..5f7f6fbc6e65 100644 --- a/Zend/zend_atomic.h +++ b/Zend/zend_atomic.h @@ -136,12 +136,8 @@ static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, #elif HAVE_NO_ATOMICS -#error No atomics support detected. Please open an issue with platform details. +#warning No atomics support detected. Please open an issue with platform details. -/* If there is a platform without atomics support, either: - * 1. Add support for it. - * 2. Remove the #error above and use the below non-atomic versions. - */ #define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired)) static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, bool desired) {