Make vm_interrupt and timed_out atomic (#8327)

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<bool>, 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

@@ -0,0 +1,47 @@
+/*
+   +----------------------------------------------------------------------+
+   | 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 <morrison.levi@gmail.com>                     |
+   +----------------------------------------------------------------------+
+ */
+
+#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);
+}
+
+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) {
+	zend_atomic_bool_store_ex(obj, 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) {
+	return zend_atomic_bool_load_ex(obj);
+}
+#else
+ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) {
+	return zend_atomic_bool_load_ex(obj);
+}
+#endif

Zend/zend_atomic.h

@@ -0,0 +1,172 @@
+/*
+   +----------------------------------------------------------------------+
+   | 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 <morrison.levi@gmail.com>                     |
+   +----------------------------------------------------------------------+
+ */
+
+#ifndef ZEND_ATOMIC_H
+#define ZEND_ATOMIC_H
+
+#include "zend_portability.h"
+
+#include <stdbool.h>
+
+#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))
+
+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*/
+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);
+}
+
+static zend_always_inline void zend_atomic_bool_store_ex(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))
+
+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);
+}
+
+static zend_always_inline bool zend_atomic_bool_load_ex(const zend_atomic_bool *obj) {
+	return __c11_atomic_load(&obj->value, __ATOMIC_SEQ_CST);
+}
+
+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);
+}
+
+#elif HAVE_GNUC_ATOMICS
+
+#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (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;
+}
+
+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;
+}
+
+static zend_always_inline void zend_atomic_bool_store_ex(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))
+
+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
+	 * immediately after.
+	 */
+	__sync_synchronize();
+	return prev;
+}
+
+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);
+}
+
+static zend_always_inline void zend_atomic_bool_store_ex(zend_atomic_bool *obj, bool desired) {
+	__sync_synchronize();
+	obj->value = desired;
+	__sync_synchronize();
+}
+
+#elif HAVE_NO_ATOMICS
+
+#warning No atomics support detected. Please open an issue with platform details.
+
+#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;
+}
+
+static zend_always_inline bool zend_atomic_bool_load_ex(const zend_atomic_bool *obj) {
+	return obj->value;
+}
+
+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;
+}
+
+#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

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_ex(&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_ex(&EG(vm_interrupt)))) { \
 			ZEND_VM_LOOP_INTERRUPT(); \
 		} \
 	} while (0)

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;
+	ZEND_ATOMIC_BOOL_INIT(&EG(vm_interrupt), false);
-	EG(timed_out) = 0;
+	ZEND_ATOMIC_BOOL_INIT(&EG(timed_out), false);
 
 	EG(exception) = NULL;
 	EG(prev_exception) = NULL;
@@ -960,9 +960,8 @@ cleanup_args:
 
 		/* 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)) {
+		if (zend_atomic_bool_exchange_ex(&EG(vm_interrupt), false)) {
-			EG(vm_interrupt) = 0;
+			if (zend_atomic_bool_load_ex(&EG(timed_out))) {
-			if (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_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
-	EG(timed_out) = 0;
+	zend_atomic_bool_store_ex(&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_ex(&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;
+	zend_atomic_bool_store_ex(&EG(timed_out), true);
-	EG(vm_interrupt) = 1;
+	zend_atomic_bool_store_ex(&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;
+	zend_atomic_bool_store_ex(&eg->timed_out, true);
-	eg->vm_interrupt = 1;
+	zend_atomic_bool_store_ex(&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_ex(&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_ex(&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_ex(&EG(timed_out), false);
 }
 /* }}} */
 

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;
+	zend_atomic_bool vm_interrupt;
-	bool timed_out;
+	zend_atomic_bool timed_out;
 	zend_long hard_timeout;
 
 #ifdef ZEND_WIN32

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_ex(&EG(vm_interrupt), false);
 	SAVE_OPLINE();
-	if (EG(timed_out)) {
+	if (zend_atomic_bool_load_ex(&EG(timed_out))) {
 		zend_timeout();
 	} else if (zend_interrupt_function) {
 		zend_interrupt_function(execute_data);

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_ex(&EG(vm_interrupt), false);
 	SAVE_OPLINE();
-	if (EG(timed_out)) {
+	if (zend_atomic_bool_load_ex(&EG(timed_out))) {
 		zend_timeout();
 	} else if (zend_interrupt_function) {
 		zend_interrupt_function(execute_data);

configure.ac

@@ -1640,7 +1640,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 \

ext/opcache/jit/zend_jit_trace.c

@@ -8032,7 +8032,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_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)) {

ext/pcntl/pcntl.c

@@ -1385,7 +1385,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_ex(&EG(vm_interrupt), true);
 	}
 }
 

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_ex(&EG(timed_out))) {
 			zend_timeout();
 		}
 #endif

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');

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_ex(vm_interrupt_flag, true);
 
 	ctrl_evt = evt;