Skip to content

Commit 9af2f0f

Browse files
dstogovdstogov
committed
More accurate tracing JIT for RETURN with unknown return address
1 parent 3f76947 commit 9af2f0f

File tree

4 files changed

+20
-19
lines changed

4 files changed

+20
-19
lines changed

‎ext/opcache/jit/zend_jit_internal.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,6 @@ typedef struct _zend_jit_trace_rec {
271271
};
272272
struct {
273273
union {
274-
uint8_t recursive; /* part of recursive return sequence for ZEND_JIT_TRACE_BACK */
275274
int8_t return_value_used; /* for ZEND_JIT_TRACE_ENTER */
276275
uint8_t fake; /* for ZEND_JIT_TRACE_INIT_CALL */
277276
};
@@ -382,18 +381,17 @@ struct _zend_jit_trace_stack_frame {
382381
#define TRACE_FRAME_MASK_RETURN_VALUE_USED 0x00000008
383382
#define TRACE_FRAME_MASK_RETURN_VALUE_UNUSED 0x00000010
384383
#define TRACE_FRAME_MASK_THIS_CHECKED 0x00000020
384+
#define TRACE_FRAME_MASK_UNKNOWN_RETURN 0x00000040
385385

386386

387-
#define TRACE_FRAME_INIT(frame, _func, nested, num_args) do { \
387+
#define TRACE_FRAME_INIT(frame, _func, _flags, num_args) do { \
388388
zend_jit_trace_stack_frame *_frame = (frame); \
389389
_frame->call = NULL; \
390390
_frame->prev = NULL; \
391391
_frame->func = (const zend_function*)_func; \
392392
_frame->call_level = 0; \
393393
_frame->_info = (((uint32_t)(num_args)) << TRACE_FRAME_SHIFT_NUM_ARGS) & TRACE_FRAME_MASK_NUM_ARGS; \
394-
if (nested) { \
395-
_frame->_info |= TRACE_FRAME_MASK_NESTED; \
396-
}; \
394+
_frame->_info |= _flags; \
397395
} while (0)
398396

399397
#define TRACE_FRAME_RETURN_SSA_VAR(frame) \
@@ -412,6 +410,8 @@ struct _zend_jit_trace_stack_frame {
412410
((frame)->_info & TRACE_FRAME_MASK_RETURN_VALUE_UNUSED)
413411
#define TRACE_FRAME_IS_THIS_CHECKED(frame) \
414412
((frame)->_info & TRACE_FRAME_MASK_THIS_CHECKED)
413+
#define TRACE_FRAME_IS_UNKNOWN_RETURN(frame) \
414+
((frame)->_info & TRACE_FRAME_MASK_UNKNOWN_RETURN)
415415

416416
#define TRACE_FRAME_SET_RETURN_SSA_VAR(frame, var) do { \
417417
(frame)->_info = var; \

‎ext/opcache/jit/zend_jit_trace.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ static int zend_jit_trace_may_exit(const zend_op_array *op_array, const zend_op
307307
case ZEND_RETURN_BY_REF:
308308
case ZEND_RETURN:
309309
/* return */
310-
return trace->op == ZEND_JIT_TRACE_BACK && trace->recursive;
310+
return !JIT_G(current_frame) || TRACE_FRAME_IS_UNKNOWN_RETURN(JIT_G(current_frame));
311311
default:
312312
break;
313313
}
@@ -2479,7 +2479,7 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
24792479
op_array = p->op_array;
24802480
frame = JIT_G(current_frame);
24812481
top = zend_jit_trace_call_frame(frame, op_array);
2482-
TRACE_FRAME_INIT(frame, op_array, 0, -1);
2482+
TRACE_FRAME_INIT(frame, op_array, TRACE_FRAME_MASK_UNKNOWN_RETURN, -1);
24832483
stack = frame->stack;
24842484

24852485
opline = ((zend_jit_trace_start_rec*)p)->opline;
@@ -3872,7 +3872,7 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
38723872
ZEND_ASSERT(&frame->func->op_array == op_array);
38733873
} else {
38743874
frame = zend_jit_trace_ret_frame(frame, op_array);
3875-
TRACE_FRAME_INIT(frame, op_array, 0, -1);
3875+
TRACE_FRAME_INIT(frame, op_array, TRACE_FRAME_MASK_UNKNOWN_RETURN, -1);
38763876
stack = frame->stack;
38773877
for (i = 0; i < op_array->last_var + op_array->T; i++) {
38783878
/* Initialize abstract stack using SSA */
@@ -3917,7 +3917,7 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
39173917
}
39183918

39193919
call = top;
3920-
TRACE_FRAME_INIT(call, p->func, 1, num_args);
3920+
TRACE_FRAME_INIT(call, p->func, TRACE_FRAME_MASK_NESTED, num_args);
39213921
call->prev = frame->call;
39223922
if (!p->fake) {
39233923
TRACE_FRAME_SET_LAST_SEND_BY_VAL(call);

‎ext/opcache/jit/zend_jit_vm_helpers.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,8 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_loop_trace_helper(ZEND_OPCODE_HAN
367367
break; \
368368
}
369369

370-
#define TRACE_RECORD_BACK(_op, _recursive, _ptr) \
370+
#define TRACE_RECORD_BACK(_op, _ptr) \
371371
trace_buffer[idx].op = _op; \
372-
trace_buffer[idx].recursive = _recursive; \
373372
trace_buffer[idx].ptr = _ptr; \
374373
idx++; \
375374
if (idx >= ZEND_JIT_TRACE_MAX_LENGTH - 1) { \
@@ -545,7 +544,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex,
545544
zend_execute_data *save_execute_data = execute_data;
546545
const zend_op *save_opline = opline;
547546
#endif
548-
const zend_op *orig_opline;
547+
const zend_op *orig_opline, *end_opline;
549548
zend_jit_trace_stop stop = ZEND_JIT_TRACE_STOP_ERROR;
550549
int level = 0;
551550
int ret_level = 0;
@@ -757,7 +756,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex,
757756
stop = ZEND_JIT_TRACE_STOP_TOO_DEEP_RET;
758757
break;
759758
}
760-
TRACE_RECORD_BACK(ZEND_JIT_TRACE_BACK, 1, &EX(func)->op_array);
759+
TRACE_RECORD_BACK(ZEND_JIT_TRACE_BACK, &EX(func)->op_array);
761760
count = zend_jit_trace_recursive_ret_count(&EX(func)->op_array, unrolled_calls, ret_level);
762761
if (opline == orig_opline) {
763762
if (count + 1 >= ZEND_JIT_TRACE_MAX_RECURSION) {
@@ -791,7 +790,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex,
791790
}
792791
} else {
793792
level--;
794-
TRACE_RECORD_BACK(ZEND_JIT_TRACE_BACK, 0, &EX(func)->op_array);
793+
TRACE_RECORD_BACK(ZEND_JIT_TRACE_BACK, &EX(func)->op_array);
795794
}
796795
}
797796
#ifdef HAVE_GCC_GLOBAL_REGS
@@ -877,14 +876,17 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex,
877876
}
878877
}
879878

879+
end_opline = opline;
880880
if (!ZEND_JIT_TRACE_STOP_OK(stop)) {
881881
if (backtrack_recursion > 0) {
882882
idx = backtrack_recursion;
883883
stop = ZEND_JIT_TRACE_STOP_RECURSIVE_CALL;
884+
end_opline = orig_opline;
884885
} else if (backtrack_ret_recursion > 0) {
885886
idx = backtrack_ret_recursion;
886887
ret_level = backtrack_ret_recursion_level;
887888
stop = ZEND_JIT_TRACE_STOP_RECURSIVE_RET;
889+
end_opline = orig_opline;
888890
}
889891
}
890892

@@ -895,7 +897,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex,
895897
}
896898
}
897899

898-
TRACE_END(ZEND_JIT_TRACE_END, stop, opline);
900+
TRACE_END(ZEND_JIT_TRACE_END, stop, end_opline);
899901

900902
#ifdef HAVE_GCC_GLOBAL_REGS
901903
if (stop != ZEND_JIT_TRACE_STOP_HALT) {

‎ext/opcache/jit/zend_jit_x86.dasc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9477,7 +9477,7 @@ static int zend_jit_leave_func(dasm_State **Dst, const zend_op *opline, const ze
94779477
|9:
94789478
if (trace) {
94799479
if (trace->op != ZEND_JIT_TRACE_END
9480-
&& (trace->op != ZEND_JIT_TRACE_BACK || !trace->recursive)) {
9480+
&& (JIT_G(current_frame) && !TRACE_FRAME_IS_UNKNOWN_RETURN(JIT_G(current_frame)))) {
94819481
zend_jit_reset_opline(Dst, NULL);
94829482
} else {
94839483
// TODO: exception handling for tracing JIT ???
@@ -9487,9 +9487,8 @@ static int zend_jit_leave_func(dasm_State **Dst, const zend_op *opline, const ze
94879487

94889488
|8:
94899489

9490-
if (opline->opcode == ZEND_RETURN
9491-
&& trace->op == ZEND_JIT_TRACE_BACK
9492-
&& trace->recursive) {
9490+
if (trace->op == ZEND_JIT_TRACE_BACK
9491+
&& (!JIT_G(current_frame) || TRACE_FRAME_IS_UNKNOWN_RETURN(JIT_G(current_frame)))) {
94939492
const zend_op *next_opline = trace->opline;
94949493
uint32_t exit_point;
94959494
const void *exit_addr;

0 commit comments

Comments
 (0)