Array iteration corrupts existing zvals (?)

[cyrus-and]

I'm developing a PHP extension which overrides zend_execute_* to fetch some values from the runtime. Whenever I need to dump a value I wrap it into a Php::Value object then I switch over value.type() to properly format it. The idea is that when I find an array I want to recursively call the format function with each member. Here's the relevant part:

switch (value.type()) {
    // ...
    case Php::Type::Array:
    case Php::Type::ConstantArray:
        for (auto &it : value) {
            output_string(it.first.stringValue().c_str());
            format(it.second);
        }
        break;
    // ...
}

Now this works against simple test cases but when I run complex PHP code, weird things happen. Scripts complain about wrong values (e.g., NULL or unexpected types), even if the iteration body is empty. This suggests that zvalues may be corrupted during the mere iteration phase.

After some debugging I noticed that the problem seems to be related to how the current value is stored or to what happens when the wrapping Php::Value is destroyed. Any thoughts?

I apologize if I'm not providing any code to reproduce the issue but I didn't find a simple way.

[cyrus-and]

I don't know if this is entirely correct, but replacing the aforementioned range-based for loop with the following snippet, both formats the values properly and avoid the zvals corruption (according to some very brief testing):

#define protected public
#include <phpcpp.h>
// ...
{
    Bucket *position = nullptr;
    HashTable *table = Z_ARRVAL_P(value._val);

    zend_hash_internal_pointer_reset_ex(table, &position);

    for (;;) {
        // fetch key
        Php::Value key;
        zend_hash_get_current_key_zval_ex(table, key._val, &position);
        if (Z_TYPE_P(key._val) == IS_NULL) {
            break;
        }
        // fetch value
        zval **value;
        zend_hash_get_current_data_ex(table, (void **)&value, &position);
        // consume
        output_string(key.stringValue().c_str());
        format(Php::Value(*value));
        // next
        assert(zend_hash_move_forward_ex(table, &position) == SUCCESS);
    }
}

As you can see I just copied the relevant parts from the HashIterator class, the only real difference is that the fetched values are not placed into the pair, rather they are consumed immediately (within the same block).

[cyrus-and]

Browsing the existing issues and PRs, I've found #115 by @andot; among the other things that PR highlights a problem with the move operator which looks the same I'm reporting here. The proposed solution (andot/PHP-CPP@cada2bd8d969fdfe1ebc4abcfa5b52e56eca7bcf) happens to fix this issue.

@andot why does that commit has never been merged with this repo?

I believe this issue is fixed in the latest commit to PHP-CPP. I experienced the same problem and after changing the move constructor of Php::Value it seems to be solved. If you still experience this problem, please reopen the issue and we'll see what we can do about it.

[cyrus-and]

@martijnotto Actually I'm no more able to compile this library, it seems that 9f2e816 broke something, at least on my setup: php5-dev Debian package version 5.6.20.

In file included from zend/includes.h:130:0,
                 from zend/inivalue.cpp:8:
zend/constantimpl.h:85:50: error: macro "ZVAL_STRINGL" requires 4 arguments, but only 3 given
         ZVAL_STRINGL(&_constant.value, value, len);
                                                  ^
[...]

[schoentoon]

@cyrus-and That's entirely correct, this repository now (master at least) is now for PHP7. For PHP5 please use https://github.com/CopernicaMarketingSoftware/PHP-CPP-LEGACY instead.

[cyrus-and]

Oh, thank you @schoentoon, didn't know it.