Bug #66337 [Asn]: Wrong size calculation on optimization of class constants

Edit report at https://bugs.php.net/bug.php?id=66337&edit=1 ID: 66337 User updated by: Terry at ellisons dot org dot uk Reported by: Terry at ellisons dot org dot uk Summary: Wrong size calculation on optimization of class constants Status: Assigned Type: Bug Package: opcache Operating System: N/A PHP Version: master-Git-2013-12-22 (Git) Assigned To: dmitry Block user comment: N Private report: N New Comment: Sorry my bad. You can close this one: the test works, now that I've synced my dev config to current PHP-5.6. One of the updates to 5.6 in the last few has already fixed this. Previous Comments: ------------------------------------------------------------------------ [2013-12-23 13:12:31] Terry at ellisons dot org dot uk OK, I'll refresh my dev snapshot from GiT -- it's a week out of date. This might have been separately fixed by Xinchen. If so it's a pity because it took quite a few hours to locate the exact failure :-( Let me examine and post back. ------------------------------------------------------------------------ [2013-12-23 12:23:20] dmitry@php.net In my opinion optimizer shouldn't create interned strings. ------------------------------------------------------------------------ [2013-12-23 12:22:19] dmitry@php.net I can't reproduce this. In my opinion ------------------------------------------------------------------------ [2013-12-22 23:42:02] Terry at ellisons dot org dot uk OK, the peephole optimizer for references to string class constants replaces the instruction sequence: FETCH_CONSTANT ~n 'SomeClass', 'CONSTANT' ASSIGN !n, ~m with ASSIGN !0, Interned_string(Value of SomeClass::CONSTANT) The while loop at zend_persist.c:263 calls zend_persist_zval() which is effectively a NOOP for existing interned strings. On the otherhand the corresponding loop at zend_persist_calc.c:172 calls zend_persist_zval_calc() to compute the size of the literal string. This invokes the ADD_INTERNED_STRING() macro at zend_persist_calc.c:118. However this logic is flawed is the string is ALREADY interned and it therefore incorrectly adds the size of the string to the computed size. This triggers the subsequent "Wrong size calculation" warning. I won't include the obvious fix here because I suspect that this exposes a couple of separate but related bugs which I want to explore and raise in their own bugreps. ------------------------------------------------------------------------ [2013-12-22 18:07:22] Terry at ellisons dot org dot uk Description: ------------ I've been trying the current 5.6 against apps such as Mediawiki and was getting Wrong size calculation errors logged. After some horrible debugging I've reduced this down to a simple test case as attached. When I run this: Sun Dec 22 18:04:53 2013 (11692): Warning Internal error: wrong size calculation: /tmp/Terry_006.inc start=0x258a88d0, end=0x258a9088, real=0x258a9078 Now that I've got a simple repeatable test case, I'll degub the bug and post a patch. Test script: --------------- --TEST-- Wrong size calculation on optimization of class constants --INI-- opcache.enable=1 opcache.enable_cli=1 opcache.optimization_level=-1 opcache.file_update_protection=0 --SKIPIF-- <?php if (!extension_loaded('Zend OPcache') || php_sapi_name() != "cli") die("skip CLI only"); ?> --FILE-- <?php class X { const A = "Constant"; } $file = str_replace( ".php", ".inc", __file__ ); file_put_contents( $file, '<?php class B { function __constuct() { $a = X::A; } }'); require $file; new B; @unlink($file); echo "done\n"; ?> --EXPECT-- done ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=66337&edit=1