From 9af2f0fa78d6255193b8bffcf30f68fe9389c20e Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Thu, 7 May 2020 22:01:59 +0300 Subject: [PATCH] More accurate tracing JIT for RETURN with unknown return address --- ext/opcache/jit/zend_jit_internal.h | 10 +++++----- ext/opcache/jit/zend_jit_trace.c | 8 ++++---- ext/opcache/jit/zend_jit_vm_helpers.c | 14 ++++++++------ ext/opcache/jit/zend_jit_x86.dasc | 7 +++---- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h index 082f10b980..d8062dc253 100644 --- a/ext/opcache/jit/zend_jit_internal.h +++ b/ext/opcache/jit/zend_jit_internal.h @@ -271,7 +271,6 @@ typedef struct _zend_jit_trace_rec { }; struct { union { - uint8_t recursive; /* part of recursive return sequence for ZEND_JIT_TRACE_BACK */ int8_t return_value_used; /* for ZEND_JIT_TRACE_ENTER */ uint8_t fake; /* for ZEND_JIT_TRACE_INIT_CALL */ }; @@ -382,18 +381,17 @@ struct _zend_jit_trace_stack_frame { #define TRACE_FRAME_MASK_RETURN_VALUE_USED 0x00000008 #define TRACE_FRAME_MASK_RETURN_VALUE_UNUSED 0x00000010 #define TRACE_FRAME_MASK_THIS_CHECKED 0x00000020 +#define TRACE_FRAME_MASK_UNKNOWN_RETURN 0x00000040 -#define TRACE_FRAME_INIT(frame, _func, nested, num_args) do { \ +#define TRACE_FRAME_INIT(frame, _func, _flags, num_args) do { \ zend_jit_trace_stack_frame *_frame = (frame); \ _frame->call = NULL; \ _frame->prev = NULL; \ _frame->func = (const zend_function*)_func; \ _frame->call_level = 0; \ _frame->_info = (((uint32_t)(num_args)) << TRACE_FRAME_SHIFT_NUM_ARGS) & TRACE_FRAME_MASK_NUM_ARGS; \ - if (nested) { \ - _frame->_info |= TRACE_FRAME_MASK_NESTED; \ - }; \ + _frame->_info |= _flags; \ } while (0) #define TRACE_FRAME_RETURN_SSA_VAR(frame) \ @@ -412,6 +410,8 @@ struct _zend_jit_trace_stack_frame { ((frame)->_info & TRACE_FRAME_MASK_RETURN_VALUE_UNUSED) #define TRACE_FRAME_IS_THIS_CHECKED(frame) \ ((frame)->_info & TRACE_FRAME_MASK_THIS_CHECKED) +#define TRACE_FRAME_IS_UNKNOWN_RETURN(frame) \ + ((frame)->_info & TRACE_FRAME_MASK_UNKNOWN_RETURN) #define TRACE_FRAME_SET_RETURN_SSA_VAR(frame, var) do { \ (frame)->_info = var; \ diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index 270fe0cec0..46455f8d54 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -307,7 +307,7 @@ static int zend_jit_trace_may_exit(const zend_op_array *op_array, const zend_op case ZEND_RETURN_BY_REF: case ZEND_RETURN: /* return */ - return trace->op == ZEND_JIT_TRACE_BACK && trace->recursive; + return !JIT_G(current_frame) || TRACE_FRAME_IS_UNKNOWN_RETURN(JIT_G(current_frame)); default: break; } @@ -2479,7 +2479,7 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par op_array = p->op_array; frame = JIT_G(current_frame); top = zend_jit_trace_call_frame(frame, op_array); - TRACE_FRAME_INIT(frame, op_array, 0, -1); + TRACE_FRAME_INIT(frame, op_array, TRACE_FRAME_MASK_UNKNOWN_RETURN, -1); stack = frame->stack; opline = ((zend_jit_trace_start_rec*)p)->opline; @@ -3872,7 +3872,7 @@ done: ZEND_ASSERT(&frame->func->op_array == op_array); } else { frame = zend_jit_trace_ret_frame(frame, op_array); - TRACE_FRAME_INIT(frame, op_array, 0, -1); + TRACE_FRAME_INIT(frame, op_array, TRACE_FRAME_MASK_UNKNOWN_RETURN, -1); stack = frame->stack; for (i = 0; i < op_array->last_var + op_array->T; i++) { /* Initialize abstract stack using SSA */ @@ -3917,7 +3917,7 @@ done: } call = top; - TRACE_FRAME_INIT(call, p->func, 1, num_args); + TRACE_FRAME_INIT(call, p->func, TRACE_FRAME_MASK_NESTED, num_args); call->prev = frame->call; if (!p->fake) { TRACE_FRAME_SET_LAST_SEND_BY_VAL(call); diff --git a/ext/opcache/jit/zend_jit_vm_helpers.c b/ext/opcache/jit/zend_jit_vm_helpers.c index 2cfb79dbb6..c4a515330d 100644 --- a/ext/opcache/jit/zend_jit_vm_helpers.c +++ b/ext/opcache/jit/zend_jit_vm_helpers.c @@ -367,9 +367,8 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_loop_trace_helper(ZEND_OPCODE_HAN break; \ } -#define TRACE_RECORD_BACK(_op, _recursive, _ptr) \ +#define TRACE_RECORD_BACK(_op, _ptr) \ trace_buffer[idx].op = _op; \ - trace_buffer[idx].recursive = _recursive; \ trace_buffer[idx].ptr = _ptr; \ idx++; \ 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, zend_execute_data *save_execute_data = execute_data; const zend_op *save_opline = opline; #endif - const zend_op *orig_opline; + const zend_op *orig_opline, *end_opline; zend_jit_trace_stop stop = ZEND_JIT_TRACE_STOP_ERROR; int level = 0; int ret_level = 0; @@ -757,7 +756,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex, stop = ZEND_JIT_TRACE_STOP_TOO_DEEP_RET; break; } - TRACE_RECORD_BACK(ZEND_JIT_TRACE_BACK, 1, &EX(func)->op_array); + TRACE_RECORD_BACK(ZEND_JIT_TRACE_BACK, &EX(func)->op_array); count = zend_jit_trace_recursive_ret_count(&EX(func)->op_array, unrolled_calls, ret_level); if (opline == orig_opline) { 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, } } else { level--; - TRACE_RECORD_BACK(ZEND_JIT_TRACE_BACK, 0, &EX(func)->op_array); + TRACE_RECORD_BACK(ZEND_JIT_TRACE_BACK, &EX(func)->op_array); } } #ifdef HAVE_GCC_GLOBAL_REGS @@ -877,14 +876,17 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex, } } + end_opline = opline; if (!ZEND_JIT_TRACE_STOP_OK(stop)) { if (backtrack_recursion > 0) { idx = backtrack_recursion; stop = ZEND_JIT_TRACE_STOP_RECURSIVE_CALL; + end_opline = orig_opline; } else if (backtrack_ret_recursion > 0) { idx = backtrack_ret_recursion; ret_level = backtrack_ret_recursion_level; stop = ZEND_JIT_TRACE_STOP_RECURSIVE_RET; + end_opline = orig_opline; } } @@ -895,7 +897,7 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex, } } - TRACE_END(ZEND_JIT_TRACE_END, stop, opline); + TRACE_END(ZEND_JIT_TRACE_END, stop, end_opline); #ifdef HAVE_GCC_GLOBAL_REGS if (stop != ZEND_JIT_TRACE_STOP_HALT) { diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index 7f2bb4b394..642fb70211 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -9477,7 +9477,7 @@ static int zend_jit_leave_func(dasm_State **Dst, const zend_op *opline, const ze |9: if (trace) { if (trace->op != ZEND_JIT_TRACE_END - && (trace->op != ZEND_JIT_TRACE_BACK || !trace->recursive)) { + && (JIT_G(current_frame) && !TRACE_FRAME_IS_UNKNOWN_RETURN(JIT_G(current_frame)))) { zend_jit_reset_opline(Dst, NULL); } else { // 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 |8: - if (opline->opcode == ZEND_RETURN - && trace->op == ZEND_JIT_TRACE_BACK - && trace->recursive) { + if (trace->op == ZEND_JIT_TRACE_BACK + && (!JIT_G(current_frame) || TRACE_FRAME_IS_UNKNOWN_RETURN(JIT_G(current_frame)))) { const zend_op *next_opline = trace->opline; uint32_t exit_point; const void *exit_addr; -- 2.40.0