From a743fd7633febe56b78feead06d2c77b0faa4ea2 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Thu, 7 Oct 2021 15:27:56 +0300 Subject: [PATCH 1/2] JIT: Fixed wrong comparison skip --- ext/opcache/jit/zend_jit_trace.c | 40 +++++++++++++++++++----------- ext/opcache/tests/jit/cmp_007.phpt | 23 +++++++++++++++++ 2 files changed, 48 insertions(+), 15 deletions(-) create mode 100644 ext/opcache/tests/jit/cmp_007.phpt diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index 62079345371..acce9324d78 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -3433,7 +3433,7 @@ static void zend_jit_trace_update_condition_ranges(const zend_op *opline, const } } -static zend_bool zend_jit_may_skip_comparison(const zend_op *opline, const zend_ssa_op *ssa_op, const zend_ssa *ssa, const zend_op **ssa_opcodes) +static zend_bool zend_jit_may_skip_comparison(const zend_op *opline, const zend_ssa_op *ssa_op, const zend_ssa *ssa, const zend_op **ssa_opcodes, const zend_op_array *op_array) { zend_uchar prev_opcode; @@ -3442,18 +3442,23 @@ static zend_bool zend_jit_may_skip_comparison(const zend_op *opline, const zend_ && Z_LVAL_P(RT_CONSTANT(opline, opline->op1)) == 0) { if (ssa_op->op2_use >= 0) { if ((ssa_op-1)->op1_def == ssa_op->op2_use) { - prev_opcode = ssa_opcodes[(ssa_op - ssa->ops) - 1]->opcode; + ssa_op--; + opline = ssa_opcodes[ssa_op - ssa->ops]; + prev_opcode = opline->opcode; if (prev_opcode == ZEND_PRE_INC || prev_opcode == ZEND_PRE_DEC || prev_opcode == ZEND_POST_INC || prev_opcode == ZEND_POST_DEC) { - return 1; + return (OP1_INFO() & ((MAY_BE_UNDEF|MAY_BE_ANY|MAY_BE_REF)-MAY_BE_LONG)) == 0; } } else if ((ssa_op-1)->result_def == ssa_op->op2_use) { - prev_opcode = ssa_opcodes[(ssa_op - ssa->ops) - 1]->opcode; + ssa_op--; + opline = ssa_opcodes[ssa_op - ssa->ops]; + prev_opcode = opline->opcode; if (prev_opcode == ZEND_ADD || prev_opcode == ZEND_SUB) { - return 1; + return (OP1_INFO() & ((MAY_BE_UNDEF|MAY_BE_ANY|MAY_BE_REF)-MAY_BE_LONG)) == 0 && + (OP2_INFO() & ((MAY_BE_UNDEF|MAY_BE_ANY|MAY_BE_REF)-MAY_BE_LONG)) == 0; } } } @@ -3462,18 +3467,23 @@ static zend_bool zend_jit_may_skip_comparison(const zend_op *opline, const zend_ && Z_LVAL_P(RT_CONSTANT(opline, opline->op2)) == 0) { if (ssa_op->op1_use >= 0) { if ((ssa_op-1)->op1_def == ssa_op->op1_use) { - prev_opcode = ssa_opcodes[(ssa_op - ssa->ops) - 1]->opcode; + ssa_op--; + opline = ssa_opcodes[ssa_op - ssa->ops]; + prev_opcode = opline->opcode; if (prev_opcode == ZEND_PRE_INC || prev_opcode == ZEND_PRE_DEC || prev_opcode == ZEND_POST_INC || prev_opcode == ZEND_POST_DEC) { - return 1; + return (OP1_INFO() & ((MAY_BE_UNDEF|MAY_BE_ANY|MAY_BE_REF)-MAY_BE_LONG)) == 0; } } else if ((ssa_op-1)->result_def == ssa_op->op1_use) { - prev_opcode = ssa_opcodes[(ssa_op - ssa->ops) - 1]->opcode; + ssa_op--; + opline = ssa_opcodes[ssa_op - ssa->ops]; + prev_opcode = opline->opcode; if (prev_opcode == ZEND_ADD || prev_opcode == ZEND_SUB) { - return 1; + return (OP1_INFO() & ((MAY_BE_UNDEF|MAY_BE_ANY|MAY_BE_REF)-MAY_BE_LONG)) == 0 && + (OP2_INFO() & ((MAY_BE_UNDEF|MAY_BE_ANY|MAY_BE_REF)-MAY_BE_LONG)) == 0; } } } @@ -4777,9 +4787,9 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par op2_info = OP2_INFO(); skip_comparison = ssa_op != ssa->ops && - (op1_info & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_LONG && - (op2_info & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_LONG && - zend_jit_may_skip_comparison(opline, ssa_op, ssa, ssa_opcodes); + (op1_info & (MAY_BE_ANY|MAY_BE_UNDEF|MAY_BE_GUARD)) == MAY_BE_LONG && + (op2_info & (MAY_BE_ANY|MAY_BE_UNDEF|MAY_BE_GUARD)) == MAY_BE_LONG && + zend_jit_may_skip_comparison(opline, ssa_op, ssa, ssa_opcodes, op_array); CHECK_OP1_TRACE_TYPE(); CHECK_OP2_TRACE_TYPE(); if ((opline->result_type & (IS_SMART_BRANCH_JMPZ|IS_SMART_BRANCH_JMPNZ)) != 0) { @@ -4825,9 +4835,9 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par op2_info = OP2_INFO(); skip_comparison = ssa_op != ssa->ops && - (op1_info & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_LONG && - (op2_info & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_LONG && - zend_jit_may_skip_comparison(opline, ssa_op, ssa, ssa_opcodes); + (op1_info & (MAY_BE_ANY|MAY_BE_UNDEF|MAY_BE_GUARD)) == MAY_BE_LONG && + (op2_info & (MAY_BE_ANY|MAY_BE_UNDEF|MAY_BE_GUARD)) == MAY_BE_LONG && + zend_jit_may_skip_comparison(opline, ssa_op, ssa, ssa_opcodes, op_array); CHECK_OP1_TRACE_TYPE(); CHECK_OP2_TRACE_TYPE(); if ((opline->result_type & (IS_SMART_BRANCH_JMPZ|IS_SMART_BRANCH_JMPNZ)) != 0) { diff --git a/ext/opcache/tests/jit/cmp_007.phpt b/ext/opcache/tests/jit/cmp_007.phpt new file mode 100644 index 00000000000..17d1fec1f19 --- /dev/null +++ b/ext/opcache/tests/jit/cmp_007.phpt @@ -0,0 +1,23 @@ +--TEST-- +JIT CMP: 007 Wrong comparison skip +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.file_update_protection=0 +opcache.jit_buffer_size=1M +opcache.protect_memory=1 +--FILE-- + +--EXPECT-- +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) From 455837139e4edd5e57ec3009d400165f2795ab33 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 7 Oct 2021 14:37:26 +0200 Subject: [PATCH 2/2] Set opline before calling undef op helper --- ext/opcache/jit/zend_jit_x86.dasc | 1 + ext/opcache/tests/jit/isset_001.phpt | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 ext/opcache/tests/jit/isset_001.phpt diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index bf0759c31c2..858fe4f57e0 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -12402,6 +12402,7 @@ static int zend_jit_isset_isempty_dim(dasm_State **Dst, if (op2_info & MAY_BE_ANY) { | IF_NOT_ZVAL_TYPE op2_addr, IS_UNDEF, >1 } + | SET_EX_OPLINE opline, r0 | mov FCARG1d, opline->op2.var | EXT_CALL zend_jit_undefined_op_helper, r0 |1: diff --git a/ext/opcache/tests/jit/isset_001.phpt b/ext/opcache/tests/jit/isset_001.phpt new file mode 100644 index 00000000000..2ed9377d396 --- /dev/null +++ b/ext/opcache/tests/jit/isset_001.phpt @@ -0,0 +1,17 @@ +--TEST-- +ISSET_ISEMPTY_DIM with undefined variable +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.file_update_protection=0 +opcache.jit_buffer_size=1M +--FILE-- + +--EXPECTF-- +Warning: Undefined variable $undef in %s on line %d +bool(false)