Skip to content

Commit 06bfe15

Browse files
morrisonleviarnaud-lb
morrisonlevi
and
arnaud-lb
committed
Fix tailcall helper dispatch after VM interrupt
In the tailcall VM, helpers which have extra args return the next opline instead of tailcalling it, relying on the parent to call it in ZEND_VM_DISPATCH_TO_HELPER(). When an interrupt is handled in the helper the opline may be tagged with ZEND_VM_ENTER_BIT, but ZEND_VM_DISPATCH_TO_HELPER() assumes an untagged opline. This PR changes ZEND_VM_INTERRUPT() so that such helpers return an opline whose handler is zend_interrupt_helper, instead of executing it directly. Closes GH-21922 Co-Authored-By: Arnaud Le Blanc <arnaud.lb@gmail.com>
1 parent 2c32bdf commit 06bfe15

13 files changed

Lines changed: 211 additions & 6 deletions

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ PHP NEWS
1515
. Fixed bug GH-21746 (Segfault with tracing JIT). (Arnaud)
1616
. Fixed bug GH-22004 (Assertion failure at ext/opcache/jit/zend_jit_trace.c).
1717
(Arnaud)
18+
. Fixed tailcall VM crash when a VM interrupt is handled from a VM helper.
19+
(Levi Morrison, Arnaud)
1820

1921
- OpenSSL:
2022
. Fix compatibility issues with OpenSSL 4.0. (jordikroon, Remi)

‎Zend/zend_vm.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,18 @@ ZEND_API void ZEND_FASTCALL zend_serialize_opcode_handler(zend_op *op);
3232
ZEND_API void ZEND_FASTCALL zend_deserialize_opcode_handler(zend_op *op);
3333
ZEND_API const void* ZEND_FASTCALL zend_get_opcode_handler_func(const zend_op *op);
3434
ZEND_API const zend_op *zend_get_halt_op(void);
35+
ZEND_API const zend_op *zend_get_interrupt_op(void);
3536
ZEND_API int ZEND_FASTCALL zend_vm_call_opcode_handler(zend_execute_data *ex);
3637
ZEND_API int zend_vm_kind(void);
3738
ZEND_API bool zend_gcc_global_regs(void);
3839

3940
void zend_vm_init(void);
4041
void zend_vm_dtor(void);
4142

43+
#if ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL
44+
const struct _zend_op *zend_vm_handle_interrupt(struct _zend_execute_data *execute_data, const struct _zend_op *opline);
45+
#endif
46+
4247
END_EXTERN_C()
4348

4449
#define ZEND_VM_SET_OPCODE_HANDLER(opline) zend_vm_set_opcode_handler(opline)

‎Zend/zend_vm_def.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10523,7 +10523,12 @@ ZEND_VM_DEFINE_OP(137, ZEND_OP_DATA);
1052310523
ZEND_VM_HELPER(zend_interrupt_helper, ANY, ANY)
1052410524
{
1052510525
zend_atomic_bool_store_ex(&EG(vm_interrupt), false);
10526+
#if ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL
10527+
/* opline is &call_interrupt_op. Load orig opline. */
10528+
LOAD_OPLINE();
10529+
#else
1052610530
SAVE_OPLINE();
10531+
#endif
1052710532
if (zend_atomic_bool_load_ex(&EG(timed_out))) {
1052810533
zend_timeout();
1052910534
} else if (zend_interrupt_function) {

‎Zend/zend_vm_execute.h‎

Lines changed: 48 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Zend/zend_vm_execute.skl‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,22 @@ ZEND_API const zend_op *zend_get_halt_op(void)
161161
#endif
162162
}
163163

164+
ZEND_API const zend_op *zend_get_interrupt_op(void)
165+
{
166+
#if ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL
167+
return &call_interrupt_op;
168+
#else
169+
return NULL;
170+
#endif
171+
}
172+
173+
#if ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL
174+
ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_FUNC_CCONV zend_vm_handle_interrupt(ZEND_OPCODE_HANDLER_ARGS)
175+
{
176+
return zend_interrupt_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU);
177+
}
178+
#endif
179+
164180
ZEND_API int zend_vm_kind(void)
165181
{
166182
return ZEND_VM_KIND;

‎Zend/zend_vm_gen.php‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1594,6 +1594,19 @@ function gen_halt_handler($f, $kind) {
15941594
out($f,"}\n\n");
15951595
}
15961596

1597+
function gen_interrupt_func($f, $kind, $spec) {
1598+
$cconv = $kind === ZEND_VM_KIND_TAILCALL ? 'ZEND_OPCODE_HANDLER_CCONV' : 'ZEND_OPCODE_HANDLER_FUNC_CCONV';
1599+
$variant = $kind === ZEND_VM_KIND_TAILCALL ? '_TAILCALL' : '';
1600+
out($f, "static ZEND_COLD zend_never_inline ZEND_OPCODE_HANDLER_RET {$cconv} zend_interrupt{$variant}(ZEND_OPCODE_HANDLER_ARGS) {\n");
1601+
out($f,"\tSAVE_OPLINE();\n");
1602+
if ($kind === ZEND_VM_KIND_TAILCALL) {
1603+
out($f,"\tZEND_VM_TAIL_CALL(zend_interrupt_helper".($spec?"_SPEC":"")."_TAILCALL(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU));\n");
1604+
} else {
1605+
out($f, "\treturn &call_interrupt_op;\n");
1606+
}
1607+
out($f, "}\n");
1608+
}
1609+
15971610
function extra_spec_name($extra_spec) {
15981611
global $prefix;
15991612

@@ -1804,10 +1817,14 @@ function gen_executor_code($f, $spec, $kind, $prolog, &$switch_labels = array())
18041817
switch ($kind) {
18051818
case ZEND_VM_KIND_CALL:
18061819
gen_null_handler($f, $kind);
1820+
out($f, "#if ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL\n");
1821+
gen_interrupt_func($f, $kind, $spec);
1822+
out($f, "#endif\n");
18071823
break;
18081824
case ZEND_VM_KIND_TAILCALL:
18091825
gen_null_handler($f, $kind);
18101826
gen_halt_handler($f, $kind);
1827+
gen_interrupt_func($f, $kind, $spec);
18111828
break;
18121829
case ZEND_VM_KIND_SWITCH:
18131830
out($f,"default: ZEND_NULL_LABEL:\n");
@@ -1845,7 +1862,7 @@ function gen_executor_code($f, $spec, $kind, $prolog, &$switch_labels = array())
18451862
out($f, "#pragma push_macro(\"ZEND_VM_INTERRUPT\")\n");
18461863
out($f, "#undef ZEND_VM_INTERRUPT\n");
18471864
out($f, "#define ZEND_VM_CONTINUE(handler) return opline\n");
1848-
out($f, "#define ZEND_VM_INTERRUPT() return zend_interrupt_helper".($spec?"_SPEC":"")."(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU)\n");
1865+
out($f, "#define ZEND_VM_INTERRUPT() return zend_interrupt(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU)\n");
18491866
out($f, $delayed_helpers);
18501867
out($f, "#pragma pop_macro(\"ZEND_VM_INTERRUPT\")\n");
18511868
out($f, "#pragma pop_macro(\"ZEND_VM_CONTINUE\")\n");
@@ -1900,7 +1917,10 @@ function gen_executor($f, $skl, $spec, $kind, $executor_name, $initializer_name)
19001917
if ($kind == ZEND_VM_KIND_HYBRID || $kind == ZEND_VM_KIND_CALL) {
19011918
out($f,"#if ZEND_VM_KIND == ZEND_VM_KIND_HYBRID || ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL\n\n");
19021919
out($f,"static zend_vm_opcode_handler_func_t const * zend_opcode_handler_funcs;\n");
1903-
out($f,"#endif\n");
1920+
out($f,"#endif\n\n");
1921+
out($f,"#if ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL\n");
1922+
out($f,"static const zend_op call_interrupt_op;\n");
1923+
out($f,"#endif\n\n");
19041924
}
19051925
out($f,"#if (ZEND_VM_KIND != ZEND_VM_KIND_HYBRID && ZEND_VM_KIND != ZEND_VM_KIND_TAILCALL) || !ZEND_VM_SPEC\n");
19061926
out($f,"static zend_vm_opcode_handler_t zend_vm_get_opcode_handler(uint8_t opcode, const zend_op* op);\n");
@@ -2139,9 +2159,11 @@ function gen_executor($f, $skl, $spec, $kind, $executor_name, $initializer_name)
21392159
out($f," ZEND_VM_TAIL_CALL(opline->handler(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU)); \\\n");
21402160
out($f," } while (0)\n");
21412161
out($f,"# define ZEND_VM_DISPATCH_TO_LEAVE_HELPER(helper) opline = &call_leave_op; SAVE_OPLINE(); ZEND_VM_CONTINUE()\n");
2142-
out($f,"# define ZEND_VM_INTERRUPT() ZEND_VM_TAIL_CALL(zend_interrupt_helper".($spec?"_SPEC":"")."_TAILCALL(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU))\n");
2162+
out($f,"# define ZEND_VM_INTERRUPT() ZEND_VM_TAIL_CALL(zend_interrupt_TAILCALL(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU))\n");
21432163
out($f,"\n");
21442164
out($f,"static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_CCONV zend_interrupt_helper".($spec?"_SPEC":"")."_TAILCALL(ZEND_OPCODE_HANDLER_ARGS);\n");
2165+
out($f,"static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_FUNC_CCONV zend_interrupt(ZEND_OPCODE_HANDLER_ARGS);\n");
2166+
out($f,"static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_CCONV zend_interrupt_TAILCALL(ZEND_OPCODE_HANDLER_ARGS);\n");
21452167
out($f,"static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_CCONV ZEND_NULL_TAILCALL_HANDLER(ZEND_OPCODE_HANDLER_ARGS);\n");
21462168
out($f,"static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_CCONV ZEND_HALT_TAILCALL_HANDLER(ZEND_OPCODE_HANDLER_ARGS);\n");
21472169
out($f,"static zend_never_inline const zend_op *ZEND_OPCODE_HANDLER_CCONV zend_leave_helper_SPEC_TAILCALL(zend_execute_data *ex, const zend_op *opline);\n");
@@ -2152,6 +2174,9 @@ function gen_executor($f, $skl, $spec, $kind, $executor_name, $initializer_name)
21522174
out($f,"static const zend_op call_leave_op = {\n");
21532175
out($f," .handler = zend_leave_helper_SPEC_TAILCALL,\n");
21542176
out($f,"};\n");
2177+
out($f,"static const zend_op call_interrupt_op = {\n");
2178+
out($f," .handler = zend_interrupt_helper_SPEC_TAILCALL,\n");
2179+
out($f,"};\n");
21552180
out($f,"\n");
21562181

21572182
gen_executor_code($f, $spec, ZEND_VM_KIND_TAILCALL, $m[1]);

‎ext/opcache/jit/zend_jit.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ int zend_jit_profile_counter_rid = -1;
7878
int16_t zend_jit_hot_counters[ZEND_HOT_COUNTERS_COUNT];
7979

8080
const zend_op *zend_jit_halt_op = NULL;
81+
const zend_op *zend_jit_interrupt_op = NULL;
8182
#ifdef HAVE_PTHREAD_JIT_WRITE_PROTECT_NP
8283
static int zend_write_protect = 1;
8384
#endif
@@ -3777,6 +3778,7 @@ int zend_jit_check_support(void)
37773778
void zend_jit_startup(void *buf, size_t size, bool reattached)
37783779
{
37793780
zend_jit_halt_op = zend_get_halt_op();
3781+
zend_jit_interrupt_op = zend_get_interrupt_op();
37803782
zend_jit_profile_counter_rid = zend_get_op_array_extension_handle(ACCELERATOR_PRODUCT_NAME);
37813783

37823784
#ifdef HAVE_PTHREAD_JIT_WRITE_PROTECT_NP

‎ext/opcache/jit/zend_jit_internal.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ typedef struct _zend_jit_op_array_hot_extension {
177177
zend_jit_hash((op_array)->opcodes)
178178

179179
extern const zend_op *zend_jit_halt_op;
180+
extern const zend_op *zend_jit_interrupt_op;
180181

181182
#ifdef HAVE_GCC_GLOBAL_REGS
182183
# define EXECUTE_DATA_D void

‎ext/opcache/jit/zend_jit_vm_helpers.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,11 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex,
10721072
if (UNEXPECTED(opline == zend_jit_halt_op)) {
10731073
#else
10741074
opline = handler(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU);
1075+
# if ZEND_VM_KIND == ZEND_VM_KIND_TAILCALL
1076+
while (UNEXPECTED(opline == zend_jit_interrupt_op)) {
1077+
opline = zend_vm_handle_interrupt(ZEND_OPCODE_HANDLER_ARGS_PASSTHRU);
1078+
}
1079+
# endif
10751080
if (UNEXPECTED(((uintptr_t)opline & ~ZEND_VM_ENTER_BIT) == 0)) {
10761081
#endif
10771082
if (prev_opline->opcode == ZEND_YIELD || prev_opline->opcode == ZEND_YIELD_FROM) {

‎ext/zend_test/object_handlers.c‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,44 @@ ZEND_METHOD(NumericCastableNoOperations, __construct)
232232
ZVAL_COPY(OBJ_PROP_NUM(Z_OBJ_P(ZEND_THIS), 0), n);
233233
}
234234

235+
static zend_class_entry *vm_interrupt_comparable_ce;
236+
static zend_object_handlers vm_interrupt_comparable_object_handlers;
237+
238+
static zend_object* vm_interrupt_comparable_object_create_ex(zend_class_entry* ce, zend_long l) {
239+
zend_object *obj = zend_objects_new(ce);
240+
object_properties_init(obj, ce);
241+
obj->handlers = &vm_interrupt_comparable_object_handlers;
242+
ZVAL_LONG(OBJ_PROP_NUM(obj, 0), l);
243+
return obj;
244+
}
245+
246+
static zend_object *vm_interrupt_comparable_object_create(zend_class_entry *ce)
247+
{
248+
return vm_interrupt_comparable_object_create_ex(ce, 0);
249+
}
250+
251+
static int vm_interrupt_comparable_compare(zval *op1, zval *op2)
252+
{
253+
ZEND_COMPARE_OBJECTS_FALLBACK(op1, op2);
254+
255+
zend_atomic_bool_store_ex(&EG(vm_interrupt), true);
256+
257+
return ZEND_THREEWAY_COMPARE(
258+
Z_LVAL_P(OBJ_PROP_NUM(Z_OBJ_P(op1), 0)),
259+
Z_LVAL_P(OBJ_PROP_NUM(Z_OBJ_P(op2), 0)));
260+
}
261+
262+
ZEND_METHOD(VmInterruptComparable, __construct)
263+
{
264+
zend_long l;
265+
266+
ZEND_PARSE_PARAMETERS_START(1, 1)
267+
Z_PARAM_LONG(l)
268+
ZEND_PARSE_PARAMETERS_END();
269+
270+
ZVAL_LONG(OBJ_PROP_NUM(Z_OBJ_P(ZEND_THIS), 0), l);
271+
}
272+
235273
static zend_class_entry *dimension_handlers_no_ArrayAccess_ce;
236274
static zend_object_handlers dimension_handlers_no_ArrayAccess_object_handlers;
237275

@@ -302,6 +340,11 @@ void zend_test_object_handlers_init(void)
302340
memcpy(&numeric_castable_no_operation_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
303341
numeric_castable_no_operation_object_handlers.cast_object = numeric_castable_no_operation_cast_object;
304342

343+
vm_interrupt_comparable_ce = register_class_VmInterruptComparable();
344+
vm_interrupt_comparable_ce->create_object = vm_interrupt_comparable_object_create;
345+
memcpy(&vm_interrupt_comparable_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
346+
vm_interrupt_comparable_object_handlers.compare = vm_interrupt_comparable_compare;
347+
305348
dimension_handlers_no_ArrayAccess_ce = register_class_DimensionHandlersNoArrayAccess();
306349
dimension_handlers_no_ArrayAccess_ce->create_object = dimension_handlers_no_ArrayAccess_object_create;
307350
memcpy(&dimension_handlers_no_ArrayAccess_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));

0 commit comments

Comments
 (0)