search

colder / php-weakref

PECL extension that implements weak references and weak maps in PHP
http://pecl.php.net/weakref
Other
36 stars 13 forks source link

small fix - wr_store.c #26

Open commercebyte opened 8 years ago

commercebyte commented 8 years ago

old_dtors it actually holds zend_object_dtor_obj_t's, not zval's, so Z_PTR_P() didn't do any good

colder commented 8 years ago

I don't understand how this can be correct. In php7 hashtables always hold zvals. To store other contents, we have the _ptr API to automate the wrap/unwrap. Since we don't have it for apply_with_arguments, I really think we have to access through Z_PTR_P within the function.

See for instance http://lxr.php.net/xref/PHP_7_0/ext/reflection/php_reflection.c#_extension_class_string called here: http://lxr.php.net/xref/PHP_7_0/ext/reflection/php_reflection.c#1120

commercebyte commented 8 years ago

wr_store.c:115 - in wr_store_track() - zend_hash_index_update_ptr(&store->old_dtors, handlers_key, ref_obj->handlers->dtor_obj);

the value assigned to the hash is zend_object_dtor_obj_t, not zval, so we need to treat the hash value as zend_object_dtor_obj_t, treating it as zval causes another seg fault

i agree it might be dirty, but looks like it's been working and it still does when i have more time i'll consider cleaning up the code and turning the hash values into zvals

colder commented 8 years ago

Yes, but: the _ptr family of functions will wrap/unwrap the value stored in the array in zvals. What ends up in the array remains a zval, even if you store it using update_ptr. (See for instance https://wiki.php.net/phpng-upgrading#hashtable_api)

It might be that the original issue is avoided with any non-NULL value for dtor, and thus it is not relevant if what is stored is invalid?

Did you experience any specific issue with my version of the patch (that does the unwrapping) ?