Skip to content

Commit 54324c9

Browse files
committed
Fix fallback paths in fast_long_{add,sub}_function
This was asked to be checked in php#17472 (comment) There are 3 issues: 1) The constant LONG_SIGN_MASK is wrong. Commit 98df5c9 changed this to avoid UB but did this incorrectly. The right fix was using zend_ulong. 2) The UB in the if can overflow, and can be fixed by using zend_ulong for the sum/sub. 3) fast_long_sub_function() has a problem when result aliases. This is fixed in the same way as fast_long_add_function() works.
1 parent f8b57ff commit 54324c9

File tree

1 file changed

+10
-4
lines changed

1 file changed

+10
-4
lines changed

Zend/zend_operators.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
#include "zend_multiply.h"
5050
#include "zend_object_handlers.h"
5151

52-
#define LONG_SIGN_MASK ZEND_LONG_MIN
52+
#define LONG_SIGN_MASK (((zend_ulong)1) << (8*sizeof(zend_long)-1))
5353

5454
BEGIN_EXTERN_C()
5555
ZEND_API zend_result ZEND_FASTCALL add_function(zval *result, zval *op1, zval *op2);
@@ -723,7 +723,7 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL
723723
*/
724724

725725
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))) {
726+
&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (((zend_ulong) Z_LVAL_P(op1) + (zend_ulong) Z_LVAL_P(op2)) & LONG_SIGN_MASK))) {
727727
ZVAL_DOUBLE(result, (double) Z_LVAL_P(op1) + (double) Z_LVAL_P(op2));
728728
} else {
729729
ZVAL_LONG(result, Z_LVAL_P(op1) + Z_LVAL_P(op2));
@@ -804,11 +804,17 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL
804804
ZVAL_LONG(result, llresult);
805805
}
806806
#else
807-
ZVAL_LONG(result, Z_LVAL_P(op1) - Z_LVAL_P(op2));
807+
/*
808+
* 'result' may alias with op1 or op2, so we need to
809+
* ensure that 'result' is not updated until after we
810+
* have read the values of op1 and op2.
811+
*/
808812

809813
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))) {
814+
&& (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (((zend_ulong) Z_LVAL_P(op1) - (zend_ulong) Z_LVAL_P(op2)) & LONG_SIGN_MASK))) {
811815
ZVAL_DOUBLE(result, (double) Z_LVAL_P(op1) - (double) Z_LVAL_P(op2));
816+
} else {
817+
ZVAL_LONG(result, Z_LVAL_P(op1) - Z_LVAL_P(op2));
812818
}
813819
#endif
814820
}

0 commit comments

Comments
 (0)