Skip to content

Make vm_interrupt and timed_out atomic #7937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions Zend/Zend.m4
Original file line number Diff line number Diff line change
Expand Up @@ -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
65 changes: 65 additions & 0 deletions Zend/zend_atomic.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
+----------------------------------------------------------------------+
| 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"

/* 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>

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
37 changes: 37 additions & 0 deletions Zend/zend_atomic.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
+----------------------------------------------------------------------+
| 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"

#if !defined(ZEND_WIN32)
#error This implementation is meant for Windows only. Please refer to zend_atomic.c.
#endif

#include <atomic>
using std::atomic_bool;

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

ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) {
return std::atomic_exchange((atomic_bool *)obj, desired);
}

ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) {
return std::atomic_load((const atomic_bool *)obj);
}

ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) {
std::atomic_store((atomic_bool *)obj, desired);
}
42 changes: 42 additions & 0 deletions Zend/zend_atomic.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
+----------------------------------------------------------------------+
| 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>

/* 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;

BEGIN_EXTERN_C()

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

END_EXTERN_C()

#endif
4 changes: 2 additions & 2 deletions Zend/zend_execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
29 changes: 14 additions & 15 deletions Zend/zend_execute_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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), 0);
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

Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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

Expand Down Expand Up @@ -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);
}
/* }}} */

Expand All @@ -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), 0);
tq_timer = NULL;
zend_error_noreturn(E_ERROR, "Could not delete queued timer");
return;
Expand All @@ -1525,7 +1524,7 @@ void zend_unset_timeout(void) /* {{{ */
# endif
}
#endif
EG(timed_out) = 0;
zend_atomic_bool_store(&EG(timed_out), false);
}
/* }}} */

Expand Down
5 changes: 3 additions & 2 deletions Zend/zend_globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Zend/zend_vm_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions Zend/zend_vm_execute.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
2 changes: 1 addition & 1 deletion ext/opcache/jit/zend_jit_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion ext/pcntl/pcntl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion sapi/phpdbg/phpdbg_prompt.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion win32/build/config.w32
Original file line number Diff line number Diff line change
Expand Up @@ -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.cpp");
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');
Expand Down
Loading