ID: 42868 User updated by: zoe dot slattery at googlemail dot com Reported By: zoe dot slattery at googlemail dot com Status: Open Bug Type: Scripting Engine problem Operating System: Linux/Windows/OSX PHP Version: 5CVS-2007-10-05 (snap) New Comment:
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 Previous Comments: ------------------------------------------------------------------------ [2007-11-02 12:13:22] 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-01 09:30:01] zoe dot slattery at googlemail dot com Sorry - fixed now I hope ------------------------------------------------------------------------ [2007-10-31 22:56:02] [EMAIL PROTECTED] Could you please fix the summary line? Plain "zoe" is not very descriptive. :) ------------------------------------------------------------------------ [2007-10-31 21:55:46] 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-10-05 15:26:19] 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) ------------------------------------------------------------------------ -- Edit this bug report at http://bugs.php.net/?id=42868&edit=1