Skip to content

Check whether expected types are present for compound op jit #7481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 36 additions & 38 deletions ext/opcache/jit/zend_jit.c
Original file line number Diff line number Diff line change
Expand Up @@ -2647,6 +2647,31 @@ static bool zend_jit_next_is_send_result(const zend_op *opline)
return 0;
}

static bool zend_can_jit_compound_binary_op(zend_uchar op, uint32_t op1_info, uint32_t op2_info)
Copy link
Member

Choose a reason for hiding this comment

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

The name is really bad. "compound" should be removed. "can" should be replaced to some better word.
e.g. zend_jit_profitable_binary_op() or zend_jit_supported_binary_op() or something better.

{
switch (op) {
case ZEND_POW:
case ZEND_DIV:
// TODO: check for division by zero ???
return false;
case ZEND_ADD:
case ZEND_SUB:
case ZEND_MUL:
return (op1_info & (MAY_BE_LONG|MAY_BE_DOUBLE))
&& (op2_info & (MAY_BE_LONG|MAY_BE_DOUBLE));
case ZEND_BW_OR:
case ZEND_BW_AND:
case ZEND_BW_XOR:
case ZEND_SL:
case ZEND_SR:
case ZEND_MOD:
return (op1_info & MAY_BE_LONG) && (op2_info & MAY_BE_LONG);
case ZEND_CONCAT:
return (op1_info & MAY_BE_STRING) && (op2_info & MAY_BE_STRING);
EMPTY_SWITCH_DEFAULT_CASE()
}
}

static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op *rt_opline)
{
int b, i, end;
Expand Down Expand Up @@ -3058,11 +3083,6 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
goto done;
case ZEND_ASSIGN_OP:
if (opline->extended_value == ZEND_POW
|| opline->extended_value == ZEND_DIV) {
// TODO: check for division by zero ???
break;
}
if (opline->op1_type != IS_CV || opline->result_type != IS_UNUSED) {
break;
}
Expand All @@ -3074,29 +3094,9 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
if ((op1_info & MAY_BE_UNDEF) || (op2_info & MAY_BE_UNDEF)) {
break;
}
if (opline->extended_value == ZEND_ADD
|| opline->extended_value == ZEND_SUB
|| opline->extended_value == ZEND_MUL
|| opline->extended_value == ZEND_DIV) {
if (!(op1_info & (MAY_BE_LONG|MAY_BE_DOUBLE))
|| !(op2_info & (MAY_BE_LONG|MAY_BE_DOUBLE))) {
break;
}
} else if (opline->extended_value == ZEND_BW_OR
|| opline->extended_value == ZEND_BW_AND
|| opline->extended_value == ZEND_BW_XOR
|| opline->extended_value == ZEND_SL
|| opline->extended_value == ZEND_SR
|| opline->extended_value == ZEND_MOD) {
if (!(op1_info & MAY_BE_LONG)
|| !(op2_info & MAY_BE_LONG)) {
break;
}
} else if (opline->extended_value == ZEND_CONCAT) {
if (!(op1_info & MAY_BE_STRING)
|| !(op2_info & MAY_BE_STRING)) {
break;
}
if (!zend_can_jit_compound_binary_op(
opline->extended_value, op1_info, op2_info)) {
break;
}
op1_def_info = OP1_DEF_INFO();
if (!zend_jit_assign_op(&dasm_state, opline,
Expand All @@ -3108,17 +3108,16 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
goto done;
case ZEND_ASSIGN_DIM_OP:
if (opline->extended_value == ZEND_POW
|| opline->extended_value == ZEND_DIV) {
// TODO: check for division by zero ???
break;
}
if (opline->op1_type != IS_CV || opline->result_type != IS_UNUSED) {
break;
}
if (PROFITABILITY_CHECKS && (!ssa->ops || !ssa->var_info)) {
break;
}
if (!zend_can_jit_compound_binary_op(
opline->extended_value, MAY_BE_ANY, OP1_DATA_INFO())) {
break;
}
if (!zend_jit_assign_dim_op(&dasm_state, opline,
OP1_INFO(), OP1_DEF_INFO(), OP1_REG_ADDR(), OP2_INFO(),
OP1_DATA_INFO(), OP1_DATA_RANGE(),
Expand Down Expand Up @@ -3183,11 +3182,6 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
goto done;
case ZEND_ASSIGN_OBJ_OP:
if (opline->extended_value == ZEND_POW
|| opline->extended_value == ZEND_DIV) {
// TODO: check for division by zero ???
break;
}
if (opline->result_type != IS_UNUSED) {
break;
}
Expand All @@ -3199,6 +3193,10 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
if (PROFITABILITY_CHECKS && (!ssa->ops || !ssa->var_info)) {
break;
}
if (!zend_can_jit_compound_binary_op(
opline->extended_value, MAY_BE_ANY, OP1_DATA_INFO())) {
break;
}
ce = NULL;
ce_is_instanceof = 0;
if (opline->op1_type == IS_UNUSED) {
Expand Down
44 changes: 11 additions & 33 deletions ext/opcache/jit/zend_jit_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -4316,11 +4316,6 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
}
goto done;
case ZEND_ASSIGN_OP:
if (opline->extended_value == ZEND_POW
|| opline->extended_value == ZEND_DIV) {
// TODO: check for division by zero ???
break;
}
if (opline->op1_type != IS_CV || opline->result_type != IS_UNUSED) {
break;
}
Expand All @@ -4331,29 +4326,9 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
if ((op1_info & MAY_BE_UNDEF) || (op2_info & MAY_BE_UNDEF)) {
break;
}
if (opline->extended_value == ZEND_ADD
|| opline->extended_value == ZEND_SUB
|| opline->extended_value == ZEND_MUL
|| opline->extended_value == ZEND_DIV) {
if (!(op1_info & (MAY_BE_LONG|MAY_BE_DOUBLE))
|| !(op2_info & (MAY_BE_LONG|MAY_BE_DOUBLE))) {
break;
}
} else if (opline->extended_value == ZEND_BW_OR
|| opline->extended_value == ZEND_BW_AND
|| opline->extended_value == ZEND_BW_XOR
|| opline->extended_value == ZEND_SL
|| opline->extended_value == ZEND_SR
|| opline->extended_value == ZEND_MOD) {
if (!(op1_info & MAY_BE_LONG)
|| !(op2_info & MAY_BE_LONG)) {
break;
}
} else if (opline->extended_value == ZEND_CONCAT) {
if (!(op1_info & MAY_BE_STRING)
|| !(op2_info & MAY_BE_STRING)) {
break;
}
if (!zend_can_jit_compound_binary_op(
opline->extended_value, op1_info, op2_info)) {
break;
}
op1_def_info = OP1_DEF_INFO();
if (op1_def_info & MAY_BE_GUARD
Expand Down Expand Up @@ -4383,6 +4358,10 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
if (opline->result_type != IS_UNUSED) {
break;
}
if (!zend_can_jit_compound_binary_op(
opline->extended_value, MAY_BE_ANY, OP1_DATA_INFO())) {
break;
}
op1_info = OP1_INFO();
op1_addr = OP1_REG_ADDR();
if (opline->op1_type == IS_VAR) {
Expand Down Expand Up @@ -4489,11 +4468,6 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
}
goto done;
case ZEND_ASSIGN_OBJ_OP:
if (opline->extended_value == ZEND_POW
|| opline->extended_value == ZEND_DIV) {
// TODO: check for division by zero ???
break;
}
if (opline->result_type != IS_UNUSED) {
break;
}
Expand All @@ -4502,6 +4476,10 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par
|| Z_STRVAL_P(RT_CONSTANT(opline, opline->op2))[0] == '\0') {
break;
}
if (!zend_can_jit_compound_binary_op(
opline->extended_value, MAY_BE_ANY, OP1_DATA_INFO())) {
break;
}
ce = NULL;
ce_is_instanceof = 0;
delayed_fetch_this = 0;
Expand Down
9 changes: 9 additions & 0 deletions ext/opcache/tests/jit/assign_dim_op_001.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ function false_to_array_nested_invalid_index($a) {
$a[[]][0] += 1;
return $a;
}
function modulo_string($a) {
$a[] %= "";
}

false_to_array(false);
false_to_array_append(false);
Expand All @@ -56,6 +59,11 @@ try {
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
try {
var_dump(modulo_string([]));
} catch (TypeError $e) {
echo $e->getMessage(), "\n";
}

?>
--EXPECTF--
Expand Down Expand Up @@ -97,3 +105,4 @@ array(1) {

Deprecated: Automatic conversion of false to array is deprecated in %s on line %d
Illegal offset type
Unsupported operand types: null % string