Bug #70173 [Com]: ZVAL_COPY_VALUE_EX broken for 32bit Solaris Sparc

Edit report at https://bugs.php.net/bug.php?id=70173&edit=1 ID: 70173 Comment by: rainer dot jung at kippdata dot de Reported by: rainer dot jung at kippdata dot de Summary: ZVAL_COPY_VALUE_EX broken for 32bit Solaris Sparc Status: Analyzed Type: Bug Package: Scripting Engine problem Operating System: Solaris 10 Sparc PHP Version: 7.0.0beta3 Block user comment: N Private report: N New Comment: Where do I find "PR #1464"? Is there a typo (the number is very small). I can test any suggested change. Previous Comments: ------------------------------------------------------------------------ [2015-08-09 13:15:57] cmb@php.net Thanks, Rainer, for the thorough analysis. AIUI the portable data layout (ZEND_ENDIAN_LOHI) allows for more efficient operations[1]. PR #1464 is supposed to solve the issue. I do not have a big-endian machine at hand, so I can't test it, though. [1] <https://github.com/php/php-src/commit/d8099d0468426dbee59f540048376653535270ce> ------------------------------------------------------------------------ [2015-08-09 04:04:26] rainer dot jung at kippdata dot de I removed the ZEND_ENDIAN_LOHI in front of w1, w2 and now the test suite results are in line with the 5.6 ones. I really don't see a reason, why the ZEND_ENDIAN_LOHI should be there. I always expect the struct layout to have lval and dval start at the lower address, as well as w1 when ZEND_ENDIAN_LOHI is removed. So for a 32 Bit build one then would always need to copy w2 in addition, which is what the code currently does. Whether w1 resp. w2 contain least significant bits or most significant bits soesn't matter, als long as bot are copied (and not used to interprete the two halves of the 64 bits individually). If you really think you need the ZEND_ENDIAN_LOHI, then you also need to switch the additional copying of w2 to w1 instead for big endian systems. ------------------------------------------------------------------------ [2015-08-08 21:53:23] rainer dot jung at kippdata dot de I extracted a C standalone reproduction case from the PHP files. I will attach it. The zval.value layout on this playform is such, that lval and dval both start at the lower address but dval extends above lval. Now because of ZEND_ENDIAN_LOHI in the definition of ww, the address of w2 is the low one (same as lval and dval), and the address of w1 is the high one. Copying in ZVAL_COPY_VALUE copies the 32 bit contents of the low address, and then additionally z->value.ww.w2 = _w2, which is again the low hald of dval. So the high half stays uninitialized resp. 0. So for this platform the use of ZEND_ENDIAN_LOHI doesn't work as expected. I will attach a small C program to show the problem. It can be compiled standalone on Solaris Sparc (I used gcc), no dependencies needed. I will attach it as a patch, because I don't see any other way of attaching source files. ------------------------------------------------------------------------ [2015-08-08 20:57:19] rainer dot jung at kippdata dot de Changed Summary, because I narrowed down the root cause. Unfortunately there is no "Zend" in the issue tracker "Package" drop down. ------------------------------------------------------------------------ [2015-08-08 20:53:59] rainer dot jung at kippdata dot de OK, I used the debugger. The problem is inside ZVAL_COPY_VALUE_EX. Only half of the double is being copied, only 32 bits of the 64 bits. ZVAL_COPY_VALUE_EX is meant to fix this by using the "#if SIZEOF_SIZE_T == 4" definition of it it additionally copies value.ww.w2. But on Solaris Sparc, value.ww.w2 has already been copied and what is missing is value.ww.w1 ! For some reason the struct layout is not the expected one, w1 and w2 have the wrong order. It seems the ZEND_ENDIAN_LOHI() logic for w1, w2 resp. the refcount field which is used in the basic copying doesn't work on sparc. It is a big endian platform and WORDS_BIGENDIAN is defined, but is nevertheless doesn't work. I hope that's enough debugging for you guys to be able to go the last mile. I can test on the target platform whatever change you want me to. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=70173 -- Edit this bug report at https://bugs.php.net/bug.php?id=70173&edit=1