The reference implementation of the "Drawing Random Floating-Point Numbers from
an Interval" paper contains two mistakes that will result in erroneous values
being returned under certain circumstances:
- For large values of `g` the multiplication of `k * g` might overflow to
infinity.
- The value of `ceilint()` might exceed 2^53, possibly leading to a rounding
error when promoting `k` to double within the multiplication of `k * g`.
This commit updates the implementation based on Prof. Goualard suggestions
after reaching out to him. It will correctly handle inputs larger than 2^-1020
in absolute values. This limitation will be documented and those inputs
possibly be rejected in a follow-up commit depending on performance concerns.
`PHPAPI` is defined in `php.h`. It appears that without the explicit include,
recent versions of Visual Studio Code’s intellisense (rightfully) no longer
detect `PHPAPI`. This will then lead to a misparsing of the file, because
`PHPAPI` is assumed to be the return type and the actual return type is assumed
to be the function name, thus expecting a semicolon after the actual return
type. This in turn colors the entire header in red due to the detected syntax
error(s), making development very hard / impossible.
This did not cause issues outside of the IDE use case, because apparently all
users of `php_random.h` include `php.h` before including `php_random.h`.
The CSPRNG is a delicate and security relevant piece of code and having it in
the giant random.c makes it much harder to verify changes to it. Split it into
a separate file.
* random: Add `max_offset` local to Randomizer::getBytesFromString()
* random: Use branchless implementation for mask generation in Randomizer::getBytesFromString()
This was benchmarked against clzl with a standalone script with random inputs
and is slightly faster. clzl requires an additional branch to handle the
source_length = 1 / max_offset = 0 case.
* Improve comment for masking in Randomizer::getBytesFromString()
With a single byte we can choose offsets between 0x00 and 0xff, thus 0x100
different offsets. We only need to use the slow path for sources of more than
0x100 bytes.
The previous version was correct with regard to the output expectations, it was
just slower than necessary. Better fix this now while we still can before being
bound by our BC guarantees with regard to emitted sequences.
This also adds a test to verify the behavior: For powers of two we never reject
any values during rejection sampling, we just need to mask off the unneeded
bits. Thus we can specifically verify that the number of calls to the engine
match the expected amount. We also verify that all the possible values are
emitted to make sure the masking does not remove any required bits. For inputs
longer than 0x100 bytes we need trust the `range()` implementation to be
unbiased, but still verify the number of engine calls and perform a basic
output check.
* random: Convert the urandom loop into a while() loop
This allows us to more easily reduce the scope of `n` in a future commit and
now matches the getrandom(2) loop.
* random: Move the errno reset immediately above the getrandom(2) call
* random: Reduce the scope of `n` in the CSPRNG
* random: Declare `n` outside of preprocessor branch
The only way the previous `if (read_bytes < size)` branch could be taken is
when the loop was exited by the `break;` statement. We can just merge this into
the loop to make the code more obvious.
This effectively reverts #8984.
As discussed in #10327 which will enable the use of the getrandom(2) syscall on
NetBSD instead of relying on the userland arc4random_buf(), the CSPRNG should
prioritize security over speed [1] and history has shown that userland
implementations unavoidably fall short on the security side. In fact the glibc
implementation is a thin wrapper around the syscall due to security concerns
and thus does not provide any benefit over just calling getrandom(2) ourselves.
Even without any performance optimizations the CSPRNG should be plenty fast for
the vast majority of applications, because they often only need a few bytes of
randomness to generate a session ID. If speed is desired, the OO API offers
faster, but non-cryptographically secure engines.
It cannot be decided whether the device is available at build time, PHP might
run in a container or chroot that does not expose the device. Simply attempt to
open it, if it does not exist it will fail.
This improves readability of php_random_bytes() by removing one layer of
preprocessor conditions.
* random: Randomizer::getFloat(): Fix check for empty open intervals
The check for invalid parameters for the IntervalBoundary::OpenOpen variant was
not correct: If two consecutive doubles are passed as parameters, the resulting
interval is empty, resulting in an uint64 underflow in the γ-section
implementation.
Instead of checking whether `$min < $max`, we must check that there is at least
one more double between `$min` and `$max`, i.e. it must hold that:
nextafter($min, $max) != $max
Instead of duplicating the comparatively complicated and expensive `nextafter`
logic for a rare error case we instead return `NAN` from the γ-section
implementation when the parameters result in an empty interval and thus underflow.
This allows us to reliably detect this specific error case *after* the fact,
but without modifying the engine state. It also provides reliable error
reporting for other internal functions that might use the γ-section
implementation.
* random: γ-section: Also check that that min is smaller than max
This extends the empty-interval check in the γ-section implementation with a
check that min is actually the smaller of the two parameters.
* random: Use PHP_FLOAT_EPSILON in getFloat_error.phpt
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
If, for whatever reason, the random_fd has been assigned file descriptor `0` it
previously failed to close during module shutdown, thus leaking the descriptor.
* random: Add Randomizer::nextFloat()
* random: Check that doubles are IEEE-754 in Randomizer::nextFloat()
* random: Add Randomizer::nextFloat() tests
* random: Add Randomizer::getFloat() implementing the y-section algorithm
The algorithm is published in:
Drawing Random Floating-Point Numbers from an Interval. Frédéric
Goualard, ACM Trans. Model. Comput. Simul., 32:3, 2022.
https://doi.org/10.1145/3503512
* random: Implement getFloat_gamma() optimization
see https://github.com/php/php-src/pull/9679/files#r994668327
* random: Add Random\IntervalBoundary
* random: Split the implementation of γ-section into its own file
* random: Add tests for Randomizer::getFloat()
* random: Fix γ-section for 32-bit systems
* random: Replace check for __STDC_IEC_559__ by compile-time check for DBL_MANT_DIG
* random: Drop nextFloat_spacing.phpt
* random: Optimize Randomizer::getFloat() implementation
* random: Reject non-finite parameters in Randomizer::getFloat()
* random: Add NEWS/UPGRADING for Randomizer’s float functionality
* Fix pre-PHP 8.2 compatibility for php_mt_rand_range() with MT_RAND_PHP
As some left-over comments indicated:
> Legacy mode deliberately not inside php_mt_rand_range()
> to prevent other functions being affected
The broken scaler was only used for `php_mt_rand_common()`, not
`php_mt_rand_range()`. The former is only used for `mt_rand()`, whereas the
latter is used for `array_rand()` and others.
With the refactoring for the introduction of ext/random `php_mt_rand_common()`
and `php_mt_rand_range()` were accidentally unified, thus introducing a
behavioral change that was reported in FakerPHP/Faker#528.
This commit moves the checks for `MT_RAND_PHP` from the general-purpose
`range()` function back into `php_mt_rand_common()` and also into
`Randomizer::getInt()` for drop-in compatibility with `mt_rand()`.
* [ci skip] NEWS for `MT_RAND_PHP` compatibility
* Remove superfluous helper variable in Randomizer::getBytes()
* Reduce the scope of `result` in Randomizer::getBytes()
Co-authored-by: Tim Düsterhus <tim@bastelstu.be>
This reverts commit 94ee4f9834.
The commit was a bit too late to be included in PHP 8.2 RC1. Given it's a massive ABI break, we decide to postpone the change to PHP 8.3.
* Apply `var_dump()` in 02_engine/all_serialize_error.phpt
This ensures that an undetected serialization error is clear identifiable in the output.
* random: Validate that the arrays do not contain extra elements when unserializing
A simple check for CommonCrypto/CommonRandom.h does not work on earlier macOS.
Must also pull in sys/types.h for size_t, Availability.h for __OSX_AVAILABLE_STARTING,
and CommonCrypto/CommonCryptoError.h for CCCryptorStatus.
Closes GH-9479.
* Unify structure for ext/random's engine tests (2)
This makes adjustments that were missed in
2d6a883b3a.
* Add `engines.inc` for ext/random tests
* Unify structure for ext/random's randomizer tests
