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

Edit report at https://bugs.php.net/bug.php?id=66337&edit=1 ID: 66337 Updated by: dmitry@php.net Reported by: Terry at ellisons dot org dot uk Summary: Wrong size calculation on optimization of class constants Status: Not a bug 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: I still can't reproduce this. $ sapi/cli/php -n -d zend_extension="opcache.so" -d opcache.enable_cli=1 Zend/tests/heredoc_015.phpt You probably right, about the condition change, but how did you get interned string there? Previous Comments: ------------------------------------------------------------------------ [2013-12-30 10:08:53] Terry at ellisons dot org dot uk My bad**2. Xinchen's fix removed this manifestation, but the underlying bug is still there. I've just done a make test on the current PHP-56 and 51 tests fail with this (see below). In PHP 5.6 zend_accel_store_interned_string() does not bump ZCG(mem) because interned strings donn't require a compiled script zval, however the corresponding macro in zend_persist_calc.c bumps memory_used by 8. This is because the logic of ADD_INTERNED_STRING(str, len) is wrong. If str is already interned then there is nothing to do, but in the current macro in this case tmp == str so sizeof(zval *) is added to memory_used. Surely this should be: --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -31,7 +31,16 @@ #define ADD_SIZE(m) memory_used += ZEND_ALIGNED_SIZE(m) #define RETURN_SIZE() return memory_used -#if ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO +#if ZEND_EXTENSION_API_NO > PHP_5_5_X_API_NO +# define ADD_INTERNED_STRING(str, len) do { \ + const char *tmp = accel_new_interned_string((str), (len), !IS_INTERNED((str)) TSRMLS + if (tmp != (str)) { \ + (str) = (char*)tmp; \ + } else if (!IS_INTERNED(str)) { \ + ADD_DUP_SIZE((str), (len)); \ + } \ + } while (0) +#elif ZEND_EXTENSION_API_NO > PHP_5_3_X_API_NO # define ADD_INTERNED_STRING(str, len) do { \ const char *tmp = accel_new_interned_string((str), (len), !IS_INTERNED((str)) TSRMLS if (tmp != (str)) { \ This certainly fixed the 51 failing tests. The existing allocation logic works for PHP-5.3 - PHP-5.5 so I've only made the change for 5.6 == List of tests which fail == Zend/tests heredoc_008.phpt heredoc_015.phpt heredoc_016.phpt nowdoc_008.phpt nowdoc_015.phpt nowdoc_016.phpt ns_024.phpt ns_069.phpt Zend/tests/traits trait_constant_002.phpt ext/standard/tests/array array_filter_variation5.phpt array_flip_variation2.phpt array_flip_variation3.phpt array_rand_variation6.phpt array_unshift_variation9.phpt shuffle_variation5.phpt uasort_variation3.phpt uasort_variation5.phpt usort_variation5.phpt ext/standard/tests/file fscanf_variation14.phpt ext/standard/tests/general_functions is_string.phpt strval.phpt ext/standard/tests/strings addslashes_variation2.phpt chop_variation3.phpt chunk_split_variation12.phpt chunk_split_variation4.phpt htmlspecialchars_decode_variation3.phpt lcfirst.phpt sprintf_variation15.phpt str_replace_variation3.phpt str_split_variation5.phpt strcspn_variation5.phpt strcspn_variation6.phpt strcspn_variation7.phpt strcspn_variation8.phpt strip_tags_variation5.phpt stripos_variation7.phpt stripslashes_variation2.phpt strrchr_variation8.phpt strrpos_variation7.phpt strspn_variation5.phpt strspn_variation6.phpt strspn_variation7.phpt strspn_variation8.phpt strtok_variation3.phpt substr_count_variation_002.phpt ucfirst.phpt ucwords_variation2.phpt vfprintf_variation7.phpt vprintf_variation7.phpt vsprintf_variation7.phpt ------------------------------------------------------------------------ [2013-12-23 16:46:53] dmitry@php.net It seems like it's a false alarm or the bug is already fixed. ------------------------------------------------------------------------ [2013-12-23 16:32:44] Terry at ellisons dot org dot uk 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. ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ 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=66337 -- Edit this bug report at https://bugs.php.net/bug.php?id=66337&edit=1