From 6d6a3ca978439c0ac6d37175087b9f288d394416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 23 May 2024 22:04:52 +0200 Subject: [PATCH 1/4] random: Remove `php_rand()` This effectively is just a slim wrapper around `(zend_long)php_mt_rand()`. It is not compatible between 32-bit and 64-bit builds of PHP, due to the use of `zend_long`, which may result in negative integersbeing returned on 32-bit platforms, whereas 64-bit platforms will be compatible with `php_mt_rand()`. An example would be the `0` seed, which emits 2357136044 on 64-bit platforms and -1937831252 on 32-bit platforms. Users of `php_rand()` should ideally migrate to one of the more modern engines, with extension-specific state. If drop-in compatibility is desired, they can just cast the result of `php_mt_rand()`. But providing it out of the box does not provide a value-add and is potentially dangerous. --- ext/random/php_random.h | 1 - ext/random/random.c | 7 ------- 2 files changed, 8 deletions(-) diff --git a/ext/random/php_random.h b/ext/random/php_random.h index 45c527a39effb..b61f7eecd8aab 100644 --- a/ext/random/php_random.h +++ b/ext/random/php_random.h @@ -65,7 +65,6 @@ PHPAPI zend_long php_mt_rand_common(zend_long min, zend_long max); # define PHP_RAND_MAX PHP_MT_RAND_MAX PHPAPI void php_srand(zend_long seed); -PHPAPI zend_long php_rand(void); typedef struct _php_random_status_state_combinedlcg { int32_t state[2]; diff --git a/ext/random/random.c b/ext/random/random.c index 8acf3ee17ef7b..c947de99240a0 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -453,13 +453,6 @@ PHPAPI void php_srand(zend_long seed) } /* }}} */ -/* {{{ php_rand */ -PHPAPI zend_long php_rand(void) -{ - return php_mt_rand(); -} -/* }}} */ - /* {{{ Returns a value from the combined linear congruential generator */ PHP_FUNCTION(lcg_value) { From bcda548b007eb5934b71939ec49651a5281869c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 23 May 2024 22:15:38 +0200 Subject: [PATCH 2/4] random: Remove `php_srand()` With `php_rand()` gone, preserving its companion `php_srand()` is just confusing. The same recommendations apply: Migrate to a modern engine if possible and just call `php_mt_srand()` with an appropriately casted input. --- ext/random/php_random.h | 2 -- ext/random/random.c | 7 ------- 2 files changed, 9 deletions(-) diff --git a/ext/random/php_random.h b/ext/random/php_random.h index b61f7eecd8aab..82ffe1cc4d2a8 100644 --- a/ext/random/php_random.h +++ b/ext/random/php_random.h @@ -64,8 +64,6 @@ PHPAPI zend_long php_mt_rand_common(zend_long min, zend_long max); # define PHP_RAND_MAX PHP_MT_RAND_MAX -PHPAPI void php_srand(zend_long seed); - typedef struct _php_random_status_state_combinedlcg { int32_t state[2]; } php_random_status_state_combinedlcg; diff --git a/ext/random/random.c b/ext/random/random.c index c947de99240a0..9c31f9306df5d 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -446,13 +446,6 @@ PHPAPI zend_long php_mt_rand_common(zend_long min, zend_long max) } /* }}} */ -/* {{{ php_srand */ -PHPAPI void php_srand(zend_long seed) -{ - php_mt_srand((uint32_t) seed); -} -/* }}} */ - /* {{{ Returns a value from the combined linear congruential generator */ PHP_FUNCTION(lcg_value) { From d9b37b7d104f866334eb412b0ab273dc300aab5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 23 May 2024 22:20:07 +0200 Subject: [PATCH 3/4] random: Remove `PHP_RAND_MAX` and `RAND_MAX` These are the companions to `php_rand()`, which was removed in a previous commit. Generally speaking the maximum returnable value is not particularly useful anyways. Attempting it to create a random float by dividing the returned integer by the maximum value would result in a bias if the maximum value would be larger than 2**53 and even for that case, the various `range()` helpers allow to easily retrieve a uniformly distributed integer from a suitable range. --- ext/random/php_random.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ext/random/php_random.h b/ext/random/php_random.h index 82ffe1cc4d2a8..4dac912e75a7d 100644 --- a/ext/random/php_random.h +++ b/ext/random/php_random.h @@ -58,12 +58,6 @@ PHPAPI uint32_t php_mt_rand(void); PHPAPI zend_long php_mt_rand_range(zend_long min, zend_long max); PHPAPI zend_long php_mt_rand_common(zend_long min, zend_long max); -# ifndef RAND_MAX -# define RAND_MAX PHP_MT_RAND_MAX -# endif - -# define PHP_RAND_MAX PHP_MT_RAND_MAX - typedef struct _php_random_status_state_combinedlcg { int32_t state[2]; } php_random_status_state_combinedlcg; From a1e47454ca6150550f964321cb2ce9ff55ca9ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 23 May 2024 22:38:26 +0200 Subject: [PATCH 4/4] UPGRADING.INTERNALS --- UPGRADING.INTERNALS | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 8f62015f733fd..cf78c23ff2e50 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -174,6 +174,17 @@ PHP 8.4 INTERNALS UPGRADE NOTES - The macro RAND_RANGE_BADSCALING() has been removed. The implementation should either be inlined and undefined behavior fixed or it should be replaced by a non-biased scaler. + - The php_srand() and php_rand() functions have been removed. These were + slim wrappers around the corresponding php_mt_srand() and php_mt_rand() + function since PHP 7.1, but using zend_long instead of uint32_t as their + input/output types. This made their behavior incompatible between 32-bit + and 64-bit builds of PHP. Users of these functions are encouraged to + migrate to one of the more modern engines provided since PHP 8.2. If that + is not possible, due to backwards compatibility requirements, then the + php_mt_srand() and php_mt_rand() functions should be called directly and + the values appropriately casted. + - The PHP_RAND_MAX and RAND_MAX constants corresponding to the removed + php_rand() have also been removed. - The generate member of a php_random_algo is now expected to return the new php_random_result struct, replacing the last_generated_size member of the php_random_status struct and the generate_size member of