Skip to content

Commit 7e06a81

Browse files
committed
Fix fallback paths in fast_long_{add,sub}_function
This was asked to be checked in #17472 (comment) There are 2 issues: 1) The UB in the if can overflow, and can be fixed by using zend_ulong for the sum/sub. 2) fast_long_sub_function() has a problem when result aliases. This is fixed in the same way as fast_long_add_function() works. Closes GH-17666.
1 parent 0c3cf1f commit 7e06a81

File tree

2 files changed

+15
-4
lines changed

2 files changed

+15
-4
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ PHP NEWS
1010
compilation). (ilutov)
1111
. Fixed bug GH-17618 (UnhandledMatchError does not take
1212
zend.exception_ignore_args=1 into account). (timwolla)
13+
. Fix fallback paths in fast_long_{add,sub}_function. (nielsdos)
1314

1415
- Opcache:
1516
. Fixed bug GH-17654 (Multiple classes using same trait causes function

Zend/zend_operators.h

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -722,11 +722,13 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL
722722
* have read the values of op1 and op2.
723723
*/
724724

725+
zend_long sum = (zend_long) ((zend_ulong) Z_LVAL_P(op1) + (zend_ulong) Z_LVAL_P(op2));
726+
725727
if (UNEXPECTED((Z_LVAL_P(op1) & LONG_SIGN_MASK) == (Z_LVAL_P(op2) & LONG_SIGN_MASK)
726-
&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != ((Z_LVAL_P(op1) + Z_LVAL_P(op2)) & LONG_SIGN_MASK))) {
728+
&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (sum & LONG_SIGN_MASK))) {
727729
ZVAL_DOUBLE(result, (double) Z_LVAL_P(op1) + (double) Z_LVAL_P(op2));
728730
} else {
729-
ZVAL_LONG(result, Z_LVAL_P(op1) + Z_LVAL_P(op2));
731+
ZVAL_LONG(result, sum);
730732
}
731733
#endif
732734
}
@@ -804,11 +806,19 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL
804806
ZVAL_LONG(result, llresult);
805807
}
806808
#else
807-
ZVAL_LONG(result, Z_LVAL_P(op1) - Z_LVAL_P(op2));
809+
/*
810+
* 'result' may alias with op1 or op2, so we need to
811+
* ensure that 'result' is not updated until after we
812+
* have read the values of op1 and op2.
813+
*/
814+
815+
zend_long sub = (zend_long) ((zend_ulong) Z_LVAL_P(op1) - (zend_ulong) Z_LVAL_P(op2));
808816

809817
if (UNEXPECTED((Z_LVAL_P(op1) & LONG_SIGN_MASK) != (Z_LVAL_P(op2) & LONG_SIGN_MASK)
810-
&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (Z_LVAL_P(result) & LONG_SIGN_MASK))) {
818+
&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (sub & LONG_SIGN_MASK))) {
811819
ZVAL_DOUBLE(result, (double) Z_LVAL_P(op1) - (double) Z_LVAL_P(op2));
820+
} else {
821+
ZVAL_LONG(result, sub);
812822
}
813823
#endif
814824
}

0 commit comments

Comments
 (0)