php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #42868 Floats cast to integer produce unpredicatable results.
Submitted: 2007-10-05 15:26 UTC Modified: 2007-12-17 14:17 UTC
From: zoe dot slattery at googlemail dot com Assigned:
Status: Closed Package: Scripting Engine problem
PHP Version: 5CVS-2007-10-05 (snap) OS: Linux/Windows/OSX
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: zoe dot slattery at googlemail dot com
New email:
PHP Version: OS:

 

 [2007-10-05 15:26 UTC] zoe dot slattery at googlemail dot com
Description:
------------
I'm not sure if this is one defect or two (or none:-)) - it would be good to understand the expected behaviour as there are a number of other MAC OSX test failures. 

(1) strspn() does not return consistent values on Linux/Windows if "$start"> PHP_INT_MAX

(2) The values returned on MAC OSX (10.4) are different to the values returned on Linux & Windows.

It's reasonably easy to see what is happening in the Windows/Linux case. The double that is passed to strspn() is cast as a long in zend_parse_parameters, and in some cases this means that the resulting long happens to be within a reasonable range (see comments in reproduce code).

From the users' point of view it would be better if strspn() returned the same thing for all values of $start which are outside the expected range.

I'm not sure why the behaviour is different on Mac, it appears that what comes back from zend_parse_parameters is "-1" in example 3 & 4.

Reproduce code:
---------------
<?php
$str = '2468 who do we appreciate';
$mask = '1234567890';

//Linux result, after zend_parse_parameters start = 2147483647, expect bool(false)
//Mac result, start = 2147483647, bool(false)
$start = PHP_INT_MAX;
var_dump( strspn($str,$mask,$start) );

//Linux result, after zend_parse_parameters start = -23, expect bool(false)?, get int(2)
//Mac result, start = -23, int(2)
$start = PHP_INT_MAX * 2 - 21;
var_dump( strspn($str,$mask,$start) );

//Linux result, after zend_parse_parameters start = 0, expect bool(false)?, get int(4)
//Mac result, start = -1, int(0)
$start = PHP_INT_MAX * 2 + 2;
var_dump( strspn($str,$mask,$start) );

//Linux result, after zend_parse_parameters start = -3, expect bool(false)?, get int(0)
//Mac result, start = -1, int(0)
$start = PHP_INT_MAX * 4 + 1;
var_dump( strspn($str,$mask,$start) );
?>


Expected result:
----------------
I'd expect to see
(1) the same result for every invalid value of $start on (Win/Lin)
(2) the same result on Mac as I see on Win/Lin

Actual result:
--------------
Lin/Win

bool(false)
int(2)
int(4)
int(0)

Mac
bool(false)
int(2)
int(0)
int(0)

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2007-10-31 21:55 UTC] zoe dot slattery at googlemail dot com
Hi - I'm appending a fix for this bug - it's not elegant. The cause
of this bug (and many similar bugs) is in the code in
zend_operators.c which casts doubles to integers. I see that someone already
tried to fix this code a couple of years ago, but the fix had to be removed
(see defect 30695).
I suppose it might be possible to special case
0x8000000 - 0xFFFFFFF which might avoid the problem described in 30695.
In the meantime here is a fix to string.c which gets around it
for this particular instance.

If someone commits this - or something like it - the test cases will
need changing, I'm happy to do that.

#P php53_dev
Index: ext/standard/string.c
===================================================================
RCS file: /repository/php-src/ext/standard/string.c,v
retrieving revision 1.445.2.14.2.69.2.5
diff -u -r1.445.2.14.2.69.2.5 string.c
--- ext/standard/string.c       7 Oct 2007 05:22:07 -0000       1.445.2.14.2.69.2.5
+++ ext/standard/string.c       31 Oct 2007 21:28:35 -0000
@@ -207,14 +207,25 @@
 {
        char *s11, *s22;
        int len1, len2;
+       double startf;
        long start, len;

        start = 0;
        len = 0;
-       if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss|ll", &s11, &len1,
-                               &s22, &len2, &start, &len) == FAILURE) {
+       startf = 0;
+       if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss|dl", &s11, &len1,
+                               &s22, &len2, &startf, &len) == FAILURE) {
                return;
        }
+       // Check to make sure that the start length isn't greater than the maximum integer
+        // or smaller than the minimum integer
+        if(startf > INT_MAX || startf < INT_MIN) {
+                RETURN_FALSE;
+        } else {
+                start = (long) startf;
+        }
+
+

        if (ZEND_NUM_ARGS() < 4) {
                len = len1;
 [2007-11-02 12:13 UTC] zoe dot slattery at googlemail dot com
Just in case anyone is looking at this, I've been working with Andy Wharmby on trying to find a better way to fix than the one I proposed. If we can find a better fix it will be something that I will want to test a lot, so may take a couple of weeks.
 [2007-11-08 10:23 UTC] zoe dot slattery at googlemail dot com
Here is a fix and a test. I've tested on Mac and Linux (32). I can't test the Win64 behaviour. 

There are a number of tests which fail on Linux/Mac - but there are all recent tests and were passing the wrong behaviour. I'll fix them if this patch gets committed.

#P php53_dev
Index: Zend/zend_operators.c
===================================================================
RCS file: /repository/ZendEngine2/zend_operators.c,v
retrieving revision 1.208.2.4.2.23.2.2
diff -u -r1.208.2.4.2.23.2.2 zend_operators.c
--- Zend/zend_operators.c       29 Oct 2007 14:36:55 -0000      1.208.2.4.2.23.2.2
+++ Zend/zend_operators.c       8 Nov 2007 10:09:10 -0000
@@ -185,21 +185,37 @@
                                break;                                                                                                          \
                }                                                                                                                                       \
        }
-
+#define MAX_UNSIGNED_INT ((double) LONG_MAX * 2) + 1
 #ifdef _WIN64
 # define DVAL_TO_LVAL(d, l) \
        if ((d) > LONG_MAX) { \
-               (l) = (long)(unsigned long)(__int64) (d); \
+                if ((d) > MAX_UNSIGNED_INT) { \
+                        (l) = LONG_MAX; \
+                } else { \
+                       (l) = (long)(unsigned long)(__int64) (d); \
+               } \
        } else { \
-               (l) = (long) (d); \
+                if((d) < LONG_MIN) { \
+                        (l) = LONG_MIN; \
+                } else { \
+                        (l) = (long) (d); \
+                } \
        }
 #else
 # define DVAL_TO_LVAL(d, l) \
-       if ((d) > LONG_MAX) { \
-               (l) = (unsigned long) (d); \
-       } else { \
-               (l) = (long) (d); \
-       }
+        if ((d) > LONG_MAX) { \
+                if ((d) > MAX_UNSIGNED_INT) { \
+                        (l) = LONG_MAX; \
+                } else { \
+                        (l) = (unsigned long) (d); \
+                } \
+        } else { \
+                if((d) < LONG_MIN) { \
+                if((d) < LONG_MIN) { \
+                        (l) = LONG_MIN; \
+                } else { \
+                        (l) = (long) (d); \
+                } \
+        }
 #endif

 #define zendi_convert_to_long(op, holder, result)   

Here is a PHPT test case that can be used to verify the fix:

--TEST--
Test intval() function : testsing cast of float to int on 32 bit systems
--SKIPIF--
<?php
if (PHP_INT_SIZE != 4) die("skip this test is for 32bit platform only");
?>
--FILE--
<?php
/* Prototype  : proto int intval(mixed var [, int base])
 * Description: Get the integer value of a variable using the optional base for the conversion
 * Source code: ext/standard/type.c
 * Alias to functions:
 */

/*
 * A test to verify expected behaviour when floats are cast to integers
 */

echo "*** Testing intval() : float to int ***\n";


// Initialise all required variables
$var = array();
$var[0] = PHP_INT_MAX + 1;
$var[1] = PHP_INT_MAX + 2;
$var[2] = PHP_INT_MAX + 3;
$var[3] = PHP_INT_MAX * 2 - 1;
$var[4] = PHP_INT_MAX * 2;
$var[5] = PHP_INT_MAX * 2 + 1;
$var[6] = PHP_INT_MAX * 2 + 2;
$var[7] = PHP_INT_MAX * 2 + 3;
$var[8] = -PHP_INT_MAX - 2;
$var[9] = -PHP_INT_MAX - 1;
$var[10] = -PHP_INT_MAX;
$var[11] = -PHP_INT_MAX + 1;

foreach ($var as $val) {
        var_dump( intval($val) );
}

echo "Done";
?>
--EXPECTF--
*** Testing intval() : float to int ***
int(-2147483648)
int(-2147483647)
int(-2147483646)
int(-3)
int(-2)
int(-1)
int(2147483647)
int(2147483647)
int(-2147483648)
int(-2147483648)
int(-2147483647)
Done
 [2007-11-08 17:11 UTC] zoe dot slattery at googlemail dot com
Following Andy W's comments, the patch is here:
http://www.pastebin.ca/766125

The test case is here:
http://www.pastebin.ca/766128
 [2007-11-10 20:42 UTC] zoe dot slattery at googlemail dot com
Patch without tabs at:
http://www.pastebin.ca/768695

Or for raw file:
http://www.pastebin.ca/raw/768695
 [2007-12-17 14:17 UTC] dmitry@php.net
This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.


 
PHP Copyright © 2001-2025 The PHP Group
All rights reserved.
Last updated: Sat May 03 07:01:29 2025 UTC