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
 [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

Add a Patch

Pull Requests

Add a Pull Request

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-2024 The PHP Group
All rights reserved.
Last updated: Thu Apr 25 05:01:33 2024 UTC