Skip to content

Commit

Permalink
JIT: Fix register clobbering
Browse files Browse the repository at this point in the history 
Fixes oss-fuzz #42657
  • Loading branch information
@dstogov
dstogov committed Dec 20, 2021
1 parent f18bb24 commit 7c674e1
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 21 deletions.
63 changes: 42 additions & 21 deletions ext/opcache/jit/zend_jit_x86.dasc
View file
Expand Up @@ -772,17 +772,12 @@ static void* dasm_labels[zend_lb_MAX];
|| }
|.endmacro

|.macro LONG_OP, long_ins, reg, addr
|.macro LONG_OP, long_ins, reg, addr, tmp_reg
|| if (Z_MODE(addr) == IS_CONST_ZVAL) {
| .if X64
|| if (!IS_SIGNED_32BIT(Z_LVAL_P(Z_ZV(addr)))) {
|| if (reg != ZREG_R0) {
| mov64 r0, Z_LVAL_P(Z_ZV(addr))
| long_ins Ra(reg), r0
|| } else {
| mov64 r1, Z_LVAL_P(Z_ZV(addr))
| long_ins Ra(reg), r1
|| }
| mov64 tmp_reg, Z_LVAL_P(Z_ZV(addr))
| long_ins Ra(reg), tmp_reg
|| } else {
| long_ins Ra(reg), Z_LVAL_P(Z_ZV(addr))
|| }
Expand Down Expand Up @@ -862,25 +857,25 @@ static void* dasm_labels[zend_lb_MAX];
|| }
|.endmacro

|.macro LONG_MATH, opcode, reg, addr
|.macro LONG_MATH, opcode, reg, addr, tmp_reg
|| switch (opcode) {
|| case ZEND_ADD:
| LONG_OP add, reg, addr
| LONG_OP add, reg, addr, Ra(tmp_reg)
|| break;
|| case ZEND_SUB:
| LONG_OP sub, reg, addr
| LONG_OP sub, reg, addr, Ra(tmp_reg)
|| break;
|| case ZEND_MUL:
| LONG_OP imul, reg, addr
| LONG_OP imul, reg, addr, Ra(tmp_reg)
|| break;
|| case ZEND_BW_OR:
| LONG_OP or, reg, addr
| LONG_OP or, reg, addr, Ra(tmp_reg)
|| break;
|| case ZEND_BW_AND:
| LONG_OP and, reg, addr
| LONG_OP and, reg, addr, Ra(tmp_reg)
|| break;
|| case ZEND_BW_XOR:
| LONG_OP xor, reg, addr
| LONG_OP xor, reg, addr, Ra(tmp_reg)
|| break;
|| default:
|| ZEND_UNREACHABLE();
Expand Down Expand Up @@ -4390,7 +4385,16 @@ static int zend_jit_math_long_long(dasm_State **Dst,
} else if (same_ops && opcode != ZEND_DIV) {
| LONG_MATH_REG opcode, Ra(result_reg), Ra(result_reg)
} else {
| LONG_MATH opcode, result_reg, op2_addr
zend_reg tmp_reg;

if (Z_MODE(res_addr) == IS_MEM_ZVAL && Z_REG(res_addr) == ZREG_R0) {
tmp_reg = ZREG_R1;
} else if (result_reg != ZREG_R0) {
tmp_reg = ZREG_R0;
} else {
tmp_reg = ZREG_R1;
}
| LONG_MATH opcode, result_reg, op2_addr, tmp_reg
}
}
if (may_overflow) {
Expand Down Expand Up @@ -5117,12 +5121,20 @@ static int zend_jit_long_math_helper(dasm_State **Dst,
} else if (zend_long_is_power_of_two(op2_lval) && op1_range && op1_range->min >= 0) {
zval tmp;
zend_jit_addr tmp_addr;
zend_reg tmp_reg;

/* Optimisation for mod of power of 2 */
ZVAL_LONG(&tmp, op2_lval - 1);
tmp_addr = ZEND_ADDR_CONST_ZVAL(&tmp);
if (Z_MODE(res_addr) == IS_MEM_ZVAL && Z_REG(res_addr) == ZREG_R0) {
tmp_reg = ZREG_R1;
} else if (result_reg != ZREG_R0) {
tmp_reg = ZREG_R0;
} else {
tmp_reg = ZREG_R1;
}
| GET_ZVAL_LVAL result_reg, op1_addr
| LONG_MATH ZEND_BW_AND, result_reg, tmp_addr
| LONG_MATH ZEND_BW_AND, result_reg, tmp_addr, tmp_reg
} else {
if (Z_MODE(res_addr) == IS_MEM_ZVAL && Z_REG(res_addr) == ZREG_RAX) {
| mov aword T1, r0 // save
Expand Down Expand Up @@ -5210,8 +5222,17 @@ static int zend_jit_long_math_helper(dasm_State **Dst,
| GET_ZVAL_LVAL result_reg, op1_addr
| LONG_MATH_REG opcode, Ra(result_reg), Ra(result_reg)
} else {
zend_reg tmp_reg;

if (Z_MODE(res_addr) == IS_MEM_ZVAL && Z_REG(res_addr) == ZREG_R0) {
tmp_reg = ZREG_R1;
} else if (result_reg != ZREG_R0) {
tmp_reg = ZREG_R0;
} else {
tmp_reg = ZREG_R1;
}
| GET_ZVAL_LVAL result_reg, op1_addr
| LONG_MATH opcode, result_reg, op2_addr
| LONG_MATH opcode, result_reg, op2_addr, tmp_reg
}

if (Z_MODE(res_addr) != IS_REG || Z_REG(res_addr) != result_reg) {
Expand Down Expand Up @@ -7025,13 +7046,13 @@ static int zend_jit_cmp_long_long(dasm_State **Dst,
if (Z_MODE(op2_addr) == IS_CONST_ZVAL && Z_LVAL_P(Z_ZV(op2_addr)) == 0) {
| test Ra(Z_REG(op1_addr)), Ra(Z_REG(op1_addr))
} else {
| LONG_OP cmp, Z_REG(op1_addr), op2_addr
| LONG_OP cmp, Z_REG(op1_addr), op2_addr, r0
}
} else if (Z_MODE(op2_addr) == IS_REG) {
if (Z_MODE(op1_addr) == IS_CONST_ZVAL && Z_LVAL_P(Z_ZV(op1_addr)) == 0) {
| test Ra(Z_REG(op2_addr)), Ra(Z_REG(op2_addr))
} else {
| LONG_OP cmp, Z_REG(op2_addr), op1_addr
| LONG_OP cmp, Z_REG(op2_addr), op1_addr, r0
}
swap = 1;
} else if (Z_MODE(op1_addr) == IS_CONST_ZVAL && Z_MODE(op2_addr) != IS_CONST_ZVAL) {
Expand All @@ -7044,7 +7065,7 @@ static int zend_jit_cmp_long_long(dasm_State **Dst,
if (Z_MODE(op2_addr) == IS_CONST_ZVAL && Z_LVAL_P(Z_ZV(op2_addr)) == 0) {
| test r0, r0
} else {
| LONG_OP cmp, ZREG_R0, op2_addr
| LONG_OP cmp, ZREG_R0, op2_addr, r0
}
}

Expand Down
19 changes: 19 additions & 0 deletions ext/opcache/tests/jit/add_012.phpt
View file
@@ -0,0 +1,19 @@
--TEST--
JIT ADD: 012 register allocation for 64-bit constant
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.file_update_protection=0
opcache.jit_buffer_size=1M
--SKIPIF--
<?php if (PHP_INT_SIZE != 8) die("skip: 64-bit only"); ?>
--FILE--
<?php
$x = 0;
$y = [0];
$y[$x]++;
$y[$x] += 4467793343;
?>
DONE
--EXPECT--
DONE

3 comments on commit 7c674e1

@Girgias
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to have broken the 32bit build, could you have a look @dstogov ?

@dstogov
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix this soon

@dstogov
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this should be fixed now.

Please sign in to comment.