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<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 check that atomic_bool has the same size and alignment as
native bool and if so we'll cast it.

Author: morrisonlevi

Zend/Zend.m4

@@ -389,3 +389,32 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([[
 if test "$ac_cv_cpuid_count_available" = "yes"; then
   AC_DEFINE([HAVE_CPUID_COUNT], 1, [whether __cpuid_count is available])
 fi
+
+dnl Check whether <stdatomic.h> is available.
+AC_CACHE_CHECK(
+  [whether <stdatomic.h> is available],
+  ac_cv_stdatomic_h,
+  [AC_RUN_IFELSE(
+    [AC_LANG_SOURCE(
+      [[
+        #include <stdatomic.h>
+        #include <stdbool.h>
+        int main(void) {
+          atomic_bool val = false;
+          (void)atomic_load(&val);
+          atomic_store(&val, true);
+          (void)atomic_exchange(&val, false);
+
+          if (sizeof(atomic_bool) != sizeof(bool)) return 1;
+          if (_Alignof(atomic_bool) != _Alignof(bool)) return 1;
+          return 0;
+        }
+      ]]
+    )],
+    [ac_cv_stdatomic_h_available=yes],
+    [ac_cv_stdatomic_h_available=no]
+  )]
+)
+if test "$ac_cv_stdatomic_h_available" = "yes"; then
+  AC_DEFINE([HAVE_STDATOMIC_H], 1, [whether <stdatomic.h> is available])
+fi

Zend/zend_atomic.c

@@ -0,0 +1,67 @@
+// TODO: license
+
+#include "zend_atomic.h"
+
+/* The general strategy here is to use C11's stdatomic.h functions when they
+ * are available, and then fall-back to non-atomic operations, except for
+ * Windows, which has C++ std::atomic_bool but not C atomic_bool.
+ *
+ * The defines HAVE_ATOMIC_BOOL should only be set when it is determined that
+ * representations of atomic_bool and bool are the same, as this is relied on
+ * in the implementation.
+ */
+
+#if HAVE_STDATOMIC_H
+#include <stdatomic.h>
+#define HAVE_ATOMIC_BOOL 1
+#elif WIN32
+#include <atomic>
+using std::atomic_bool;
+using std::atomic_exchange;
+using std::atomic_load;
+using std::atomic_store;
+static_assert(sizeof(atomic_bool) == sizeof(bool), "Repr of atomic_bool and bool must match");
+static_assert(alignof(atomic_bool) == alignof(bool), "Repr of atomic_bool and bool must match");
+#define HAVE_ATOMIC_BOOL 1
+#else
+#define HAVE_ATOMIC_BOOL 0
+#endif
+
+#if HAVE_ATOMIC_BOOL
+
+ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) {
+	return atomic_exchange((atomic_bool *)obj, desired);
+}
+
+ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) {
+	return atomic_load((const atomic_bool *)obj);
+}
+
+ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) {
+	atomic_store((atomic_bool *)obj, desired);
+}
+
+#else
+
+/* 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.
+ */
+
+ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) {
+	bool previous = obj->bytes;
+	obj->bytes = desired;
+	return previous;
+}
+
+ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) {
+	return obj->bytes;
+}
+
+ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) {
+	obj->bytes = desired;
+}
+
+#endif

Zend/zend_atomic.h

@@ -0,0 +1,18 @@
+// TODO: license
+#include "zend_portability.h"
+
+/* These functions correspond to C11's stdatomic.h functions but are type-
+ * specialized because it's difficult to provide portable routines such as
+ * exchange without making functions.
+ *
+ * Treat zend_atomic_* types as opaque. They have definitions only for size
+ * and alignment purposes.
+ */
+
+typedef struct zend_atomic_bool_s {
+	volatile bool bytes;
+} zend_atomic_bool;
+
+ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj);
+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);

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)

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_store(&EG(vm_interrupt), false);
+	zend_atomic_bool_store(&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));
@@ -1337,7 +1336,7 @@ ZEND_API ZEND_NORETURN void ZEND_FASTCALL zend_timeout(void) /* {{{ */
 	}
 # 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);
 }
 /* }}} */
 
@@ -1525,7 +1524,7 @@ void zend_unset_timeout(void) /* {{{ */
 # endif
 	}
 #endif
-	EG(timed_out) = 0;
+	zend_atomic_bool_store(&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;
-	bool timed_out;
+	zend_atomic_bool vm_interrupt;
+	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(&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);

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

build/php.m4

@@ -2152,7 +2152,7 @@ EOF
    else
     break
    fi
-   $as_echo "$CURRENT_ARG \\" >>$1
+   AS_ECHO(["$CURRENT_ARG \\"]) >>$1
    CONFIGURE_OPTIONS="$CONFIGURE_OPTIONS $CURRENT_ARG"
   done
   echo '"[$]@"' >> $1

configure.ac

@@ -1637,7 +1637,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

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

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

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.h");
 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 volatile bool *vm_interrupt_flag = NULL;
 
 static void (*orig_interrupt_function)(zend_execute_data *execute_data);