# Bug #46701 - Thread: https://externals.io/message/43021 - Original archive: https://news-web.php.net/php.internals/43021 - Started by Moriyoshi Koizumi on 2009-02-12T16:48:21+00:00 - Messages: 9 - 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 --- ## #43021 — Moriyoshi Koizumi — 2009-02-12T16:48:21+00:00 URL: https://externals.io/message/43021#43021 Hey, I guess the patch relies on the 5.3's DVAL_TO_LVAL behavior that was changed by the fix for bug #42868, right? If so, this patch shouldn't be MFH'ed as the #42868 patch was not merged although I didn't remember any discussion on this. See also: http://marc.info/?l=php-internals&m=120799720922202&w=2 Regards, Moriyoshi --- ## #43027 — Moriyoshi Koizumi — 2009-02-12T20:56:34+00:00 URL: https://externals.io/message/43021#43027 · In reply to: #43021 See the results of the following on 5.2.6, 5.2.9rc2 and 5.3: php -r '$a[1e100] = 1; var_dump($a);' 5.2.6: array(1) { [-2147483648]=> int(1) } 5.2.9rc2: array(1) { [-1]=> int(1) } 5.3: array(1) { [2147483647]=> int(1) } I doubt the result of 5.2.9rc2 is quite what we expect, and this problem should be addressed in 5.3 with the 5.2's behavior unchanged. Moriyoshi > Hey, > > I guess the patch relies on the 5.3's DVAL_TO_LVAL behavior that was > changed by the fix for bug #42868, right? > If so, this patch shouldn't be MFH'ed as the #42868 patch was not > merged although I didn't remember any discussion on this. > > See also: http://marc.info/?l=php-internals&m=120799720922202&w=2 > > Regards, > Moriyoshi --- ## #43033 — Ilia Alshanetsky — 2009-02-13T13:57:02+00:00 URL: https://externals.io/message/43021#43033 · In reply to: #43027 Dmitry, Does it make sense to backport 42868 fix to address this issue? > See the results of the following on 5.2.6, 5.2.9rc2 and 5.3: > > php -r '$a[1e100] = 1; var_dump($a);' > > 5.2.6: > array(1) { > [-2147483648]=> > int(1) > } > > 5.2.9rc2: > array(1) { > [-1]=> > int(1) > } > > 5.3: > array(1) { > [2147483647]=> > int(1) > } > > I doubt the result of 5.2.9rc2 is quite what we expect, and this > problem should be addressed in 5.3 with the 5.2's behavior unchanged. > > Moriyoshi > > On Fri, Feb 13, 2009 at 1:48 AM, Moriyoshi Koizumi > wrote: > > […] > > -- > > > Ilia Alshanetsky --- ## #43040 — Moriyoshi Koizumi — 2009-02-14T08:27:40+00:00 URL: https://externals.io/message/43021#43040 · In reply to: #43033 Please don't even think of backporting. This will definitely break a lot of things, and this kind of thing must not be done in a minor release. Moriyoshi > Dmitry, > > Does it make sense to backport 42868 fix to address this issue? > > > > > > […] > > Ilia Alshanetsky --- ## #43089 — zoe.slattery@googlemail.com — 2009-02-18T11:28:15+00:00 URL: https://externals.io/message/43021#43089 · In reply to: #43040 Moriyoshi Koizumi wrote: > Please don't even think of backporting. This will definitely break a > lot of things, and this kind of thing must not be done in a minor > release. > > --snip-- > > […] Hey all I'm sorry - I should have replied to this before since I was responsible for raising #42868. I didn't do a good job at explaining what the issue was in that bug, mainly because I didn't know what it was when I started. The central problem is that PHP's behaviour on casting double to int defaults to whatever the underlying C environment does. On Windows and Linux (all of the versions that I looked at) this turns out to be a simple truncation of the last 32 bits. Unfortunately the C behaviour is 'undefined' (Kernigan and Ritchie, page 197, A6.3). The issue that I found in #42868 was that on the Mac the casting behaviour is completely different so many of the PHP tests failed. I believe that PHP should behave in a platform independent way - that is what the fix to #42868 achieves. It is also fair to say that any applications that depend on the overflow behaviour in PHP 5.2 cannot be guaranteed to run on any platform. Zoe > > […] --- ## #43091 — Matt Wilmas — 2009-02-18T13:25:39+00:00 URL: https://externals.io/message/43021#43091 · In reply to: #43089 Hi Zoe, all, ----- Original Message ----- From: "zoe" Sent: Wednesday, February 18, 2009 > Moriyoshi Koizumi wrote: > > […] > --snip-- > > > […] > Hey all > > I'm sorry - I should have replied to this before since I was responsible > for raising #42868. I didn't do a good job at explaining what the issue > was in that bug, mainly because I didn't know what it was when I > started. The central problem is that PHP's behaviour on casting double > to int defaults to whatever the underlying C environment does. On > Windows and Linux (all of the versions that I looked at) this turns out > to be a simple truncation of the last 32 bits. Unfortunately the C > behaviour is 'undefined' (Kernigan and Ritchie, page 197, A6.3). The > issue that I found in #42868 was that on the Mac the casting behaviour > is completely different so many of the PHP tests failed. I believe that > PHP should behave in a platform independent way - that is what the fix > to #42868 achieves. It is also fair to say that any applications that > depend on the overflow behaviour in PHP 5.2 cannot be guaranteed to run > on any platform. (I'm glad this change was reverted for 5.2...) Anyway, like I said in my "5.3 todos" reply, I'll send a possible DVAL_TO_LVAL, etc. solution (consistent/reliable overflow across platforms) for consideration as soon as I can... There are other inconsistencies and behavior changes with the existing code, and it definitely doesn't behave in a platform-independent way! With the 4 versions of DVAL_...: old, 32 bit, 64 bit, 64 bit Windows (not sure why it's there; its longs are still 32 bit?), things I can think of: - 64-bit Windows has no range limiting for casting, so it's free to overflow like the old way...? Other 64 bit limits the range to LONG_MAX/MIN. 32 bit still casts between LONG_MAX and MAX_UNSIGNED_INT (ULONG_MAX-ish?), and, presumably, it's supposed to overflow between those 2, but casting of the out-of-range unsigned long is also "implementation-defined," so still cannot be guaranteed. - Conversion of INF/-INF: the old version and 64 bit Windows now will convert them to 0. 32 bit and other 64 bit will convert them to LONG_MAX/LONG_MIN. - With zend_parse_parameters(), a numeric string that results in a double is still converted using a simple (long) cast only. (In my proposed changes, I was going to add a new conversion specifier (simple few lines), like capital 'L', for functions that want/need to limit the long without overflow, like lengths for some string functions that I *thought* was trying to be fixed orginally.) > Zoe - Matt --- ## #43094 — Moriyoshi Koizumi — 2009-02-18T14:35:55+00:00 URL: https://externals.io/message/43021#43094 · In reply to: #43089 To summarize what were the problems: 1. casting a float value that is unrepresentable in a target type is undefined according to C spec. 2. any constant values that are unrepresentable in the standard integer type are automagically represented as double values in PHP. i.e. 0xc0000000 will result in a double value in the 32bit architecture. 3. PHP's cast behavior used to rely on the C cast, which was addressed in #42868 and fixed for 5.3.x and later. 4. PHP's associative array allows strings and integers for its keys. 5. when a double value is fed to an array as the key, it is converted to integer using the standard C cast. 6. bug #46701 addresses the problem that the result of 6. varies by the architecture. 7. the patch for bug #46701 uses DVAL_TO_LVAL to get the consistent cast behavior. 8. therefore, it's natural to consider bug #46701 is based on the behavior that is achieved by the patch for bug #42868. 9. the patch for bug #42868 may affect existing applications that rely on the old behavior. (http://marc.info/?l=php-internals&m=120799720922202&w=2) 10. the change like 9. is not tolerable in a minor release. 11. if bug #42868 is not going to be merged, then bug #46701 should be reverted from the 5.2 branch. Moriyoshi > Moriyoshi Koizumi wrote: > > […] > > Hey all > > I'm sorry - I should have replied to this before since I was responsible for > raising #42868. I didn't do a good job at explaining what the issue was in > that bug, mainly because I didn't know what it was when I started. The > central problem is that PHP's behaviour on casting double to int defaults to > whatever the underlying C environment does. On Windows and Linux (all of the > versions that I looked at) this turns out to be a simple truncation of the > last 32 bits. Unfortunately the C behaviour is 'undefined' (Kernigan and > Ritchie, page 197, A6.3). The issue that I found in #42868 was that on the > Mac the casting behaviour is completely different so many of the PHP tests > failed. I believe that PHP should behave in a platform independent way - > that is what the fix to #42868 achieves. It is also fair to say that any > applications that depend on the overflow behaviour in PHP 5.2 cannot be > guaranteed to run on any platform. > > Zoe > > […] --- ## #43170 — Iain Lewis — 2009-02-25T13:25:14+00:00 URL: https://externals.io/message/43021#43170 · In reply to: #43091 Matt Wilmas wrote: > > (I'm glad this change was reverted for 5.2...) > > Anyway, like I said in my "5.3 todos" reply, I'll send a possible > DVAL_TO_LVAL, etc. solution (consistent/reliable overflow across platforms) > for consideration as soon as I can... There are other inconsistencies and > behavior changes with the existing code, and it definitely doesn't behave in > a platform-independent way! With the 4 versions of DVAL_...: old, 32 bit, > 64 bit, 64 bit Windows (not sure why it's there; its longs are still 32 > bit?), things I can think of: > > - Matt > Longs are still 32bit on 64bit windows, which explains the special case for windows. I'd be interested to see your possible solution to this. Having consistent behaviour across platorms/32bit/64bit would be great from my p.o.v, as writing tests for the current behaviour isn't fun at all! Iain --- ## #43171 — Pierre Joye — 2009-02-25T13:40:26+00:00 URL: https://externals.io/message/43021#43171 · In reply to: #43170 > Matt Wilmas wrote: > > > […] > > Longs are still 32bit on 64bit windows, which explains the special case for > windows. That's correct. long is always 32bit on windows. See: http://msdn.microsoft.com/en-us/library/s3f49ktz.aspx and http://msdn.microsoft.com/en-us/library/cc953fe1.aspx > I'd be interested to see your possible solution to this. Having consistent > behaviour across platorms/32bit/64bit would be great from my p.o.v, as > writing tests for the current behaviour isn't fun at all! Something on our TODO is to use fixed and portable size types like what we can find in stdint. I have put a windows version in win32/php_stdint.h if you are interested. Ideally we should do that for all platforms and make them available for all parts of php (extensions, engine or main). Cheers, -- Pierre http://blog.thepimp.net | http://www.libgd.org