Conversation
This is equivalent to the behavior of the original PHP API, and similar events occur in PHP 8.1 mt_rand() in PHPT test cases. > mt_rand(PHP_INT_MIN, PHP_INT_MAX);
|
|
||
| /* {{{ php_random_range */ | ||
| PHPAPI zend_long php_random_range(const php_random_algo *algo, php_random_status *status, zend_long min, zend_long max) | ||
| UBSAN_SUPPRESS_SIGNED_INTEGER_OVERFLOW PHPAPI zend_long php_random_range(const php_random_algo *algo, php_random_status *status, zend_long min, zend_long max) |
There was a problem hiding this comment.
Can we not avoid this undefined behavior by using unsigned integers and only then casting them to signed? I think all we might need is:
return (zend_long) (rand_range64(algo, status, umax) + min);
There was a problem hiding this comment.
Related, php_random_int does umax = (zend_ulong) max - (zend_ulong) min;
There was a problem hiding this comment.
@iluuu1994 @guilliamxavier
I had BC break concerns about this, but apparently my fears were unfounded. I will correct it to the appropriate form. Thanks!
$ docker run --rm -it arm64v8/php:8.1-cli -r 'mt_srand(1234); echo mt_rand(PHP_INT_MIN, PHP_INT_MAX) . PHP_EOL;'
-5690461752414248237
$ docker run --rm -it amd64/php:8.1-cli -r 'mt_srand(1234); echo mt_rand(PHP_INT_MIN, PHP_INT_MAX) . PHP_EOL;'
WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
-5690461752414248237
|
|
||
| #if __has_feature(memory_sanitizer) | ||
| # include <sanitizer/msan_interface.h> | ||
| # define MSAN_UNPOISON(ptr, size) __msan_unpoison((ptr), (size)); |
There was a problem hiding this comment.
trailing semicolon to remove? (but why touch this part anyway?)
There was a problem hiding this comment.
This is because we initially used a similar macro branch. Revert back to the original.
|
Having found the original solution to be inadequate, close and remediate this PR. |
3 participants
Undefined behavior is similarly detected at the time of mt_rand() in PHP 8.1.
This is detrimental to testing and should be suppressed in the code.