Bug #66440 [NEW]: Optimisation of conditional JMPs based on pre-evaluate constant function calls

From: Date: Wed, 08 Jan 2014 13:19:39 +0000
Subject: Bug #66440 [NEW]: Optimisation of conditional JMPs based on pre-evaluate constant function calls
Groups: php.bugs 
Request: Send a blank email to php-bugs+get-183643@lists.php.net to get a copy of this message
From:             Terry at ellisons dot org dot uk
Operating system: N/A
PHP version:      master-Git-2014-01-08 (Git)
Package:          opcache
Bug Type:         Bug
Bug description:Optimisation of conditional JMPs based on pre-evaluate constant function calls

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 bug report at https://bugs.php.net/bug.php?id=66440&edit=1
-- 
Try a snapshot (PHP 5.4):   https://bugs.php.net/fix.php?id=66440&r=trysnapshot54
Try a snapshot (PHP 5.5):   https://bugs.php.net/fix.php?id=66440&r=trysnapshot55
Try a snapshot (trunk):     https://bugs.php.net/fix.php?id=66440&r=trysnapshottrunk
Fixed in SVN:               https://bugs.php.net/fix.php?id=66440&r=fixed
Fixed in release:           https://bugs.php.net/fix.php?id=66440&r=alreadyfixed
Need backtrace:             https://bugs.php.net/fix.php?id=66440&r=needtrace
Need Reproduce Script:      https://bugs.php.net/fix.php?id=66440&r=needscript
Try newer version:          https://bugs.php.net/fix.php?id=66440&r=oldversion
Not developer issue:        https://bugs.php.net/fix.php?id=66440&r=support
Expected behavior:          https://bugs.php.net/fix.php?id=66440&r=notwrong
Not enough info:            https://bugs.php.net/fix.php?id=66440&r=notenoughinfo
Submitted twice:            https://bugs.php.net/fix.php?id=66440&r=submittedtwice
register_globals:           https://bugs.php.net/fix.php?id=66440&r=globals
PHP 4 support discontinued: https://bugs.php.net/fix.php?id=66440&r=php4
Daylight Savings:           https://bugs.php.net/fix.php?id=66440&r=dst
IIS Stability:              https://bugs.php.net/fix.php?id=66440&r=isapi
Install GNU Sed:            https://bugs.php.net/fix.php?id=66440&r=gnused
Floating point limitations: https://bugs.php.net/fix.php?id=66440&r=float
No Zend Extensions:         https://bugs.php.net/fix.php?id=66440&r=nozend
MySQL Configuration Error:  https://bugs.php.net/fix.php?id=66440&r=mysqlcfg



Thread (3 messages)

« previous php.bugs (#183643) next »