Random extension random.c:95:20: runtime error: shift exponent 32 is too large for 32-bit type 'unsigned int' #9083
Comments
|
ext/spl/php_spl.c includes ext/random/php_random.h: Line 39 in 067a302 However, that is apparently not needed for that compilation unit. |
|
@cmb69 indeed, this line (previously Line 39 in 8fec415 But besides that, there's indeed an UB to "fix": #9056 (comment) |
Removing it doesn't seem to fix the issue in SPL anyway |
|
Sorry for the noise: Line 25 in f60f6be Line 25 in 8fec415 Anyway, removing it won't fix the issue either 😅 |
Nope, but #9056 (comment) should. |
The previous shifting logic is problematic for two reasons: 1. It invokes undefined behavior when the `->last_generated_size` is at least as large as the target integer in `result`, because the shift is larger than the target integer. This was reported in phpGH-9083. 2. It expands the returned bytes in a big-endian fashion: Earlier bytes are shifting into the most-significant position. As all the other logic in the random extension treats byte-strings as little-endian numbers this is inconsistent. By fixing the second issue, we can implicitly fix the first one: Instead of shifting the existing bits by the number of "newly added" bits, we shift the newly added bits by the number of existing bits. As we stop requesting new bits once the total_size reached the size of the target integer we can be sure to never invoke undefined behavior during shifting. The get_int_user.phpt test was adjusted to verify the little-endian behavior. It generates a single byte per call and we expect the first byte generated to appear at the start of the resulting number. see phpGH-9056 for a previous fix in the same area. Fixes phpGH-9083 which reports the undefined behavior. Resolves phpGH-9085 which was an alternative attempt to fix phpGH-9083.
The previous shifting logic is problematic for two reasons: 1. It invokes undefined behavior when the `->last_generated_size` is at least as large as the target integer in `result`, because the shift is larger than the target integer. This was reported in GH-9083. 2. It expands the returned bytes in a big-endian fashion: Earlier bytes are shifting into the most-significant position. As all the other logic in the random extension treats byte-strings as little-endian numbers this is inconsistent. By fixing the second issue, we can implicitly fix the first one: Instead of shifting the existing bits by the number of "newly added" bits, we shift the newly added bits by the number of existing bits. As we stop requesting new bits once the total_size reached the size of the target integer we can be sure to never invoke undefined behavior during shifting. The get_int_user.phpt test was adjusted to verify the little-endian behavior. It generates a single byte per call and we expect the first byte generated to appear at the start of the resulting number. see GH-9056 for a previous fix in the same area. Fixes GH-9083 which reports the undefined behavior. Resolves GH-9085 which was an alternative attempt to fix GH-9083.
Description
Various SPL tests on master now fail with:
PHP Version
master
Operating System
Fedora 35
The text was updated successfully, but these errors were encountered: