Bug #66440 [Asn->Csd]: Optimisation of conditional JMPs based on pre-evaluate constant function calls
Edit report at https://bugs.php.net/bug.php?id=66440&edit=1
ID: 66440
Updated by: dmitry@php.net
Reported by: Terry at ellisons dot org dot uk
Summary: Optimisation of conditional JMPs based on
pre-evaluate constant function calls
-Status: Assigned
+Status: Closed
Type: Bug
Package: opcache
Operating System: N/A
PHP Version: master-Git-2014-01-08 (Git)
Assigned To: dmitry
Block user comment: N
Private report: N
New Comment:
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=9b74dc4a07a2c5178cbedc41b0beedd979537ffa
Log: Fixed bug #66440 (Optimisation of conditional JMPs based on pre-evaluate constant function
calls)
Previous Comments:
------------------------------------------------------------------------
[2014-01-08 14:49:27] Terry at ellisons dot org dot uk
I've just done a quick fix, but this only addresses this single dangerous use of
literal. The wider review is a separate issue.
_
--- a/Optimizer/pass2.c
+++ b/Optimizer/pass2.c
@@ -108,7 +108,7 @@ if (ZEND_OPTIMIZER_PASS_2 & OPTIMIZATION_LEVEL) {
if (opline->opcode == ZEND_JMPZ) {
should_jmp = !should_jmp;
}
- literal_dtor(&ZEND_OP1_LITERAL(opline));
+ Z_TYPE(ZEND_OP1_LITERAL(opline)) = IS_NULL;
ZEND_OP1_TYPE(opline) = IS_UNUSED;
if (should_jmp) {
opline->opcode = ZEND_JMP;
------------------------------------------------------------------------
[2014-01-08 13:19:38] Terry at ellisons dot org dot uk
Description:
------------
I was hitting this memory error in run-test.php itself if run with optimization enabled. So I used
gdb to work out what in this script was being optimized at the error and used this to generate the
attached minimal PHPT test which show the error.
The error is caused by the coupling of two factors:
* pass1_5.c:435 calls zend_get_persistent_constant() to obtain the constant value. However, this
routine (block_pass.c:11&16) look this symbol up in EG(zend_constants) and return this value.
This is the wrong copy to use as it is not interned within OPcache. In fact
accel_use_shm_interned_strings() has already cloned all EG(zend_constants) into
ZCSG(interned_strings), and it is this copy that should be referenced -- for example by passing the
return value through accel_new_interned_string().
* pass2.c:106 The JMPZ, JMPNZ processing optimizes the conditional into a absolute JMP or NOP so
that block_pass can remove the dead code. However it also does a
literal_dtor(&ZEND_OP1_LITERAL(opline)) on the now unused literal. This executes
_zval_dtor_func() on the zval which then str_efree_rel() the string, which is a bit of a disaster
since it wasn't emalloced in the first place.
I really need to question the logic for the widescale use of literal_dtor(). What are you trying to
do here? Surely this is just a case of two mistakes sort of cancelling each other out. All
literals should have been interned by this stage so doing a zval_dtor() is unnecessary and efreeing
string which in is_ref zvals with rc > 1 is crazy anyway. Yes the literal slots need to be
compacted out in compact_literals.c, but isn't setting Z_TYPE_P(zv) to IS_NULL adequate here?
So one simple but to fix and a review of the use of literal_dtor() is needed
Test script:
---------------
--TEST--
Check pre-evaluate constant function call on branch optimization
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.optimization_level=-1
--SKIPIF--
<?php require_once('skipif.inc'); ?>
--FILE--
<?php
if(constant('PHP_BINARY')) {
echo "OK\n";
}
?>
--EXPECTF--
OK
Expected result:
----------------
As per phpt
Actual result:
--------------
Generates a memory corruption error on debug build of php, e.g.
---------------------------------------
./Optimizer/pass2.c(111) : Block 0x01fa85e0 status:
/home/terry/work/php56/Zend/zend_variables.c(37) : Actual location (location was relayed)
Invalid pointer: ((size=0x00000041) != (next.prev=0x7265742f7261762f))
Invalid pointer: ((prev=0x00000008) != (prev.size=0x455a4953))
---------------------------------------
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=66440&edit=1
Thread (3 messages)