Skip to content

ext/gmp: Improve parsing of parameters #16685

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

Merged
merged 22 commits into from
Nov 27, 2024
Merged

ext/gmp: Improve parsing of parameters #16685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
10f5161
ext/gmp: Split out non-existent inverse modulo cases
Girgias Nov 2, 2024
5b5af60
ext/gmp: Use Fast ZPP for GMP functions
Girgias Nov 2, 2024
4181a1e
Inline gmp_unary_opl() as it was only used once
Girgias Nov 2, 2024
282142f
ext/gmp: Create custom Fast ZPP specifier to parse into mpz_ptr
Girgias Nov 3, 2024
5441972
ext/gmp: Use new custom ZPP specifier
Girgias Nov 2, 2024
76033e1
ext/gmp: Refactor generation of unary GMP functions
Girgias Nov 2, 2024
602dd27
ext/gmp: Refactor generation of some binary GMP functions
Girgias Nov 2, 2024
de20a29
ext/gmp: Use new specifier for gmp_cmp()
Girgias Nov 2, 2024
4278aee
ext/gmp: Refactor gmp_random_range() to use new ZPP specifier
Girgias Nov 2, 2024
69c835a
ext/gmp: Refactor gmp_div_qr() to use new ZPP specifier
Girgias Nov 2, 2024
440e9fd
ext/gmp: Refactor gmp_div_(q|r)() to use new ZPP specifier
Girgias Nov 2, 2024
fd21bb4
ext/gmp: Refactor gmp_divexact() and gmp_mod() to use custom ZPP spec…
Girgias Nov 2, 2024
da912d2
ext/gmp: Remove now unused FETCH_GMP_ZVAL_DEP_DEP macro
Girgias Nov 2, 2024
18a16db
ext/gmp: Refactor gmp_fact() to use new ZPP specifier
Girgias Nov 2, 2024
84c4114
ext/gmp: Start refactoring operator overloading to use new parsing me…
Girgias Nov 3, 2024
80407a8
ext/gmp: Convert GMP operator overloading to new parsing mechanism
Girgias Nov 3, 2024
fc66ec2
ext/gmp: Add weak mode support for parsing
Girgias Nov 3, 2024
29a167c
ext/gmp: Use new parsing API in shift helper
Girgias Nov 3, 2024
f15f78e
ext/gmp: Use new parsing mechanism in comparison operator overloading
Girgias Nov 3, 2024
837c04d
ext/gmp: Refactor gmp_cmp() test
Girgias Nov 3, 2024
ff1eed3
Normalize gmp_cmp() to -1/0/+1
Girgias Nov 10, 2024
1b89878
ext/gmp: Remove redundant parenthesis
Girgias Nov 16, 2024
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
1,394 changes: 460 additions & 934 deletions ext/gmp/gmp.c
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions ext/gmp/php_gmp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ ZEND_MODULE_INFO_D(gmp);
ZEND_BEGIN_MODULE_GLOBALS(gmp)
bool rand_initialized;
gmp_randstate_t rand_state;
mpz_t zpp_arg[3];
ZEND_END_MODULE_GLOBALS(gmp)

#define GMPG(v) ZEND_MODULE_GLOBALS_ACCESSOR(gmp, v)
Expand Down
4 changes: 2 additions & 2 deletions ext/gmp/tests/bug32773.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ try {
--EXPECT--
10 + 0 = 10
10 + "0" = 10
Division by zero
Division by zero
gmp_div(): Argument #2 ($num2) Division by zero
gmp_div_qr(): Argument #2 ($num2) Division by zero
38 changes: 24 additions & 14 deletions ext/gmp/tests/gmp_cmp.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,27 @@ gmp
--FILE--
<?php

var_dump(gmp_cmp(123123,-123123));
var_dump(gmp_cmp("12345678900987654321","12345678900987654321"));
var_dump(gmp_cmp("12345678900987654321","123456789009876543211"));
var_dump(gmp_cmp(0,0));
var_dump(gmp_cmp(1231222,0));
var_dump(gmp_cmp(0,345355));
function cmp_helper($l, $r) {
echo 'gmp(', var_export($l, true), ', ', var_export($r, true), '): ';
$r = gmp_cmp($l, $r);
echo match (true) {
$r === 0 => "equals\n",
$r < 0 => "right greater than left\n",
$r > 0 => "left greater than right\n",
};
}

cmp_helper(123123,-123123);
cmp_helper("12345678900987654321","12345678900987654321");
cmp_helper("12345678900987654321","123456789009876543211");
cmp_helper(0,0);
cmp_helper(1231222,0);
cmp_helper(0,345355);

$n = gmp_init("827278512385463739");
var_dump(gmp_cmp(0,$n) < 0);
$n1 = gmp_init("827278512385463739");
var_dump(gmp_cmp($n1,$n));
var_dump(gmp_cmp($n1,$n) === 0);

try {
var_dump(gmp_cmp(array(),array()));
Expand All @@ -26,13 +36,13 @@ try {
echo "Done\n";
?>
--EXPECT--
int(2)
Copy link
Member

Choose a reason for hiding this comment

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

This seems to cause a test failure on some CI jobs, so something more is going on

Copy link
Member Author

Choose a reason for hiding this comment

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

The documentation of GMP says:

Compare op1 and op2. Return a positive value if op1 > op2, zero if op1 = op2, or a negative value if op1 < op2.

So probably we shouldn't be using the output of the values directly?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, reading source codes, seems mpir and gmp differ with mpz_gmp. The former can return the values -1, >= 0, GMP it is only -1, 0, 1 (if this is what you meant by "normalization" in your gmp_cmp commit).

Copy link
Member

Choose a reason for hiding this comment

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

Good find! I guess PHP should normalize such that the two libs have the same behaviour

int(0)
int(-1)
int(0)
int(1)
int(-1)
gmp(123123, -123123): left greater than right
gmp('12345678900987654321', '12345678900987654321'): equals
gmp('12345678900987654321', '123456789009876543211'): right greater than left
gmp(0, 0): equals
gmp(1231222, 0): left greater than right
gmp(0, 345355): right greater than left
bool(true)
bool(true)
int(0)
gmp_cmp(): Argument #1 ($num1) must be of type GMP|string|int, array given
Done
2 changes: 1 addition & 1 deletion ext/gmp/tests/gmp_div_q.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ object(GMP)#1 (1) {
["num"]=>
string(1) "0"
}
Division by zero
gmp_div_q(): Argument #2 ($num2) Division by zero
object(GMP)#2 (1) {
["num"]=>
string(1) "0"
Expand Down
4 changes: 2 additions & 2 deletions ext/gmp/tests/gmp_div_qr.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ array(2) {
string(1) "0"
}
}
Division by zero
Division by zero
gmp_div_qr(): Argument #2 ($num2) Division by zero
gmp_div_qr(): Argument #2 ($num2) Division by zero
array(2) {
[0]=>
object(GMP)#2 (1) {
Expand Down
2 changes: 1 addition & 1 deletion ext/gmp/tests/gmp_div_r.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ object(GMP)#1 (1) {
["num"]=>
string(1) "0"
}
Division by zero
gmp_div_r(): Argument #2 ($num2) Division by zero
object(GMP)#3 (1) {
["num"]=>
string(5) "12653"
Expand Down
2 changes: 1 addition & 1 deletion ext/gmp/tests/gmp_divexact.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ echo "Done\n";
?>
--EXPECT--
string(1) "0"
Division by zero
gmp_divexact(): Argument #2 ($num2) Division by zero
string(2) "10"
string(3) "512"
string(19) "5000000000000000000"
Expand Down
18 changes: 10 additions & 8 deletions ext/gmp/tests/gmp_invert.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ gmp
<?php

var_dump(gmp_strval(gmp_invert(123123,5467624)));
var_dump(gmp_strval(gmp_invert(123123,"3333334345467624")));
var_dump(gmp_strval(gmp_invert("12312323213123123",7624)));

try {
Expand All @@ -22,9 +21,11 @@ try {
echo $e->getMessage() . \PHP_EOL;
}

var_dump(gmp_strval(gmp_invert(0,28347)));
var_dump(gmp_strval(gmp_invert(-12,456456)));
var_dump(gmp_strval(gmp_invert(234234,-435345)));
echo "No inverse modulo\n";
var_dump(gmp_invert(123123,"3333334345467624"));
var_dump(gmp_invert(0,28347));
var_dump(gmp_invert(-12,456456));
var_dump(gmp_invert(234234,-435345));

$n = gmp_init("349827349623423452345");
$n1 = gmp_init("3498273496234234523451");
Expand Down Expand Up @@ -52,13 +53,14 @@ echo "Done\n";
?>
--EXPECT--
string(7) "2293131"
string(1) "0"
string(4) "5827"
Division by zero
Division by zero
string(1) "0"
string(1) "0"
string(1) "0"
No inverse modulo
bool(false)
bool(false)
bool(false)
bool(false)
string(22) "3498273496234234523441"
string(1) "1"
gmp_invert(): Argument #1 ($num1) must be of type GMP|string|int, array given
Expand Down
2 changes: 1 addition & 1 deletion ext/gmp/tests/gmp_mod.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ object(GMP)#2 (1) {
["num"]=>
string(1) "0"
}
Modulo by zero
gmp_mod(): Argument #2 ($num2) Modulo by zero
gmp_mod(): Argument #1 ($num1) must be of type GMP|string|int, array given
object(GMP)#4 (1) {
["num"]=>
Expand Down
2 changes: 1 addition & 1 deletion ext/gmp/tests/gmp_strval.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ echo "Done\n";
?>
--EXPECT--
gmp_strval(): Argument #1 ($num) is not an integer string
gmp_strval(): Argument #2 ($base) must be between 2 and 62, or -2 and -36
gmp_strval(): Argument #1 ($num) is not an integer string
gmp_strval(): Argument #1 ($num) must be of type GMP|string|int, resource given
string(7) "9765456"
gmp_strval(): Argument #2 ($base) must be between 2 and 62, or -2 and -36
Expand Down