Bug #22510: Working toward a fix...

Hi:

I believe there's a missing refcount increment causing this bug, and I
think I may have uncovered some evidence that could get us closer to a
fix. I've added opcode dumps to zend_execute.c with a well-placed perl
command, so as to see the instructions as they're executed.

All of the following has been performed against a clean snapshot of PHP4
CVS HEAD.

Given the following script, containing one class with two methods
differing only by a silence operator:

<?php
class foo
{
function &method1() {
return $this->foo;
}

  function &method2() {
    return @$this->foo;
  }
}

$i = new foo();
$i->method1();
echo "\n";
$i->method2();

?>

The following opcodes pass throuch the main switch() in execute(), in
this order:

ZEND_NOP
ZEND_NEW
ZEND_JMP_NO_CTOR
ZEND_FETCH_W
ZEND_FETCH_LOCAL
ZEND_ASSIGN
ZEND_FETCH_W
ZEND_FETCH_LOCAL
ZEND_INIT_FCALL_BY_NAME
ZEND_DO_FCALL_BY_NAME
ZEND_FETCH_W
ZEND_FETCH_LOCAL
ZEND_FETCH_OBJ_W
ZEND_RETURN
ZEND_ECHO
ZEND_FETCH_W
ZEND_FETCH_LOCAL
ZEND_INIT_FCALL_BY_NAME
ZEND_DO_FCALL_BY_NAME
ZEND_BEGIN_SILENCE
ZEND_FETCH_R
ZEND_FETCH_LOCAL
ZEND_FETCH_OBJ_R
ZEND_END_SILENCE
ZEND_RETURN
ZEND_RETURN

Here, we see the object property which will eventually become the return
value of method1() being fetched by the engine with ZEND_FETCH_OBJ_W,
which triggers a zend_fetch_property_address(..., BP_VAR_W) from
zend_execute.c:1354, eventually bumping the reference count in
zend_fetch_property_address_inner:

case BP_VAR_W: {
zval *new_zval = &EG(uninitialized_zval);
new_zval->refcount++;
zend_hash_update(ht, prop_ptr->value.str.val, ...snip...);
}

However, it seems the addition of the silence operator (which wouldn't
have an effect on the read/write status of the fetch or the refcount, I
wouldn't think) makes the engine fetch with ZEND_FETCH_OBJ_R; eventually
triggering another branch of the switch in zend_get_property_address_inner:

case BP_VAR_R:
zend_error(E_NOTICE,"Undefined property: %s", prop_ptr->value.str.val);
/* break missing intentionally */
case BP_VAR_IS:
retval = &EG(uninitialized_zval_ptr);
break;

Which omits the increment of the reference count. There is a call to
AI_USE_PTR() on the read path that isn't on the write path, but that
looks like it doesn't touch the refcount of anything.

So, anyway.

I may be completely off in left-field here; apologies if you've made it
this far and none of this represents any sort of sane thinking. :) But,
is it possible that the silence operator, when applied to a fetch of an
object property that is returned by reference, is being miscompiled as
ZEND_FETCH_OBJ_R rather than ZEND_FETCH_OBJ_W?

(Be gentle with replies; this is the first time I've ever really touched
the Zend Engine's guts. I've appended this to the entry on bugs.php.net
as well.)

Thanks in advance,

• Dave
  dave@codewhore.org