Skip to content

Commit 03f8bcc

Browse files
committed
Fixed bug #80404
For a division like [1..1]/[2..2] produce [0..1] as a result, which would be the integer envelope of the floating-point result. The implementation is pretty ugly (we're now taking min/max across eight values...) but I couldn't come up with a more elegant way to handle this that doesn't make things a lot more complex (the division sign handling is the annoying issue here).
1 parent 912cb8b commit 03f8bcc

File tree

3 files changed

+44
-12
lines changed

3 files changed

+44
-12
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ PHP NEWS
88
. Fixed bug #72964 (White space not unfolded for CC/Bcc headers). (cmb)
99
. Fixed bug #80391 (Iterable not covariant to mixed). (Nikita)
1010

11+
- Opcache:
12+
. Fixed bug #80404 (Incorrect range inference result when division results
13+
in float). (Nikita)
14+
1115
- Tidy:
1216
. Fixed bug #77594 (ob_tidyhandler is never reset). (cmb)
1317

Zend/tests/bug80404.phpt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--TEST--
2+
Bug #80404: Incorrect range inference result when division results in float
3+
--FILE--
4+
<?php
5+
6+
$n = 63;
7+
var_dump((int) ($n / 120 * 100));
8+
9+
?>
10+
--EXPECT--
11+
int(52)

ext/opcache/Optimizer/zend_inference.c

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,19 @@ static inline zend_bool shift_left_overflows(zend_long n, zend_long s) {
548548
}
549549
}
550550

551+
/* If b does not divide a exactly, return the two adjacent values between which the real result
552+
* lies. */
553+
static void float_div(zend_long a, zend_long b, zend_long *r1, zend_long *r2) {
554+
*r1 = *r2 = a / b;
555+
if (a % b != 0) {
556+
if (*r2 < 0) {
557+
(*r2)--;
558+
} else {
559+
(*r2)++;
560+
}
561+
}
562+
}
563+
551564
static int zend_inference_calc_binary_op_range(
552565
const zend_op_array *op_array, zend_ssa *ssa,
553566
zend_op *opline, zend_ssa_op *ssa_op, zend_uchar opcode, zend_ssa_range *tmp) {
@@ -644,32 +657,36 @@ static int zend_inference_calc_binary_op_range(
644657
op1_max = OP1_MAX_RANGE();
645658
op2_max = OP2_MAX_RANGE();
646659
if (op2_min <= 0 && op2_max >= 0) {
660+
/* If op2 crosses zero, then floating point values close to zero might be
661+
* possible, which will result in arbitrarily large results. As such, we can't
662+
* do anything useful in that case. */
647663
break;
648664
}
649665
if (op1_min == ZEND_LONG_MIN && op2_max == -1) {
650666
/* Avoid ill-defined division, which may trigger SIGFPE. */
651667
break;
652668
}
653-
t1 = op1_min / op2_min;
654-
t2 = op1_min / op2_max;
655-
t3 = op1_max / op2_min;
656-
t4 = op1_max / op2_max;
657-
// FIXME: more careful overflow checks?
669+
670+
zend_long t1_, t2_, t3_, t4_;
671+
float_div(op1_min, op2_min, &t1, &t1_);
672+
float_div(op1_min, op2_max, &t2, &t2_);
673+
float_div(op1_max, op2_min, &t3, &t3_);
674+
float_div(op1_max, op2_max, &t4, &t4_);
675+
676+
/* The only case in which division can "overflow" either a division by an absolute
677+
* value smaller than one, or LONG_MIN / -1 in particular. Both cases have already
678+
* been excluded above. */
658679
if (OP1_RANGE_UNDERFLOW() ||
659680
OP2_RANGE_UNDERFLOW() ||
660681
OP1_RANGE_OVERFLOW() ||
661-
OP2_RANGE_OVERFLOW() ||
662-
t1 != (zend_long)((double)op1_min / (double)op2_min) ||
663-
t2 != (zend_long)((double)op1_min / (double)op2_max) ||
664-
t3 != (zend_long)((double)op1_max / (double)op2_min) ||
665-
t4 != (zend_long)((double)op1_max / (double)op2_max)) {
682+
OP2_RANGE_OVERFLOW()) {
666683
tmp->underflow = 1;
667684
tmp->overflow = 1;
668685
tmp->min = ZEND_LONG_MIN;
669686
tmp->max = ZEND_LONG_MAX;
670687
} else {
671-
tmp->min = MIN(MIN(t1, t2), MIN(t3, t4));
672-
tmp->max = MAX(MAX(t1, t2), MAX(t3, t4));
688+
tmp->min = MIN(MIN(MIN(t1, t2), MIN(t3, t4)), MIN(MIN(t1_, t2_), MIN(t3_, t4_)));
689+
tmp->max = MAX(MAX(MAX(t1, t2), MAX(t3, t4)), MAX(MAX(t1_, t2_), MAX(t3_, t4_)));
673690
}
674691
return 1;
675692
}

0 commit comments

Comments
 (0)