From c0149f29be0f2096b272596d5018f5ad706db636 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Thu, 8 Oct 2026 18:26:53 -0400 Subject: [PATCH] ext/opcache: Fix GH-24088 integer range propagation Range inference runs before type inference, so a Pi constraint such as $v > 5 && $v < 7 narrows the range of a value that may be a float, bool or string. That range only describes the integer values, but casts, arithmetic and bitwise operations on other types, symbolic bounds, call results and division by zero checks read it as a bound on the converted value. After inference, check those consumers against the final types, and only when one reads a range derived from such a constraint, drop the offending constraints and rerun range and type inference. Fixes GH-24088 --- NEWS | 1 + Zend/Optimizer/zend_inference.c | 215 ++++++++++++++++++++++++- ext/opcache/tests/opt/gh24088.phpt | 50 ++++++ ext/opcache/tests/opt/gh24088_002.phpt | 95 +++++++++++ ext/opcache/tests/opt/gh24088_003.phpt | 42 +++++ ext/opcache/tests/opt/gh24088_005.phpt | 41 +++++ ext/opcache/tests/opt/gh24088_006.phpt | 58 +++++++ 7 files changed, 500 insertions(+), 2 deletions(-) create mode 100644 ext/opcache/tests/opt/gh24088.phpt create mode 100644 ext/opcache/tests/opt/gh24088_002.phpt create mode 100644 ext/opcache/tests/opt/gh24088_003.phpt create mode 100644 ext/opcache/tests/opt/gh24088_005.phpt create mode 100644 ext/opcache/tests/opt/gh24088_006.phpt 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