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
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
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 mozo@mozo.jp
wrote: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--
Ilia Alshanetsky
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?
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--
Ilia Alshanetsky
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--
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
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
Regards,
Moriyoshi--
Ilia Alshanetsky
Hi Zoe, all,
----- Original Message -----
From: "zoe"
Sent: Wednesday, February 18, 2009
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--
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
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
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
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.
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
To summarize what were the problems:
- casting a float value that is unrepresentable in a target type is
undefined according to C spec. - 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. - PHP's cast behavior used to rely on the C cast, which was addressed
in #42868 and fixed for 5.3.x and later. - PHP's associative array allows strings and integers for its keys.
- when a double value is fed to an array as the key, it is converted
to integer using the standard C cast. - bug #46701 addresses the problem that the result of 6. varies by
the architecture. - the patch for bug #46701 uses DVAL_TO_LVAL to get the consistent
cast behavior. - therefore, it's natural to consider bug #46701 is based on the
behavior that is achieved by the patch for bug #42868. - 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) - the change like 9. is not tolerable in a minor release.
- if bug #42868 is not going to be merged, then bug #46701 should be
reverted from the 5.2 branch.
Moriyoshi
Moriyoshi Koizumi wrote:
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
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
Regards,
Moriyoshi--
Ilia Alshanetsky