From 52ebdfbe00212e69f0d8f6d9017b805adea2417c Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Tue, 17 Dec 2024 00:17:23 +0900 Subject: [PATCH] Correctly round rounding mode with zero edge case (#17065) Fixes #17064 Closes #17065 --- NEWS | 2 + ext/bcmath/libbcmath/src/round.c | 48 +++++++++++++++++++- ext/bcmath/tests/bcround_away_from_zero.phpt | 3 ++ ext/bcmath/tests/bcround_ceiling.phpt | 3 ++ ext/bcmath/tests/bcround_early_return.phpt | 8 ---- ext/bcmath/tests/bcround_floor.phpt | 3 ++ ext/bcmath/tests/bcround_half_down.phpt | 3 ++ ext/bcmath/tests/bcround_half_even.phpt | 3 ++ ext/bcmath/tests/bcround_half_odd.phpt | 3 ++ ext/bcmath/tests/bcround_half_up.phpt | 3 ++ ext/bcmath/tests/bcround_test_helper.inc | 3 ++ ext/bcmath/tests/bcround_toward_zero.phpt | 3 ++ ext/bcmath/tests/number/methods/round.phpt | 14 ++++-- 13 files changed, 84 insertions(+), 15 deletions(-) diff --git a/NEWS b/NEWS index b19642cd409a6..6f9d8da5f57a7 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ PHP NEWS . Fixed bug GH-17049 (Correctly compare 0 and -0). (Saki Takamachi) . Fixed bug GH-17061 (Now Number::round() does not remove trailing zeros). (Saki Takamachi) + . Fixed bug GH-17064 (Correctly round rounding mode with zero edge case). + (Saki Takamachi) - Core: . Fixed bug OSS-Fuzz #382922236 (Duplicate dynamic properties in hooked object diff --git a/ext/bcmath/libbcmath/src/round.c b/ext/bcmath/libbcmath/src/round.c index 3a4892cc7079e..ac3c7c41a315e 100644 --- a/ext/bcmath/libbcmath/src/round.c +++ b/ext/bcmath/libbcmath/src/round.c @@ -33,10 +33,54 @@ void bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) * - If the fractional part ends with zeros, the zeros are omitted and the number of digits in num is reduced. * Meaning we might end up in the previous case. */ + + /* e.g. value is 0.1 and precision is -3, ret is 0 or 1000 */ if (precision < 0 && num->n_len < (size_t) (-(precision + Z_L(1))) + 1) { - *result = bc_copy_num(BCG(_zero_)); + switch (mode) { + case PHP_ROUND_HALF_UP: + case PHP_ROUND_HALF_DOWN: + case PHP_ROUND_HALF_EVEN: + case PHP_ROUND_HALF_ODD: + case PHP_ROUND_TOWARD_ZERO: + *result = bc_copy_num(BCG(_zero_)); + return; + + case PHP_ROUND_CEILING: + if (num->n_sign == MINUS) { + *result = bc_copy_num(BCG(_zero_)); + return; + } + break; + + case PHP_ROUND_FLOOR: + if (num->n_sign == PLUS) { + *result = bc_copy_num(BCG(_zero_)); + return; + } + break; + + case PHP_ROUND_AWAY_FROM_ZERO: + break; + + EMPTY_SWITCH_DEFAULT_CASE() + } + + if (bc_is_zero(num)) { + *result = bc_copy_num(BCG(_zero_)); + return; + } + + /* If precision is -3, it becomes 1000. */ + if (UNEXPECTED(precision == ZEND_LONG_MIN)) { + *result = bc_new_num((size_t) ZEND_LONG_MAX + 2, 0); + } else { + *result = bc_new_num(-precision + 1, 0); + } + (*result)->n_value[0] = 1; + (*result)->n_sign = num->n_sign; return; } + /* Just like bcadd('1', '1', 4) becomes '2.0000', it pads with zeros at the end if necessary. */ if (precision >= 0 && num->n_scale <= precision) { if (num->n_scale == precision) { @@ -61,7 +105,7 @@ void bc_round(bc_num num, zend_long precision, zend_long mode, bc_num *result) * If the result of rounding is carried over, it will be added later, so first set it to 0 here. */ if (rounded_len == 0) { - *result = bc_copy_num(BCG(_zero_)); + *result = bc_new_num(1, 0); } else { *result = bc_new_num(num->n_len, precision > 0 ? precision : 0); memcpy((*result)->n_value, num->n_value, rounded_len); diff --git a/ext/bcmath/tests/bcround_away_from_zero.phpt b/ext/bcmath/tests/bcround_away_from_zero.phpt index df1a16ac18774..b50072d048b15 100644 --- a/ext/bcmath/tests/bcround_away_from_zero.phpt +++ b/ext/bcmath/tests/bcround_away_from_zero.phpt @@ -27,6 +27,9 @@ run_round_test(RoundingMode::AwayFromZero); [-1.9, 0] => -2 ========== minus precision ========== + [0, -3] => 0 + [0.01, -3] => 1000 + [-0.01, -3] => -1000 [50, -2] => 100 [-50, -2] => -100 [1230, -1] => 1230 diff --git a/ext/bcmath/tests/bcround_ceiling.phpt b/ext/bcmath/tests/bcround_ceiling.phpt index 5fd93c2621477..a335695aea1c2 100644 --- a/ext/bcmath/tests/bcround_ceiling.phpt +++ b/ext/bcmath/tests/bcround_ceiling.phpt @@ -27,6 +27,9 @@ run_round_test(RoundingMode::PositiveInfinity); [-1.9, 0] => -1 ========== minus precision ========== + [0, -3] => 0 + [0.01, -3] => 1000 + [-0.01, -3] => 0 [50, -2] => 100 [-50, -2] => 0 [1230, -1] => 1230 diff --git a/ext/bcmath/tests/bcround_early_return.phpt b/ext/bcmath/tests/bcround_early_return.phpt index e1980dd3778c9..cea57fcb24ad7 100644 --- a/ext/bcmath/tests/bcround_early_return.phpt +++ b/ext/bcmath/tests/bcround_early_return.phpt @@ -6,15 +6,11 @@ bcmath --EXPECT-- - [123, -4] => 0 - [123.123456, -4] => 0 [123, 1] => 123.0 [123.5, 1] => 123.5 [123.5, 2] => 123.50 [123.0000000000000000000001, 22] => 123.0000000000000000000001 [123.0000000000000000000001, 23] => 123.00000000000000000000010 - [-123, -4] => 0 - [-123.123456, -4] => 0 [-123, 1] => -123.0 [-123.5, 1] => -123.5 [-123.5, 2] => -123.50 diff --git a/ext/bcmath/tests/bcround_floor.phpt b/ext/bcmath/tests/bcround_floor.phpt index 346bc81e86e7c..767d39159578b 100644 --- a/ext/bcmath/tests/bcround_floor.phpt +++ b/ext/bcmath/tests/bcround_floor.phpt @@ -27,6 +27,9 @@ run_round_test(RoundingMode::NegativeInfinity); [-1.9, 0] => -2 ========== minus precision ========== + [0, -3] => 0 + [0.01, -3] => 0 + [-0.01, -3] => -1000 [50, -2] => 0 [-50, -2] => -100 [1230, -1] => 1230 diff --git a/ext/bcmath/tests/bcround_half_down.phpt b/ext/bcmath/tests/bcround_half_down.phpt index 69a2a24ed8c24..d179402f27f7a 100644 --- a/ext/bcmath/tests/bcround_half_down.phpt +++ b/ext/bcmath/tests/bcround_half_down.phpt @@ -27,6 +27,9 @@ run_round_test(RoundingMode::HalfTowardsZero); [-1.9, 0] => -2 ========== minus precision ========== + [0, -3] => 0 + [0.01, -3] => 0 + [-0.01, -3] => 0 [50, -2] => 0 [-50, -2] => 0 [1230, -1] => 1230 diff --git a/ext/bcmath/tests/bcround_half_even.phpt b/ext/bcmath/tests/bcround_half_even.phpt index 1413ca69ce123..2ae29bc8e3104 100644 --- a/ext/bcmath/tests/bcround_half_even.phpt +++ b/ext/bcmath/tests/bcround_half_even.phpt @@ -27,6 +27,9 @@ run_round_test(RoundingMode::HalfEven); [-1.9, 0] => -2 ========== minus precision ========== + [0, -3] => 0 + [0.01, -3] => 0 + [-0.01, -3] => 0 [50, -2] => 0 [-50, -2] => 0 [1230, -1] => 1230 diff --git a/ext/bcmath/tests/bcround_half_odd.phpt b/ext/bcmath/tests/bcround_half_odd.phpt index 5207142152e9c..970e565afb04d 100644 --- a/ext/bcmath/tests/bcround_half_odd.phpt +++ b/ext/bcmath/tests/bcround_half_odd.phpt @@ -27,6 +27,9 @@ run_round_test(RoundingMode::HalfOdd); [-1.9, 0] => -2 ========== minus precision ========== + [0, -3] => 0 + [0.01, -3] => 0 + [-0.01, -3] => 0 [50, -2] => 100 [-50, -2] => -100 [1230, -1] => 1230 diff --git a/ext/bcmath/tests/bcround_half_up.phpt b/ext/bcmath/tests/bcround_half_up.phpt index 52129535f2d7f..21a76407ea582 100644 --- a/ext/bcmath/tests/bcround_half_up.phpt +++ b/ext/bcmath/tests/bcround_half_up.phpt @@ -27,6 +27,9 @@ run_round_test(RoundingMode::HalfAwayFromZero); [-1.9, 0] => -2 ========== minus precision ========== + [0, -3] => 0 + [0.01, -3] => 0 + [-0.01, -3] => 0 [50, -2] => 100 [-50, -2] => -100 [1230, -1] => 1230 diff --git a/ext/bcmath/tests/bcround_test_helper.inc b/ext/bcmath/tests/bcround_test_helper.inc index 00adf18c52982..f41c4b800cf69 100644 --- a/ext/bcmath/tests/bcround_test_helper.inc +++ b/ext/bcmath/tests/bcround_test_helper.inc @@ -30,6 +30,9 @@ function run_round_test(RoundingMode $mode) ]; $minus_precision_cases = [ + ['0', -3], + ['0.01', -3], + ['-0.01', -3], ['50', -2], ['-50', -2], ['1230', -1], diff --git a/ext/bcmath/tests/bcround_toward_zero.phpt b/ext/bcmath/tests/bcround_toward_zero.phpt index 2d7275ada9589..8fd94d975532a 100644 --- a/ext/bcmath/tests/bcround_toward_zero.phpt +++ b/ext/bcmath/tests/bcround_toward_zero.phpt @@ -27,6 +27,9 @@ run_round_test(RoundingMode::TowardsZero); [-1.9, 0] => -1 ========== minus precision ========== + [0, -3] => 0 + [0.01, -3] => 0 + [-0.01, -3] => 0 [50, -2] => 0 [-50, -2] => 0 [1230, -1] => 1230 diff --git a/ext/bcmath/tests/number/methods/round.phpt b/ext/bcmath/tests/number/methods/round.phpt index e54d3f5c9a15d..9385b2667db5e 100644 --- a/ext/bcmath/tests/number/methods/round.phpt +++ b/ext/bcmath/tests/number/methods/round.phpt @@ -6,6 +6,7 @@ bcmath round(0, $mode); - if ($method_ret->compare($func_ret) !== 0) { - echo "Result is incorrect.\n"; - var_dump($number, $mode, $func_ret, $method_ret); + foreach ([0, 5, -5] as $scale) { + $func_ret = bcround($number, $scale, $mode); + $method_ret = (new BcMath\Number($number))->round($scale, $mode); + if ($method_ret->compare($func_ret) !== 0) { + echo "Result is incorrect.\n"; + var_dump($number, $mode, $func_ret, $method_ret); + } } } } foreach (RoundingMode::cases() as $mode) { foreach ([ + '0', '1.2345678', '-1.2345678', ] as $number) {