Skip to content

Commit 35c5a7c

Browse files
morrisonlevimorrisonlevi
committed
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 use platform specific functions, preferring compiler intrinsics.
1 parent 862ef7a commit 35c5a7c

File tree

13 files changed

+241
-30
lines changed

13 files changed

+241
-30
lines changed

‎Zend/zend_atomic.c

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
+----------------------------------------------------------------------+
3+
| This source file is subject to version 3.01 of the PHP license, |
4+
| that is bundled with this package in the file LICENSE, and is |
5+
| available through the world-wide-web at the following url: |
6+
| https://www.php.net/license/3_01.txt |
7+
| If you did not receive a copy of the PHP license and are unable to |
8+
| obtain it through the world-wide-web, please send a note to |
9+
| license@php.net so we can mail you a copy immediately. |
10+
+----------------------------------------------------------------------+
11+
| Authors: Levi Morrison <morrison.levi@gmail.com> |
12+
+----------------------------------------------------------------------+
13+
*/
14+
15+
#include "zend_atomic.h"
16+
17+
/* This file contains the non-inline copy of atomic functions. This is useful
18+
* for extensions written in languages such as Rust. C and C++ compilers are
19+
* probably going to inline these functions, but in the case they don't, this
20+
* is also where the code will go.
21+
*/
22+
23+
/* Defined for FFI users; everyone else use ZEND_ATOMIC_BOOL_INIT.
24+
* This is NOT ATOMIC as it is meant for initialization.
25+
*/
26+
ZEND_API void zend_atomic_bool_init(zend_atomic_bool *obj, bool desired) {
27+
ZEND_ATOMIC_BOOL_INIT(obj, desired);
28+
}
29+
30+
extern inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj,
31+
bool desired);
32+
extern inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj,
33+
bool desired);
34+
35+
#if ZEND_WIN32 || HAVE_SYNC_ATOMICS
36+
/* On these platforms it is non-const due to underlying APIs. */
37+
extern inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj);
38+
#else
39+
extern inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj);
40+
#endif

‎Zend/zend_atomic.h

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/*
2+
+----------------------------------------------------------------------+
3+
| This source file is subject to version 3.01 of the PHP license, |
4+
| that is bundled with this package in the file LICENSE, and is |
5+
| available through the world-wide-web at the following url: |
6+
| https://www.php.net/license/3_01.txt |
7+
| If you did not receive a copy of the PHP license and are unable to |
8+
| obtain it through the world-wide-web, please send a note to |
9+
| license@php.net so we can mail you a copy immediately. |
10+
+----------------------------------------------------------------------+
11+
| Authors: Levi Morrison <morrison.levi@gmail.com> |
12+
+----------------------------------------------------------------------+
13+
*/
14+
15+
#ifndef ZEND_ATOMIC_H
16+
#define ZEND_ATOMIC_H
17+
18+
#include "zend_portability.h"
19+
20+
#include <stdbool.h>
21+
22+
#ifndef __has_feature
23+
#define __has_feature(x) 0
24+
#endif
25+
26+
#define ZEND_GCC_PREREQ(x, y) \
27+
((__GNUC__ == (x) && __GNUC_MINOR__ >= (y)) || (__GNUC__ > (x)))
28+
29+
/* Builtins are used to avoid library linkage */
30+
#if __has_feature(c_atomic)
31+
#define HAVE_C11_ATOMICS 1
32+
#elif ZEND_GCC_PREREQ(4, 7)
33+
#define HAVE_GNUC_ATOMICS 1
34+
#elif defined(__GNUC__)
35+
#define HAVE_SYNC_ATOMICS 1
36+
#elif !defined(ZEND_WIN32)
37+
#define HAVE_NO_ATOMICS 1
38+
#endif
39+
40+
#undef ZEND_GCC_PREREQ
41+
42+
/* Treat zend_atomic_* types as opaque. They have definitions only for size
43+
* and alignment purposes.
44+
*/
45+
46+
#if ZEND_WIN32 || HAVE_SYNC_ATOMICS
47+
typedef struct zend_atomic_bool_s {
48+
volatile char value;
49+
} zend_atomic_bool;
50+
#elif HAVE_C11_ATOMICS
51+
typedef struct zend_atomic_bool_s {
52+
_Atomic(bool) value;
53+
} zend_atomic_bool;
54+
#else
55+
typedef struct zend_atomic_bool_s {
56+
volatile bool value;
57+
} zend_atomic_bool;
58+
#endif
59+
60+
BEGIN_EXTERN_C()
61+
62+
#if ZEND_WIN32
63+
64+
#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired))
65+
66+
inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) {
67+
return InterlockedExchange8(&obj->value, desired);
68+
}
69+
70+
/* On this platform it is non-const due to Iterlocked API*/
71+
inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj) {
72+
/* Or'ing with false won't change the value. */
73+
return InterlockedOr8(&obj->value, false);
74+
}
75+
76+
inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) {
77+
(void)InterlockedExchange8(&obj->value, desired);
78+
}
79+
80+
#elif HAVE_C11_ATOMICS
81+
82+
#define ZEND_ATOMIC_BOOL_INIT(obj, desired) __c11_atomic_init(&(obj)->value, (desired))
83+
84+
inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) {
85+
return __c11_atomic_exchange(&obj->value, desired, __ATOMIC_SEQ_CST);
86+
}
87+
88+
inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) {
89+
return __c11_atomic_load(&obj->value, __ATOMIC_SEQ_CST);
90+
}
91+
92+
inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) {
93+
__c11_atomic_store(&obj->value, desired, __ATOMIC_SEQ_CST);
94+
}
95+
96+
#elif HAVE_GNUC_ATOMICS
97+
98+
#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired))
99+
100+
inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) {
101+
bool prev = false;
102+
__atomic_exchange(&obj->value, &desired, &prev, __ATOMIC_SEQ_CST);
103+
return prev;
104+
}
105+
106+
inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) {
107+
bool prev = false;
108+
__atomic_load(&obj->value, &prev, __ATOMIC_SEQ_CST);
109+
return prev;
110+
}
111+
112+
inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) {
113+
__atomic_store(&obj->value, &desired, __ATOMIC_SEQ_CST);
114+
}
115+
116+
#elif HAVE_SYNC_ATOMICS
117+
118+
#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired))
119+
120+
inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) {
121+
bool prev = __sync_lock_test_and_set(&obj->value, desired);
122+
123+
/* __sync_lock_test_and_set only does an acquire barrier, so sync
124+
* immediately after.
125+
*/
126+
__sync_synchronize();
127+
return prev;
128+
}
129+
130+
inline ZEND_API bool zend_atomic_bool_load(zend_atomic_bool *obj) {
131+
/* Or'ing false won't change the value */
132+
return __sync_fetch_and_or(&obj->value, false);
133+
}
134+
135+
inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) {
136+
__sync_synchronize();
137+
obj->value = desired;
138+
__sync_synchronize();
139+
}
140+
141+
#elif HAVE_NO_ATOMICS
142+
143+
#define ZEND_ATOMIC_BOOL_INIT(obj, desired) ((obj)->value = (desired))
144+
145+
/* Yes, these are not guaranteed to be atomic. Understand that previously
146+
* atomics were never used, so the fact they are sometimes used is an
147+
* improvement. As more platforms support C11 atomics, or as we add support
148+
* for more platforms through intrinsics/asm, this should be used less and
149+
* less until it can be removed.
150+
* At the time of this writing, all platforms in CI avoided this fallback.
151+
*/
152+
153+
inline ZEND_API void zend_atomic_bool_store(zend_atomic_bool *obj, bool desired) {
154+
obj->value = desired;
155+
}
156+
157+
inline ZEND_API bool zend_atomic_bool_load(const zend_atomic_bool *obj) {
158+
return obj->value;
159+
}
160+
161+
inline ZEND_API bool zend_atomic_bool_exchange(zend_atomic_bool *obj, bool desired) {
162+
bool prev = obj->value;
163+
obj->value = true;
164+
return prev;
165+
}
166+
167+
#endif
168+
169+
END_EXTERN_C()
170+
171+
#endif

‎Zend/zend_execute.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3726,13 +3726,13 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec
37263726
/* }}} */
37273727

37283728
#define ZEND_VM_INTERRUPT_CHECK() do { \
3729-
if (UNEXPECTED(EG(vm_interrupt))) { \
3729+
if (UNEXPECTED(zend_atomic_bool_load(&EG(vm_interrupt)))) { \
37303730
ZEND_VM_INTERRUPT(); \
37313731
} \
37323732
} while (0)
37333733

37343734
#define ZEND_VM_LOOP_INTERRUPT_CHECK() do { \
3735-
if (UNEXPECTED(EG(vm_interrupt))) { \
3735+
if (UNEXPECTED(zend_atomic_bool_load(&EG(vm_interrupt)))) { \
37363736
ZEND_VM_LOOP_INTERRUPT(); \
37373737
} \
37383738
} while (0)

‎Zend/zend_execute_API.c

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,8 @@ void init_executor(void) /* {{{ */
168168
zend_objects_store_init(&EG(objects_store), 1024);
169169

170170
EG(full_tables_cleanup) = 0;
171-
EG(vm_interrupt) = 0;
172-
EG(timed_out) = 0;
171+
ZEND_ATOMIC_BOOL_INIT(&EG(vm_interrupt), false);
172+
ZEND_ATOMIC_BOOL_INIT(&EG(timed_out), false);
173173

174174
EG(exception) = NULL;
175175
EG(prev_exception) = NULL;
@@ -960,9 +960,8 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_
960960

961961
/* This flag is regularly checked while running user functions, but not internal
962962
* So see whether interrupt flag was set while the function was running... */
963-
if (EG(vm_interrupt)) {
964-
EG(vm_interrupt) = 0;
965-
if (EG(timed_out)) {
963+
if (zend_atomic_bool_exchange(&EG(vm_interrupt), false)) {
964+
if (zend_atomic_bool_load(&EG(timed_out))) {
966965
zend_timeout();
967966
} else if (zend_interrupt_function) {
968967
zend_interrupt_function(EG(current_execute_data));
@@ -1330,14 +1329,14 @@ ZEND_API ZEND_NORETURN void ZEND_FASTCALL zend_timeout(void) /* {{{ */
13301329
timer is not restarted properly, it could hang in the shutdown
13311330
function. */
13321331
if (EG(hard_timeout) > 0) {
1333-
EG(timed_out) = 0;
1332+
zend_atomic_bool_store(&EG(timed_out), false);
13341333
zend_set_timeout_ex(EG(hard_timeout), 1);
13351334
/* XXX Abused, introduce an additional flag if the value needs to be kept. */
13361335
EG(hard_timeout) = 0;
13371336
}
13381337
# endif
13391338
#else
1340-
EG(timed_out) = 0;
1339+
zend_atomic_bool_store(&EG(timed_out), false);
13411340
zend_set_timeout_ex(0, 1);
13421341
#endif
13431342

@@ -1349,7 +1348,7 @@ ZEND_API ZEND_NORETURN void ZEND_FASTCALL zend_timeout(void) /* {{{ */
13491348
static void zend_timeout_handler(int dummy) /* {{{ */
13501349
{
13511350
#ifndef ZTS
1352-
if (EG(timed_out)) {
1351+
if (zend_atomic_bool_load(&EG(timed_out))) {
13531352
/* Die on hard timeout */
13541353
const char *error_filename = NULL;
13551354
uint32_t error_lineno = 0;
@@ -1384,8 +1383,8 @@ static void zend_timeout_handler(int dummy) /* {{{ */
13841383
zend_on_timeout(EG(timeout_seconds));
13851384
}
13861385

1387-
EG(timed_out) = 1;
1388-
EG(vm_interrupt) = 1;
1386+
zend_atomic_bool_store(&EG(timed_out), true);
1387+
zend_atomic_bool_store(&EG(vm_interrupt), true);
13891388

13901389
#ifndef ZTS
13911390
if (EG(hard_timeout) > 0) {
@@ -1409,8 +1408,8 @@ VOID CALLBACK tq_timer_cb(PVOID arg, BOOLEAN timed_out)
14091408
}
14101409

14111410
eg = (zend_executor_globals *)arg;
1412-
eg->timed_out = 1;
1413-
eg->vm_interrupt = 1;
1411+
zend_atomic_bool_store(&eg->timed_out, true);
1412+
zend_atomic_bool_store(&eg->vm_interrupt, true);
14141413
}
14151414
#endif
14161415

@@ -1496,7 +1495,7 @@ void zend_set_timeout(zend_long seconds, bool reset_signals) /* {{{ */
14961495

14971496
EG(timeout_seconds) = seconds;
14981497
zend_set_timeout_ex(seconds, reset_signals);
1499-
EG(timed_out) = 0;
1498+
zend_atomic_bool_store(&EG(timed_out), false);
15001499
}
15011500
/* }}} */
15021501

@@ -1505,7 +1504,7 @@ void zend_unset_timeout(void) /* {{{ */
15051504
#ifdef ZEND_WIN32
15061505
if (NULL != tq_timer) {
15071506
if (!DeleteTimerQueueTimer(NULL, tq_timer, INVALID_HANDLE_VALUE)) {
1508-
EG(timed_out) = 0;
1507+
zend_atomic_bool_store(&EG(timed_out), false);
15091508
tq_timer = NULL;
15101509
zend_error_noreturn(E_ERROR, "Could not delete queued timer");
15111510
return;
@@ -1525,7 +1524,7 @@ void zend_unset_timeout(void) /* {{{ */
15251524
# endif
15261525
}
15271526
#endif
1528-
EG(timed_out) = 0;
1527+
zend_atomic_bool_store(&EG(timed_out), false);
15291528
}
15301529
/* }}} */
15311530

‎Zend/zend_globals.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include "zend_globals_macros.h"
2727

28+
#include "zend_atomic.h"
2829
#include "zend_stack.h"
2930
#include "zend_ptr_stack.h"
3031
#include "zend_hash.h"
@@ -189,8 +190,8 @@ struct _zend_executor_globals {
189190
/* for extended information support */
190191
bool no_extensions;
191192

192-
bool vm_interrupt;
193-
bool timed_out;
193+
zend_atomic_bool vm_interrupt;
194+
zend_atomic_bool timed_out;
194195
zend_long hard_timeout;
195196

196197
#ifdef ZEND_WIN32

‎Zend/zend_vm_def.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9892,9 +9892,9 @@ ZEND_VM_DEFINE_OP(137, ZEND_OP_DATA);
98929892

98939893
ZEND_VM_HELPER(zend_interrupt_helper, ANY, ANY)
98949894
{
9895-
EG(vm_interrupt) = 0;
9895+
zend_atomic_bool_store(&EG(vm_interrupt), false);
98969896
SAVE_OPLINE();
9897-
if (EG(timed_out)) {
9897+
if (zend_atomic_bool_load(&EG(timed_out))) {
98989898
zend_timeout();
98999899
} else if (zend_interrupt_function) {
99009900
zend_interrupt_function(execute_data);

‎Zend/zend_vm_execute.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3537,9 +3537,9 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_JMP_FORWARD_SPEC_H
35373537

35383538
static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_interrupt_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS)
35393539
{
3540-
EG(vm_interrupt) = 0;
3540+
zend_atomic_bool_store(&EG(vm_interrupt), false);
35413541
SAVE_OPLINE();
3542-
if (EG(timed_out)) {
3542+
if (zend_atomic_bool_load(&EG(timed_out))) {
35433543
zend_timeout();
35443544
} else if (zend_interrupt_function) {
35453545
zend_interrupt_function(execute_data);

‎configure.ac

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1639,7 +1639,7 @@ PHP_ADD_SOURCES(Zend, \
16391639
zend_closures.c zend_weakrefs.c zend_float.c zend_string.c zend_signal.c zend_generators.c \
16401640
zend_virtual_cwd.c zend_ast.c zend_objects.c zend_object_handlers.c zend_objects_API.c \
16411641
zend_default_classes.c zend_inheritance.c zend_smart_str.c zend_cpuinfo.c zend_gdb.c \
1642-
zend_observer.c zend_system_id.c zend_enum.c zend_fibers.c \
1642+
zend_observer.c zend_system_id.c zend_enum.c zend_fibers.c zend_atomic.c \
16431643
Optimizer/zend_optimizer.c \
16441644
Optimizer/pass1.c \
16451645
Optimizer/pass3.c \

‎ext/opcache/jit/zend_jit_trace.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8052,7 +8052,7 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num, zend_jit_registers_buf
80528052
EX(opline) = opline;
80538053
}
80548054

8055-
if (EG(vm_interrupt) || JIT_G(tracing)) {
8055+
if (zend_atomic_bool_load(&EG(vm_interrupt)) || JIT_G(tracing)) {
80568056
return 1;
80578057
/* Lock-free check if the side trace was already JIT-ed or blacklist-ed in another process */
80588058
} else if (t->exit_info[exit_num].flags & (ZEND_JIT_EXIT_JITED|ZEND_JIT_EXIT_BLACKLISTED)) {

‎ext/pcntl/pcntl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1383,7 +1383,7 @@ static void pcntl_signal_handler(int signo)
13831383
PCNTL_G(tail) = psig;
13841384
PCNTL_G(pending_signals) = 1;
13851385
if (PCNTL_G(async_signals)) {
1386-
EG(vm_interrupt) = 1;
1386+
zend_atomic_bool_store(&EG(vm_interrupt), true);
13871387
}
13881388
}
13891389

0 commit comments

Comments
 (0)