Skip to content

Commit 144dd19

Browse files
arnaud-lbarnaud-lb
committed
Save registers in generated code
Saving them in execute_ex is not safe when not assigning them to global regs, as the compiler allocate them.
1 parent dbcf4a1 commit 144dd19

File tree

4 files changed

+23
-60
lines changed

4 files changed

+23
-60
lines changed

‎Zend/zend_execute.c

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -71,41 +71,6 @@
7171
# endif
7272
#endif
7373

74-
#ifdef __clang__
75-
# if defined(i386)
76-
# define ZEND_VM_FP_LOCAL_REG "%esi"
77-
# define ZEND_VM_IP_LOCAL_REG "%edi"
78-
# elif defined(__x86_64__)
79-
# define ZEND_VM_FP_LOCAL_REG "%r14"
80-
# define ZEND_VM_IP_LOCAL_REG "%r15"
81-
// TODO: not portable
82-
# define ZEND_VM_SAVE_REGISTER(reg, r) __asm__ volatile ( \
83-
"movq %" reg ", %0" \
84-
: "=r" (r) \
85-
: \
86-
: \
87-
)
88-
# define ZEND_VM_RESTORE_REGISTER(reg, r) __asm__ volatile ( \
89-
"movq %0, %" reg \
90-
: \
91-
: "r" (r) \
92-
: reg \
93-
)
94-
# elif defined(__powerpc64__)
95-
# define ZEND_VM_FP_LOCAL_REG "r14"
96-
# define ZEND_VM_IP_LOCAL_REG "r15"
97-
# elif defined(__IBMC__) && ZEND_GCC_VERSION >= 4002 && defined(__powerpc64__)
98-
# define ZEND_VM_FP_LOCAL_REG "r14"
99-
# define ZEND_VM_IP_LOCAL_REG "r15"
100-
# elif defined(__aarch64__)
101-
# define ZEND_VM_FP_LOCAL_REG "x27"
102-
# define ZEND_VM_IP_LOCAL_REG "x28"
103-
# elif defined(__riscv) && __riscv_xlen == 64
104-
# define ZEND_VM_FP_LOCAL_REG "x18"
105-
# define ZEND_VM_IP_LOCAL_REG "x19"
106-
# endif
107-
#endif
108-
10974
#if defined(ZEND_VM_FP_GLOBAL_REG) && ((ZEND_VM_KIND == ZEND_VM_KIND_CALL) || (ZEND_VM_KIND == ZEND_VM_KIND_HYBRID))
11075
# pragma GCC diagnostic ignored "-Wvolatile-register-var"
11176
register zend_execute_data* volatile execute_data __asm__(ZEND_VM_FP_GLOBAL_REG);

‎Zend/zend_vm_execute.h

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

‎Zend/zend_vm_gen.php

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2060,30 +2060,25 @@ function gen_executor($f, $skl, $spec, $kind, $executor_name, $initializer_name)
20602060
out($f,$m[1]."zend_execute_data *execute_data = ex;\n");
20612061
out($f,"#endif\n");
20622062
} else {
2063-
out($f,"#if defined(ZEND_VM_IP_GLOBAL_REG) || defined(ZEND_VM_FP_GLOBAL_REG) || defined(ZEND_VM_FP_LOCAL_REG) || defined(ZEND_VM_IP_LOCAL_REG)\n");
2063+
out($f,"#if defined(ZEND_VM_IP_GLOBAL_REG) || defined(ZEND_VM_FP_GLOBAL_REG)\n");
20642064
out($f,$m[1]."struct {\n");
20652065
out($f,"#ifdef ZEND_VM_HYBRID_JIT_RED_ZONE_SIZE\n");
20662066
out($f,$m[1]."\tchar hybrid_jit_red_zone[ZEND_VM_HYBRID_JIT_RED_ZONE_SIZE];\n");
20672067
out($f,"#endif\n");
2068-
out($f,"#if defined(ZEND_VM_IP_GLOBAL_REG) || defined(ZEND_VM_IP_LOCAL_REG)\n");
2068+
out($f,"#ifdef ZEND_VM_IP_GLOBAL_REG\n");
20692069
out($f,$m[1]."\tconst zend_op *orig_opline;\n");
20702070
out($f,"#endif\n");
2071-
out($f,"#if defined(ZEND_VM_FP_GLOBAL_REG) || defined(ZEND_VM_FP_LOCAL_REG)\n");
2071+
out($f,"#ifdef ZEND_VM_FP_GLOBAL_REG\n");
20722072
out($f,$m[1]."\tzend_execute_data *orig_execute_data;\n");
20732073
out($f,"#endif\n");
20742074
out($f,$m[1]."} vm_stack_data;\n");
20752075
out($f,"#endif\n");
20762076
out($f,"#ifdef ZEND_VM_IP_GLOBAL_REG\n");
20772077
out($f,$m[1]."vm_stack_data.orig_opline = opline;\n");
2078-
out($f,"#elif defined(ZEND_VM_IP_LOCAL_REG)\n");
2079-
out($f,$m[1]."ZEND_VM_SAVE_REGISTER(ZEND_VM_IP_LOCAL_REG, vm_stack_data.orig_opline);\n");
20802078
out($f,"#endif\n");
20812079
out($f,"#ifdef ZEND_VM_FP_GLOBAL_REG\n");
20822080
out($f,$m[1]."vm_stack_data.orig_execute_data = execute_data;\n");
20832081
out($f,$m[1]."execute_data = ex;\n");
2084-
out($f,"#elif defined(ZEND_VM_FP_LOCAL_REG)\n");
2085-
out($f,$m[1]."ZEND_VM_SAVE_REGISTER(ZEND_VM_FP_LOCAL_REG, vm_stack_data.orig_opline);\n");
2086-
out($f,$m[1]."zend_execute_data *execute_data = ex;\n");
20872082
out($f,"#else\n");
20882083
out($f,$m[1]."zend_execute_data *execute_data = ex;\n");
20892084
out($f,"#endif\n");
@@ -2187,10 +2182,7 @@ function gen_executor($f, $skl, $spec, $kind, $executor_name, $initializer_name)
21872182
$m[1]."} else {\n" .
21882183
"# ifdef ZEND_VM_IP_GLOBAL_REG\n" .
21892184
$m[1]."\topline = vm_stack_data.orig_opline;\n" .
2190-
"# elif defined(ZEND_VM_FP_LOCAL_REG) || defined(ZEND_VM_IP_LOCAL_REG)\n".
2191-
$m[1]."\tZEND_VM_RESTORE_REGISTER(ZEND_VM_IP_LOCAL_REG, vm_stack_data.orig_opline);\n".
2192-
$m[1]."\tZEND_VM_RESTORE_REGISTER(ZEND_VM_FP_LOCAL_REG, vm_stack_data.orig_execute_data);\n".
2193-
" #endif\n".
2185+
"# endif\n" .
21942186
$m[1]."\treturn;\n".
21952187
$m[1]."}\n".
21962188
"#endif\n");

‎ext/opcache/jit/zend_jit_ir.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2672,7 +2672,15 @@ static void zend_jit_init_ctx(zend_jit_ctx *jit, uint32_t flags)
26722672
#else /* IR_TARGET_x86 */
26732673
jit->ctx.fixed_stack_frame_size = sizeof(void*) * 11; /* 4 saved registers and 7 spill slots (4 bytes) */
26742674
#endif
2675+
/* JIT-ed code is called only from execute_ex, which takes care
2676+
* of saving ZREG_FP, ZREG_IP when GCC_GLOBAL_REGS is 1, so we don't
2677+
* have to save them. When GCC_GLOBAL_REGS is 1, always save them.
2678+
*/
2679+
#if GCC_GLOBAL_REGS
26752680
jit->ctx.fixed_save_regset = IR_REGSET_PRESERVED & ~((1<<ZREG_FP) | (1<<ZREG_IP));
2681+
#else
2682+
jit->ctx.fixed_save_regset = IR_REGSET_PRESERVED;
2683+
#endif
26762684
//#ifdef _WIN64
26772685
// jit->ctx.fixed_save_regset &= 0xffff; // TODO: don't save FP registers ???
26782686
//#endif
@@ -17037,6 +17045,12 @@ static int zend_jit_trace_handler(zend_jit_ctx *jit, const zend_op_array *op_arr
1703717045
ir_CALL(IR_VOID, ir_CONST_FUNC(handler));
1703817046
} else {
1703917047
ref = ir_CALL_2(IR_ADDR, ir_CONST_FC_FUNC(handler), jit_FP(jit), jit_IP(jit));
17048+
if (opline->opcode == ZEND_RETURN ||
17049+
opline->opcode == ZEND_RETURN_BY_REF ||
17050+
opline->opcode == ZEND_GENERATOR_CREATE) {
17051+
// TODO: what other ops need this?
17052+
ref = ir_AND_A(ref, ir_CONST_ADDR(~(1ULL<<63)));
17053+
}
1704017054
jit_LOAD_IP(jit, ref);
1704117055
}
1704217056
if (may_throw

0 commit comments

Comments
 (0)