[random] Suppress UBSan #9060
[random] Suppress UBSan #9060
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);
| @@ -317,7 +325,7 @@ PHPAPI zend_object *php_random_engine_common_clone_object(zend_object *object) | |||
| } | |||
|
|
|||
| /* {{{ 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
| @@ -61,6 +61,14 @@ | |||
|
|
|||
| #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. |
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.