diff --git a/NEWS b/NEWS index 9c5f7bede786..2a3ed5a38862 100644 --- a/NEWS +++ b/NEWS @@ -54,6 +54,7 @@ PHP NEWS . Fixed field_count not resetting on OK packet. (Kamil Tekiela) - Opcache: + . Fixed bug GH-24088 (Range inference optimizer bug). (Ilia Alshanetsky) . Fixed bug GH-23693 (Tracing JIT produces wrong results for a guard on a loop-invariant addition). (Ilia Alshanetsky) . Fixed OSS-Fuzz #545352966 (default value AST of an SHM-persisted partial). diff --git a/Zend/Optimizer/zend_inference.c b/Zend/Optimizer/zend_inference.c index 71c9247edf67..175cedb0f82f 100644 --- a/Zend/Optimizer/zend_inference.c +++ b/Zend/Optimizer/zend_inference.c @@ -1297,7 +1297,7 @@ ZEND_API bool zend_inference_propagate_range(const zend_op_array *op_array, cons } } else if (ssa_op->result_def == var) { if (opline->extended_value == IS_LONG) { - if (OP1_HAS_RANGE()) { + if (OP1_HAS_RANGE() && !OP1_RANGE_UNDERFLOW() && !OP1_RANGE_OVERFLOW()) { tmp->min = OP1_MIN_RANGE(); tmp->max = OP1_MAX_RANGE(); return 1; @@ -1562,7 +1562,9 @@ ZEND_API bool zend_inference_propagate_range(const zend_op_array *op_array, cons } if (call_info->callee_func->type == ZEND_USER_FUNCTION) { func_info = ZEND_FUNC_INFO(&call_info->callee_func->op_array); - if (func_info && func_info->return_info.has_range) { + if (func_info + && func_info->return_info.has_range + && !(func_info->return_info.type & (MAY_BE_ANY | MAY_BE_UNDEF | MAY_BE_REF) & ~MAY_BE_LONG)) { *tmp = func_info->return_info.range; return 1; } @@ -4887,16 +4889,217 @@ static void zend_mark_cv_references(const zend_op_array *op_array, const zend_sc free_alloca(worklist, use_heap); } +static bool zend_inference_is_long_only(const zend_ssa *ssa, int var) +{ + return !(ssa->var_info[var].type & (MAY_BE_ANY | MAY_BE_UNDEF | MAY_BE_REF) & ~MAY_BE_LONG); +} + +static bool zend_drop_non_long_pi_ranges(zend_ssa *ssa) +{ + bool dropped = false; + + for (int i = 0; i < ssa->vars_count; i++) { + zend_ssa_phi *p = ssa->vars[i].definition_phi; + zend_ssa_range_constraint *constraint; + + if (!p || p->pi < 0 || !p->has_range_constraint || !ssa->var_info[i].has_range) { + continue; + } + constraint = &p->constraint.range; + if (constraint->min_ssa_var < 0 + && constraint->max_ssa_var < 0 + && constraint->range.underflow + && constraint->range.overflow + && constraint->negative == NEG_NONE) { + continue; + } + if (zend_inference_is_long_only(ssa, p->sources[0]) + && (constraint->min_ssa_var < 0 || zend_inference_is_long_only(ssa, constraint->min_ssa_var)) + && (constraint->max_ssa_var < 0 || zend_inference_is_long_only(ssa, constraint->max_ssa_var))) { + continue; + } + constraint->min_var = -1; + constraint->max_var = -1; + constraint->min_ssa_var = -1; + constraint->max_ssa_var = -1; + constraint->range.underflow = true; + constraint->range.min = ZEND_LONG_MIN; + constraint->range.max = ZEND_LONG_MAX; + constraint->range.overflow = true; + constraint->negative = NEG_NONE; + dropped = true; + } + return dropped; +} + +static bool zend_range_may_coerce(const zend_ssa *ssa, const zend_bitset tainted, int var, uint32_t mask) +{ + const zend_ssa_var_info *info; + + if (var < 0 || !zend_bitset_in(tainted, var)) { + return false; + } + info = &ssa->var_info[var]; + return info->has_range + && (info->type & mask) + && ((!info->range.underflow && info->range.min != ZEND_LONG_MIN) + || (!info->range.overflow && info->range.max != ZEND_LONG_MAX)); +} + +static bool zend_range_is_flagged(const zend_ssa *ssa, int var) +{ + return var >= 0 + && ssa->var_info[var].has_range + && (ssa->var_info[var].range.underflow || ssa->var_info[var].range.overflow); +} + +static bool zend_pi_narrows_min(const zend_ssa *ssa, const zend_ssa_phi *p) +{ + const zend_ssa_range *range = &ssa->var_info[p->ssa_var].range; + const zend_ssa_var_info *src = &ssa->var_info[p->sources[0]]; + + return !range->underflow && (!src->has_range || src->range.underflow || range->min > src->range.min); +} + +static bool zend_pi_narrows_max(const zend_ssa *ssa, const zend_ssa_phi *p) +{ + const zend_ssa_range *range = &ssa->var_info[p->ssa_var].range; + const zend_ssa_var_info *src = &ssa->var_info[p->sources[0]]; + + return !range->overflow && (!src->has_range || src->range.overflow || range->max < src->range.max); +} + +static bool zend_ssa_range_coercion_unsafe(const zend_op_array *op_array, const zend_ssa *ssa) +{ + const uint32_t non_long = (MAY_BE_ANY | MAY_BE_UNDEF | MAY_BE_REF) & ~MAY_BE_LONG; + const uint32_t non_number = non_long & ~MAY_BE_DOUBLE; + int worklist_len = zend_bitset_len(ssa->vars_count); + bool tainted_any = false, unsafe = false; + zend_bitset tainted, worklist; + int j; + ALLOCA_FLAG(use_heap); + + tainted = do_alloca(sizeof(zend_ulong) * worklist_len * 2, use_heap); + worklist = tainted + worklist_len; + memset(tainted, 0, sizeof(zend_ulong) * worklist_len * 2); + + for (j = 0; j < ssa->vars_count; j++) { + const zend_ssa_phi *p = ssa->vars[j].definition_phi; + + if (p && p->pi >= 0 && p->has_range_constraint && ssa->var_info[j].has_range + && (ssa->var_info[p->sources[0]].type & non_long) + && (zend_pi_narrows_min(ssa, p) || zend_pi_narrows_max(ssa, p))) { + zend_bitset_incl(tainted, j); + zend_bitset_incl(worklist, j); + tainted_any = true; + } + } + +#define TAINT_VAR(_var) do { \ + if (!zend_bitset_in(tainted, _var)) { \ + zend_bitset_incl(tainted, _var); \ + zend_bitset_incl(worklist, _var); \ + } \ + } while (0) + + WHILE_WORKLIST(worklist, worklist_len, j) { + FOR_EACH_VAR_USAGE(j, TAINT_VAR); + } WHILE_WORKLIST_END(); + +#undef TAINT_VAR + + for (uint32_t i = 0; tainted_any && i < op_array->last; i++) { + const zend_op *opline = &op_array->opcodes[i]; + const zend_ssa_op *ssa_op = &ssa->ops[i]; + uint8_t opcode = opline->opcode == ZEND_ASSIGN_OP ? opline->extended_value : opline->opcode; + uint32_t mask = non_long; + + switch (opcode) { + case ZEND_ADD: + case ZEND_SUB: + case ZEND_PRE_INC: + case ZEND_PRE_DEC: + case ZEND_POST_INC: + case ZEND_POST_DEC: + unsafe = zend_range_may_coerce(ssa, tainted, ssa_op->op1_use, non_number) + || zend_range_may_coerce(ssa, tainted, ssa_op->op2_use, non_number); + break; + case ZEND_CAST: + if (opline->extended_value != IS_LONG) { + break; + } + ZEND_FALLTHROUGH; + case ZEND_MUL: + case ZEND_DIV: + case ZEND_MOD: + case ZEND_SL: + case ZEND_SR: + case ZEND_BW_OR: + case ZEND_BW_AND: + case ZEND_BW_NOT: + if (opcode == ZEND_MUL || opcode == ZEND_DIV) { + mask = non_number; + } + unsafe = ((opcode == ZEND_DIV || opcode == ZEND_MOD) + && zend_range_may_coerce(ssa, tainted, ssa_op->op2_use, non_number)) + || (!zend_range_is_flagged(ssa, ssa_op->op1_use) + && !zend_range_is_flagged(ssa, ssa_op->op2_use) + && (zend_range_may_coerce(ssa, tainted, ssa_op->op1_use, mask) + || zend_range_may_coerce(ssa, tainted, ssa_op->op2_use, mask))); + break; + } + if (unsafe) { + break; + } + } + + for (j = 0; !unsafe && j < ssa->vars_count; j++) { + const zend_ssa_phi *p = ssa->vars[j].definition_phi; + const zend_ssa_range_constraint *constraint; + int bound; + + if (!p || p->pi < 0 || !p->has_range_constraint || !ssa->var_info[j].has_range) { + continue; + } + constraint = &p->constraint.range; + bound = constraint->min_ssa_var; + if (bound >= 0 + && ((ssa->var_info[bound].type & non_number) + || ((ssa->var_info[bound].type & MAY_BE_DOUBLE) + && (zend_bitset_in(tainted, bound) || ssa->var_info[bound].range.underflow))) + && zend_pi_narrows_min(ssa, p)) { + unsafe = true; + } + bound = constraint->max_ssa_var; + if (bound >= 0 + && ((ssa->var_info[bound].type & non_number) + || ((ssa->var_info[bound].type & MAY_BE_DOUBLE) + && (zend_bitset_in(tainted, bound) || ssa->var_info[bound].range.overflow))) + && zend_pi_narrows_max(ssa, p)) { + unsafe = true; + } + } + + free_alloca(tainted, use_heap); + return unsafe; +} + ZEND_API zend_result zend_ssa_inference(zend_arena **arena, const zend_op_array *op_array, const zend_script *script, zend_ssa *ssa, zend_long optimization_level) /* {{{ */ { zend_ssa_var_info *ssa_var_info; + zend_func_info *func_info = ZEND_FUNC_INFO(op_array); + zend_ssa_var_info return_info; int i; if (!ssa->var_info) { ssa->var_info = zend_arena_calloc(arena, ssa->vars_count, sizeof(zend_ssa_var_info)); } ssa_var_info = ssa->var_info; + if (func_info) { + return_info = func_info->return_info; + } +restart: if (!op_array->function_name) { for (i = 0; i < op_array->last_var; i++) { ssa_var_info[i].type = MAY_BE_UNDEF | MAY_BE_RC1 | MAY_BE_RCN | MAY_BE_REF | MAY_BE_ANY | MAY_BE_ARRAY_KEY_ANY | MAY_BE_ARRAY_OF_ANY | MAY_BE_ARRAY_OF_REF; @@ -4924,6 +5127,14 @@ ZEND_API zend_result zend_ssa_inference(zend_arena **arena, const zend_op_array return FAILURE; } + if (zend_ssa_range_coercion_unsafe(op_array, ssa) && zend_drop_non_long_pi_ranges(ssa)) { + memset(ssa_var_info, 0, sizeof(zend_ssa_var_info) * ssa->vars_count); + if (func_info) { + func_info->return_info = return_info; + } + goto restart; + } + return SUCCESS; } /* }}} */ diff --git a/ext/opcache/tests/opt/gh24088.phpt b/ext/opcache/tests/opt/gh24088.phpt new file mode 100644 index 000000000000..5c44a99b2eae --- /dev/null +++ b/ext/opcache/tests/opt/gh24088.phpt @@ -0,0 +1,50 @@ +--TEST-- +GH-24088 (Integer casts must not reuse ranges inferred for other types) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=-1 +--FILE-- + 5 && $value < 7) { + return match ((int) $value) { + 6 => 'six', + default => 'not six', + }; + } +} + +function equal($value) { + if ($value == 6) { + return (int) $value; + } +} + +function copied($value, $copy) { + if ($value == 6) { + if ($copy) { + $result = $value; + } else { + $result = 6; + } + return (int) $result; + } +} + +echo 'float: ', bounded(5.5), "\n"; +echo 'numeric string: ', bounded('5.5'), "\n"; +echo 'integer: ', bounded(6), "\n"; +echo 'boolean: ', equal(true), "\n"; +echo 'copied boolean: ', copied(true, true), "\n"; +echo 'joined integer: ', copied(true, false), "\n"; +?> +--EXPECT-- +float: not six +numeric string: not six +integer: six +boolean: 1 +copied boolean: 1 +joined integer: 6 diff --git a/ext/opcache/tests/opt/gh24088_002.phpt b/ext/opcache/tests/opt/gh24088_002.phpt new file mode 100644 index 000000000000..548497c14084 --- /dev/null +++ b/ext/opcache/tests/opt/gh24088_002.phpt @@ -0,0 +1,95 @@ +--TEST-- +GH-24088 (Range propagation must account for implicit integer conversions) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=-1 +error_reporting=E_ALL & ~E_DEPRECATED & ~E_WARNING +--FILE-- + $value % 100, + 'left shift' => $value << 0, + 'right shift' => $value >> 0, + 'bitwise or' => $value | 0, + 'bitwise and' => $value & 255, + 'addition' => (int) ($value + 0), + 'subtraction' => (int) ($value - 0), + 'multiplication' => (int) ($value * 1), + 'division' => (int) ($value / 1), + ]; + } +} + +function bitwiseNot($value) { + if ($value > 5 && $value < 7) { + $result = ~$value; + if (is_int($result)) { + return $result; + } + } +} + +function assignment($value) { + if ($value == 6) { + $value %= 100; + return $value; + } +} + +function increment($value) { + if ($value == 6) { + return (int) ++$value; + } +} + +function decrement($value) { + if ($value == 6) { + return (int) --$value; + } +} + +function postIncrement($value) { + if ($value == 6) { + $result = $value++; + return [(int) $result, (int) $value]; + } +} + +function postDecrement($value) { + if ($value == 6) { + $result = $value--; + return [(int) $result, (int) $value]; + } +} + +foreach (operations(true) as $operation => $result) { + echo $operation, ': ', $result, "\n"; +} +echo 'bitwise not: ', bitwiseNot(5.5), "\n"; +echo 'assignment: ', assignment(true), "\n"; +echo 'pre-increment: ', increment(true), "\n"; +echo 'pre-decrement: ', decrement(true), "\n"; +echo 'post-increment: ', implode(', ', postIncrement(true)), "\n"; +echo 'post-decrement: ', implode(', ', postDecrement(true)), "\n"; +?> +--EXPECT-- +modulo: 1 +left shift: 1 +right shift: 1 +bitwise or: 1 +bitwise and: 1 +addition: 1 +subtraction: 1 +multiplication: 1 +division: 1 +bitwise not: -6 +assignment: 1 +pre-increment: 1 +pre-decrement: 1 +post-increment: 1, 1 +post-decrement: 1, 1 diff --git a/ext/opcache/tests/opt/gh24088_003.phpt b/ext/opcache/tests/opt/gh24088_003.phpt new file mode 100644 index 000000000000..31e4d1c4b5e5 --- /dev/null +++ b/ext/opcache/tests/opt/gh24088_003.phpt @@ -0,0 +1,42 @@ +--TEST-- +GH-24088 (Symbolic ranges must account for the compared operand type) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=-1 +--FILE-- + 9 ? 'large' : 'medium'); + } +} + +function loop() { + $sum = 0; + for ($i = 0; $i < 10; $i++) { + $sum += $i; + } + return $sum; +} + +echo 'symbolic boolean: ', symbolic(true, 42), "\n"; +echo 'symbolic integer: ', symbolic(6, 6), "\n"; +echo 'symbolic lower bound: ', symbolicBounds(true, 1), "\n"; +echo 'symbolic upper bound: ', symbolicBounds(true, 42), "\n"; +echo 'integer loop: ', loop(), "\n"; +?> +--EXPECT-- +symbolic boolean: 42 +symbolic integer: 6 +symbolic lower bound: small +symbolic upper bound: large +integer loop: 45 diff --git a/ext/opcache/tests/opt/gh24088_005.phpt b/ext/opcache/tests/opt/gh24088_005.phpt new file mode 100644 index 000000000000..588285fab409 --- /dev/null +++ b/ext/opcache/tests/opt/gh24088_005.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-24088 (Class unions must not be inferred as numeric-only types) +--EXTENSIONS-- +opcache +zend_test +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=-1 +error_reporting=E_ALL & ~E_NOTICE +--FILE-- + +--EXPECT-- +parameter union: 6 +return union: 6 diff --git a/ext/opcache/tests/opt/gh24088_006.phpt b/ext/opcache/tests/opt/gh24088_006.phpt new file mode 100644 index 000000000000..222c32394dd1 --- /dev/null +++ b/ext/opcache/tests/opt/gh24088_006.phpt @@ -0,0 +1,58 @@ +--TEST-- +GH-24088 (Consumers outside range propagation must not trust ranges inferred for other types) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=-1 +--FILE-- += 6 && $value <= 6) { + return $value; + } + return 6; +} + +function callerModulo($value) { + return constrained($value) % 1000; +} + +function nullBound(int $integer, ?int $bound) { + if ($integer > $bound) { + return $integer - 1; + } + return 0; +} + +foreach (['divide', 'modulo'] as $function) { + try { + $result = $function(false); + echo $function, ': ', $result, "\n"; + } catch (DivisionByZeroError $e) { + echo $function, ': ', $e::class, ': ', $e->getMessage(), "\n"; + } +} +echo 'call result: ', callerModulo(true), "\n"; +echo 'null bound: ', var_export(nullBound(PHP_INT_MIN, null), true), "\n"; +?> +--EXPECT-- +divide: DivisionByZeroError: Division by zero +modulo: DivisionByZeroError: Modulo by zero +call result: 1 +null bound: -9.223372036854776E+18