Skip to content

ext/bcmath: Clear the sign of BcMath\Number results that truncate to zero #23967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
iliaal wants to merge 1 commit into from
Closed

ext/bcmath: Clear the sign of BcMath\Number results that truncate to zero #23967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ PHP NEWS
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
?? ??? ????, PHP 8.4.27

- BCMath:
. Fixed BcMath\Number results that truncate to zero keeping a negative sign
and comparing less than zero. (Ilia Alshanetsky)

- CLI
. Fix GH-22567 (Windows ZTS CLI SAPI should refresh its TSRMLS cache during
request activation). (matyhtf)
Expand Down
9 changes: 9 additions & 0 deletions ext/bcmath/bcmath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,9 @@ static zend_always_inline void bcmath_number_add_internal(
}
*ret = bc_add(n1, n2, *scale);
(*ret)->n_scale = MIN(*scale, (*ret)->n_scale);
if (bc_is_zero(*ret)) {
(*ret)->n_sign = PLUS;
}
bc_rm_trailing_zeros(*ret);
}

Expand All @@ -1060,6 +1063,9 @@ static zend_always_inline void bcmath_number_sub_internal(
}
*ret = bc_sub(n1, n2, *scale);
(*ret)->n_scale = MIN(*scale, (*ret)->n_scale);
if (bc_is_zero(*ret)) {
(*ret)->n_sign = PLUS;
}
bc_rm_trailing_zeros(*ret);
}

Expand All @@ -1076,6 +1082,9 @@ static zend_always_inline zend_result bcmath_number_mul_internal(
}
*ret = bc_multiply(n1, n2, *scale);
(*ret)->n_scale = MIN(*scale, (*ret)->n_scale);
if (bc_is_zero(*ret)) {
(*ret)->n_sign = PLUS;
}
bc_rm_trailing_zeros(*ret);
return SUCCESS;
}
Expand Down
12 changes: 10 additions & 2 deletions ext/bcmath/libbcmath/src/div.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,12 @@ bool bc_divide(bc_num numerator, bc_num divisor, bc_num *quot, size_t scale)
*quot = bc_new_num_nonzeroed(numerator->n_len, quot_scale);
char *qptr = (*quot)->n_value;
memcpy(qptr, numerator->n_value, numerator->n_len + quot_scale);
(*quot)->n_sign = numerator->n_sign == divisor->n_sign ? PLUS : MINUS;
_bc_rm_leading_zeros(*quot);
if (bc_is_zero(*quot)) {

@ndossche ndossche Sep 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the other n_sign assignment in this else branch

@iliaal iliaal Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

(*quot)->n_sign = PLUS;
} else {
(*quot)->n_sign = numerator->n_sign == divisor->n_sign ? PLUS : MINUS;
}
return true;
}

Expand Down Expand Up @@ -475,7 +479,11 @@ bool bc_divide(bc_num numerator, bc_num divisor, bc_num *quot, size_t scale)
for (size_t i = 0; i < numerator_bottom_extension; i++) {
*qptr++ = 0;
}
(*quot)->n_sign = numerator->n_sign == divisor->n_sign ? PLUS : MINUS;
if (bc_is_zero(*quot)) {

@ndossche ndossche Sep 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@iliaal iliaal Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

(*quot)->n_sign = PLUS;
} else {
(*quot)->n_sign = numerator->n_sign == divisor->n_sign ? PLUS : MINUS;
}
return true;
}

Expand Down
3 changes: 3 additions & 0 deletions ext/bcmath/libbcmath/src/raise.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ bool bc_raise(bc_num base, long exponent, bc_num *result, size_t scale) {
bc_free_num (result);
*result = temp;
(*result)->n_scale = MIN(scale, (*result)->n_scale);
if (bc_is_zero(*result)) {
(*result)->n_sign = PLUS;
}
}
bc_free_num (&power);
return true;
Expand Down
35 changes: 35 additions & 0 deletions ext/bcmath/tests/number/methods/calc_methods_zero_result_sign.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
--TEST--
BcMath\Number calc methods return an unsigned zero when the result truncates to zero
--EXTENSIONS--
bcmath
--FILE--
<?php
$cases = [
['div', '-0.001', '1', 0],
['div', '0.001', '-1', 0],
['div', '-0.001', '10', 2],
['div', '-0.001', '0.1', 1],
['add', '-0.001', '-0.001', 0],
['sub', '-0.001', '0.001', 0],
['mul', '-0.001', '1', 0],
['pow', '-0.1', 3, 2],
];

foreach ($cases as [$method, $num, $arg, $scale]) {
$ret = (new BcMath\Number($num))->$method($arg, $scale);
echo "{$num} {$method} {$arg}: {$ret} ", $ret <=> 0, ' ', var_export($ret == 0, true), "\n";
}

[$quot, $rem] = (new BcMath\Number('-0.001'))->divmod('1', 0);
echo "-0.001 divmod 1: {$quot} ", $quot <=> 0, ' ', var_export($quot == 0, true), "\n";
?>
--EXPECT--
-0.001 div 1: 0 0 true
0.001 div -1: 0 0 true
-0.001 div 10: 0.00 0 true
-0.001 div 0.1: 0.0 0 true
-0.001 add -0.001: 0 0 true
-0.001 sub 0.001: 0 0 true
-0.001 mul 1: 0 0 true
-0.1 pow 3: 0.00 0 true
-0.001 divmod 1: 0 0 true
Loading