carusogabriel · GitHub

EDIT: Never mind, my comments were already mentioned by others in the review comments for 5358 already > It looks like `exit` suffers from the same problem.

If valgrind doesn't warn about a memory leak, it's probably not a severe issue - optimizing bad php code like some_fn(exit()) isn't a high priority.

Also note that INIT_FCALL_BY_NAME would throw if the function was undefined, but INIT_FCALL should almost always have a defined function

; unoptimized
0000 V1 = NEW 1 string("Exception")
0001 V2 = NEW 1 string("Exception")
; optimized
0000 V1 = NEW 1 string("Exception")
0001 V1 = NEW 1 string("Exception")

A part of the problem is that the memory for V1 is getting reused, so the first incomplete object is lost track of. I don't know why opcache thought it was safe to reuse that temporary value - it might be specific to ZEND_THROW.

I see that throw new Exception(exit()) was also affected - I haven't tried with that patch but it looks like it should handle that

Actually, it already incorrectly optimizes exit(). It doesn't keep live range for result of the first NEW.

Before rebasing this branch

USE_ZEND_ALLOC=0 valgrind --leak-check=full which php -d zend_extension=opcache.so -d opcache.enable_cli=1 -d opcache.enable=1 -d opcache.opt_debug_level=0x20000 throw.php

0000 V0 = NEW 1 string("Exception")
0001 EXIT
==10313== 152 bytes in 1 blocks are definitely lost in loss record 30 of 38
==10313==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10313==    by 0x8A0998: __zend_malloc (in /path/to/bin/php)
==10313==    by 0x9114B9: zend_objects_new (in /path/to/bin/php)
==10313==    by 0x8FA217: zend_default_exception_new_ex (in /path/to/bin/php)
==10313==    by 0x8D87AF: object_init_ex (in /path/to/bin/php)
==10313==    by 0x94D7D0: ZEND_NEW_SPEC_CONST_UNUSED_HANDLER (in /path/to/bin/php)
==10313==    by 0x96B247: execute_ex (in /path/to/bin/php)
==10313==    by 0x974073: zend_execute (in /path/to/bin/php)
==10313==    by 0x8D6852: zend_execute_scripts (in /path/to/bin/php)
==10313==    by 0x861597: php_execute_script (in /path/to/bin/php)
==10313==    by 0x9766B2: do_cli (in /path/to/bin/php)
==10313==    by 0x464D24: main (in /path/to/bin/php)
(and a false positive for zend_accel_load_script)

Read the original on github.com ↗