# Fix for #64450 - Thread: https://externals.io/message/66741 - Original archive: https://news-web.php.net/php.internals/66741 - Started by Anatol Belski on 2013-03-21T16:46:49+00:00 - Messages: 5 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #66741 — Anatol Belski — 2013-03-21T16:46:49+00:00 URL: https://externals.io/message/66741#66741 Hi Dmitry, I developed a patch for this one https://bugs.php.net/bug.php?id=64450 . It's regarding to the long overflow in `mt_rand()`. The main idea is to work with the args as double internally and then return php float if it exceeds the LONG_MAX. I strived to let the old behavior to be unchanged, all the older tests pass. Please take a look. Regards Anatol --- ## #66746 — Anatol Belski — 2013-03-22T08:08:39+00:00 URL: https://externals.io/message/66741#66746 Dmitry, first of all thanks for taking a look :) The issue in a few words - mt_rand reads arguments as long - user pass a float from the userspace - zend_parse_parameters with 'l' casts to long Here's the essential snippet echo mt_rand(0,pow(10,12)); PHP Warning: `mt_rand()`: max(-727379968) is smaller than min(0) So reading as long would probably not work, in the first place because the input data exceeding LONG_MAX will be corrupted by the conversion. So what I did - read input as max precision possible, no corruption - calculate as well with double - return the old way int if it's in LONG_MAX range, otherwise return float Is there a solution I don't see (or do I get your suggestion wrong)? It's just essential to get the input as precise as it can be across PHP. Besides that what could be impacts changing input arg types in this concrete case? User probably wouldn't realize that at all. Thanks Anatol > Hi Anatol, > > > To be honest, I didn't understand all the details of the patch :) > > > However, I see a problem: > You changed the prototype of user level `rand()` function to receive double > arguments instead of long. I think it's disallowed, but you may get > arguments as longs and then convert them to double. Will it work? > > > Thanks. Dmitry. > > > > > > > […] --- ## #66749 — Dmitry Stogov — 2013-03-22T09:08:29+00:00 URL: https://externals.io/message/66741#66749 · In reply to: #66746 Thanks, now I understood. :) anyway I see a problem. For example on x86_64 double is not always able to keep a long number without precision lost. May be you should receive arguments as zvals and use old or new code depending on input types. BTW: I'm not sure if `rand()` should be fixed at all. According to http://php.net/manual/en/function.rand.php it should accept integers and return integer. Thanks. Dmitry. > Dmitry, > > first of all thanks for taking a look :) > > The issue in a few words > - mt_rand reads arguments as long > - user pass a float from the userspace > - zend_parse_parameters with 'l' casts to long > > Here's the essential snippet > echo mt_rand(0,pow(10,12)); > PHP Warning: `mt_rand()`: max(-727379968) is smaller than min(0) > > So reading as long would probably not work, in the first place because the > input data exceeding LONG_MAX will be corrupted by the conversion. So what > I did > - read input as max precision possible, no corruption > - calculate as well with double > - return the old way int if it's in LONG_MAX range, otherwise return float > > Is there a solution I don't see (or do I get your suggestion wrong)? It's > just essential to get the input as precise as it can be across PHP. > > Besides that what could be impacts changing input arg types in this > concrete case? User probably wouldn't realize that at all. > > Thanks > > Anatol > > > > […] --- ## #66752 — Anatol Belski — 2013-03-22T10:34:49+00:00 URL: https://externals.io/message/66741#66752 · In reply to: #66749 > Thanks, now I understood. :) > > > anyway I see a problem. For example on x86_64 double is not always able > to keep a long number without precision lost. May be you should receive > arguments as zvals and use old or new code depending on input types. Damn that's right, I've just read this page http://www.viva64.com/en/a/0004/#ID0EQ3BI double is on both x64 and x86 64 bit, but effectively 52 bits used for the integer part. My patch would work on 32 bit Linux and both x86 and x64 windows, but would fail on x64 Linux/unix systems. I have to come up with a better solution to do this check on compile or (more likely) on run time. > > BTW: I'm not sure if `rand()` should be fixed at all. > According to http://php.net/manual/en/function.rand.php it should accept > integers and return integer. Yep, formally it's documented. De facto users meet that issue and that could indeed be made better, that's my motivation. Thanks for the tips. Anatol > Thanks. Dmitry. > > > > > > > […] --- ## #66806 — Anatol Belski — 2013-03-25T19:41:01+00:00 URL: https://externals.io/message/66741#66806 · In reply to: #66752 Dmitry, after spending more time on this issue I came to the conclusion it shouldn't be touched :) . The __x86_64__ you've mentioned is solvable on compile time using a macros like this defined(PHP_WIN32) || ((defined(__i386__) || defined(__i386)) && defined(__STDC_IEC_559__)) so either old or new variant would be activated. __STDC_IEC_559__ is a macros wintessing IEEE 754 conformity, so double can hold long on 32 bit. But another issue I've overseen is that the php_mt_rand() delivers in uint range, that's way too few if one wants to work with double as incoming type. There are also some other places like PHP_RAND_MAX which are incompatible. That isn't solvable with just touching the pt_rand function. At the end of the day seems there is no way around to get this issue properly working on both 64 and 32 bit platforms than implementing the Mersenne Twister for every applicable variation, effectively double or long. That might coexist in pecl or in core. Any other intrusion would change the core significantly. Thanks for your support! Anatol On Fri, 2013-03-22 at 11:34 +0100, Anatol Belski wrote: > > > […] > > Damn that's right, I've just read this page > http://www.viva64.com/en/a/0004/#ID0EQ3BI > > double is on both x64 and x86 64 bit, but effectively 52 bits used for the > integer part. My patch would work on 32 bit Linux and both x86 and x64 > windows, but would fail on x64 Linux/unix systems. I have to come up with > a better solution to do this check on compile or (more likely) on run > time. > > > […] > > Yep, formally it's documented. De facto users meet that issue and that > could indeed be made better, that's my motivation. > > Thanks for the tips. > > Anatol > > > > […]