Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:2114
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Sat, 31 May 2003 19:36:11 -0400
To: PHP Developers List <internals@lists.php.net>
Message-ID: <20030531233611.GA2916@codewhore.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.4i
Sender: David Brown <dave@lan.codewhore.org>
Subject: Bug #22510: Working toward a fix...
From: dave@codewhore.org (David Brown)

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