At 14:56 22/07/2003, Sascha Schumann wrote:
> Can you give me a concrete example of when you think this macro doesn't
> behave properly?  If it's correct, I'll try to fix it.  Note that the last
> time you had problems with this (and other) macros these were actually bugs
> in the session module.

    That was another instance showing the inherent lack of
    abstraction in this particular API.

Well, it was more of a misunderstanding on your part as to what you may and may not do with zvals which are managed by the engine. Apparently, this is the very same issue right now.


  What should be a black
    box is a test of familiarity with implementation details.

My English parser stuck on that one...


        <?php
        $foo = $bar = $a;
        @session_start();
        $_SESSION['foo'] = $_SESSION['bar'] = $a;

    All four zvals are initialized using an undefined variable.
    During the shutdown phase, this occurs:

zval **val;

ht = Z_ARRVAL_P(PS(http_session_vars));

        zend_hash_find(&EG(symbol_table), "foo", &val);
        ZEND_SET_SYMBOL_WITH_LENGTH(ht, "foo", *val,
                    (*val)->refcount + 1 , 1);

zend_hash_find(&EG(symbol_table), "bar", &val);

        /* crash in FREE_ZVAL */
        ZEND_SET_SYMBOL_WITH_LENGTH(ht, "bar", *val,
                    (*val)->refcount + 1 , 1);

From a quick glance it appears to be the very same bug I told you about a few months ago. Here's what I said back then:


---
The source of the problem is this:

1. You fetch a value from the symbol table that has is_ref=0, and refcount>1. EG(uninit..) is quite a common example of that, but it's definitely not unique - $foo = $bar = "baz"; will create such a beast too.

2. You tell zend_set_hash_symbol to make this value a reference. That in itself is a bug - you most probably want to attach only to one symbol, and not to other symbols who might be pointing to the same value.

3. Things really turn for the worse when EG(uninit..) becomes is_ref - that may cause all sorts of unexpected problems.

What should be done is SEPARATE_ZVAL() on the symbol, prior to calling zend_set_hash_symbol(). As a matter of fact, chances are that we'd want SEPARATE_ZVAL() to be inside zend_set_hash_symbol() - I can't imagine a situation where we'd be in a position to enable/disable the is_ref bit arbitrarily without separating first.
---


ZEND_SET_SYMBOL_WITH_LENGTH() was not designed to handle zvals which are already managed by the engine, but to introduce new zvals (you can see that it's being used by the various SET_VAR_*() macros, and was actually introduced to abstract them). It overwrites certain properties, such as the reference count and is_ref without taking into account the values that were in there before.

For the record, there's nothing special about uninitialized_zval as far as these macros are concerned, even though doing the wrong thing on uninitialized_zval may have a more noticeable effect. A reference count higher than 1 with is_ref being 0 is perfectly normal, and like I told you last time around - can happen with things as simple as $a=$b="foo"; Changing such a value to is_ref=1 (i.e., calling ZEND_SET_SYMBOL on that value, and providing is_ref=1) is almost definitely NOT what you want to do. The reason this macro argument is there to begin with is,
again, because it was designed to introduce new values.


The bottom line is that you should simply SEPARATE this zval (probably SEPARATE_IF_NOT_REF) before you send it back into the engine.

Zeev


-- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php



Reply via email to