Skip to content

Commit ba58be9

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

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)