On 12/21/18 3:12 AM, Levi Morrison wrote: > On Thu, Dec 20, 2018 at 3:35 PM Dmitry Stogov <dmi...@zend.com> wrote: >> >> Hi Levi, >> >> >> It looks like the patch broke something related to opcache. >> >> It crashes at least on Wordpress and Drupal. >> >> The backtrace >> https://gist.github.com/dstogov/a2305381a5c9982cceca9e4e252d26c7 shows >> use-after-free in opcache (works fine with master). >> >> Inability to work with opcache, doesn't allow to check the performance >> impact. >> >> >> It also broke few tests. Some crash. Some produce different warning/errors. >> >> >> $ make test TESTS="Zend tests" >> >> ... >> >> ===================================================================== >> FAILED TEST SUMMARY >> --------------------------------------------------------------------- >> ZE2 ArrayAccess and ArrayAccessReferenceProxy with references to main array >> [tests/classes/array_access_011.phpt] >> Bug #21478 (Zend/zend_alloc.c :: shutdown_memory_manager produces segfault) >> [Zend/tests/bug21478.phpt] >> Generator methods can yield by reference >> [Zend/tests/generators/generator_method_by_ref.phpt] >> Testing to implement Serializable interface by traits >> [Zend/tests/traits/interface_003.phpt] >> Handling of public fields with traits needs to have same semantics as with >> normal inheritance, however, we do add strict warnings since it is easier to >> run into something unexpeted with changing traits. >> [Zend/tests/traits/property009.phpt] >> iterable type#004 - Parameter covariance >> [Zend/tests/type_declarations/iterable_004.phpt] >> iterable type#005 - Return type covariance >> [Zend/tests/type_declarations/iterable_005.phpt] >> ===================================================================== >> >> I'll try to play with patch and make a full code review on next week. >> >> >> It would be great, if you fix opcache compatibility. >> >> If it can't be done in reasonable time, it's probably better to cancel >> voting and restart when ready. > > What OS and compiler are these on?
Linux (Fedora 28), GCC 8.2.1. > How are you ensuring that opcache > is on when these tests are run? I have not been experiencing these > issues, so maybe I am not running it correctly. If I cannot reproduce > them soon then I will agree to cancel the voting. You should enable opcache in your php.ini zend_extension=opcache.so opcache.enable=1 opcache.enable_cli=1 opcache.optimization_level=-1 opcache.protect_memory=1 ; this is for testing only Memory corruption bugs may lead to crash or not because of "luck", but it's possible to see them using valgrind. $ make test TESTS="-m tests/classes/array_access_011.phpt" $ cat tests/classes/array_access_011.mem This shows almost the same backtrace, as I already published. Looks like an incorrect reference-counting on some string. > > There are some known issues outside of Zend. Notably some internal > classes do not have valid method signatures with regards to > inheritance which this patch exposed. These need fixed regardless of > this RFC and I have begun work on some of them (see > https://github.com/php/php-src/pull/3686 for one example). That PR looks fine. There is also a problem that this PR has merge conflict with master and travis doesn't run tests. Thanks. Dmitry.
