diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c index be826b2f06d4d..0ff9f48b7cb36 100644 --- a/ext/spl/spl_observer.c +++ b/ext/spl/spl_observer.c @@ -44,10 +44,17 @@ PHPAPI zend_class_entry *spl_ce_MultipleIterator; PHPAPI zend_object_handlers spl_handler_SplObjectStorage; +/* Bit flags for marking internal functionality overridden by SplObjectStorage subclasses. */ +#define SOS_OVERRIDDEN_READ_DIMENSION 1 +#define SOS_OVERRIDDEN_WRITE_DIMENSION 2 +#define SOS_OVERRIDDEN_UNSET_DIMENSION 4 + typedef struct _spl_SplObjectStorage { /* {{{ */ HashTable storage; zend_long index; HashPosition pos; + /* In SplObjectStorage, flags is a hidden implementation detail to optimize ArrayAccess handlers. + * In MultipleIterator on a different class hierarchy, flags is a user settable value controlling iteration behavior. */ zend_long flags; zend_function *fptr_get_hash; zend_object std; @@ -76,7 +83,7 @@ void spl_SplObjectStorage_free_storage(zend_object *object) /* {{{ */ } /* }}} */ static int spl_object_storage_get_hash(zend_hash_key *key, spl_SplObjectStorage *intern, zend_object *obj) { - if (intern->fptr_get_hash) { + if (UNEXPECTED(intern->fptr_get_hash)) { zval param; zval rv; ZVAL_OBJ(¶m, obj); @@ -125,8 +132,54 @@ static spl_SplObjectStorageElement* spl_object_storage_get(spl_SplObjectStorage } } /* }}} */ +static spl_SplObjectStorageElement *spl_object_storage_create_element(zend_object *obj, zval *inf) /* {{{ */ +{ + spl_SplObjectStorageElement *pelement = emalloc(sizeof(spl_SplObjectStorageElement)); + pelement->obj = obj; + GC_ADDREF(obj); + if (inf) { + ZVAL_COPY(&pelement->inf, inf); + } else { + ZVAL_NULL(&pelement->inf); + } + return pelement; +} /* }}} */ + +/* A faster version of spl_object_storage_attach used when neither SplObjectStorage->getHash nor SplObjectStorage->offsetSet is overridden. */ +static spl_SplObjectStorageElement *spl_object_storage_attach_handle(spl_SplObjectStorage *intern, zend_object *obj, zval *inf) /* {{{ */ +{ + uint32_t handle = obj->handle; + zval *entry_zv = zend_hash_index_lookup(&intern->storage, handle); + spl_SplObjectStorageElement *pelement; + ZEND_ASSERT(!(intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION)); + + if (Z_TYPE_P(entry_zv) != IS_NULL) { + zval zv_inf; + ZEND_ASSERT(Z_TYPE_P(entry_zv) == IS_PTR); + pelement = Z_PTR_P(entry_zv); + ZVAL_COPY_VALUE(&zv_inf, &pelement->inf); + if (inf) { + ZVAL_COPY(&pelement->inf, inf); + } else { + ZVAL_NULL(&pelement->inf); + } + /* Call the old value's destructor last, in case it moves the entry */ + zval_ptr_dtor(&zv_inf); + return pelement; + } + + pelement = spl_object_storage_create_element(obj, inf); + ZVAL_PTR(entry_zv, pelement); + return pelement; +} /* }}} */ + spl_SplObjectStorageElement *spl_object_storage_attach(spl_SplObjectStorage *intern, zend_object *obj, zval *inf) /* {{{ */ { + if (EXPECTED(!(intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION))) { + return spl_object_storage_attach_handle(intern, obj, inf); + } + /* getHash or offsetSet is overridden. */ + spl_SplObjectStorageElement *pelement, element; zend_hash_key key; if (spl_object_storage_get_hash(&key, intern, obj) == FAILURE) { @@ -136,13 +189,16 @@ spl_SplObjectStorageElement *spl_object_storage_attach(spl_SplObjectStorage *int pelement = spl_object_storage_get(intern, &key); if (pelement) { - zval_ptr_dtor(&pelement->inf); + zval zv_inf; + ZVAL_COPY_VALUE(&zv_inf, &pelement->inf); if (inf) { ZVAL_COPY(&pelement->inf, inf); } else { ZVAL_NULL(&pelement->inf); } spl_object_storage_free_hash(intern, &key); + /* Call the old value's destructor last, in case it moves the entry */ + zval_ptr_dtor(&zv_inf); return pelement; } @@ -164,6 +220,9 @@ spl_SplObjectStorageElement *spl_object_storage_attach(spl_SplObjectStorage *int static int spl_object_storage_detach(spl_SplObjectStorage *intern, zend_object *obj) /* {{{ */ { + if (EXPECTED(!(intern->flags & SOS_OVERRIDDEN_UNSET_DIMENSION))) { + return zend_hash_index_del(&intern->storage, obj->handle); + } int ret = FAILURE; zend_hash_key key; if (spl_object_storage_get_hash(&key, intern, obj) == FAILURE) { @@ -189,6 +248,9 @@ void spl_object_storage_addall(spl_SplObjectStorage *intern, spl_SplObjectStorag intern->index = 0; } /* }}} */ +#define SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, zstr_method) \ + (((zend_function *)zend_hash_find_ptr(&(class_type)->function_table, ZSTR_KNOWN(zstr_method)))->common.scope != spl_ce_SplObjectStorage) + static zend_object *spl_object_storage_new_ex(zend_class_entry *class_type, zend_object *orig) /* {{{ */ { spl_SplObjectStorage *intern; @@ -207,10 +269,27 @@ static zend_object *spl_object_storage_new_ex(zend_class_entry *class_type, zend while (parent) { if (parent == spl_ce_SplObjectStorage) { + /* Possible optimization: Cache these results with a map from class entry to IS_NULL/IS_PTR. + * Or maybe just a single item with the result for the most recently loaded subclass. */ if (class_type != spl_ce_SplObjectStorage) { - intern->fptr_get_hash = zend_hash_str_find_ptr(&class_type->function_table, "gethash", sizeof("gethash") - 1); - if (intern->fptr_get_hash->common.scope == spl_ce_SplObjectStorage) { - intern->fptr_get_hash = NULL; + zend_function *get_hash = zend_hash_str_find_ptr(&class_type->function_table, "gethash", sizeof("gethash") - 1); + if (get_hash->common.scope != spl_ce_SplObjectStorage) { + intern->fptr_get_hash = get_hash; + } + if (intern->fptr_get_hash != NULL || + SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, ZEND_STR_OFFSETGET) || + SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, ZEND_STR_OFFSETEXISTS)) { + intern->flags |= SOS_OVERRIDDEN_READ_DIMENSION; + } + + if (intern->fptr_get_hash != NULL || + SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, ZEND_STR_OFFSETSET)) { + intern->flags |= SOS_OVERRIDDEN_WRITE_DIMENSION; + } + + if (intern->fptr_get_hash != NULL || + SPL_OBJECT_STORAGE_CLASS_HAS_OVERRIDE(class_type, ZEND_STR_OFFSETUNSET)) { + intern->flags |= SOS_OVERRIDDEN_UNSET_DIMENSION; } } break; @@ -328,20 +407,21 @@ static zend_object *spl_SplObjectStorage_new(zend_class_entry *class_type) } /* }}} */ -int spl_object_storage_contains(spl_SplObjectStorage *intern, zend_object *obj) /* {{{ */ +/* Returns true if the SplObjectStorage contains an entry for getHash(obj), even if the corresponding value is null. */ +bool spl_object_storage_contains(spl_SplObjectStorage *intern, zend_object *obj) /* {{{ */ { - int found; + if (EXPECTED(!intern->fptr_get_hash)) { + return zend_hash_index_find(&intern->storage, obj->handle) != NULL; + } zend_hash_key key; if (spl_object_storage_get_hash(&key, intern, obj) == FAILURE) { - return 0; + return true; } - if (key.key) { - found = zend_hash_exists(&intern->storage, key.key); - } else { - found = zend_hash_index_exists(&intern->storage, key.h); - } - spl_object_storage_free_hash(intern, &key); + ZEND_ASSERT(key.key); + bool found = zend_hash_exists(&intern->storage, key.key); + zend_string_release_ex(key.key, 0); + return found; } /* }}} */ @@ -361,6 +441,69 @@ PHP_METHOD(SplObjectStorage, attach) spl_object_storage_attach(intern, obj, inf); } /* }}} */ +static int spl_object_storage_has_dimension(zend_object *object, zval *offset, int check_empty) +{ + spl_SplObjectStorage *intern = spl_object_storage_from_obj(object); + if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_READ_DIMENSION))) { + /* Can't optimize empty()/isset() check if getHash, offsetExists, or offsetGet is overridden */ + return zend_std_has_dimension(object, offset, check_empty); + } + spl_SplObjectStorageElement *element = zend_hash_index_find_ptr(&intern->storage, Z_OBJ_HANDLE_P(offset)); + if (!element) { + return 0; + } + + if (check_empty) { + return i_zend_is_true(&element->inf); + } + /* NOTE: SplObjectStorage->offsetExists() is an alias of SplObjectStorage->contains(), so this returns true even if the value is null. */ + return 1; +} + +static zval *spl_object_storage_read_dimension(zend_object *object, zval *offset, int type, zval *rv) +{ + spl_SplObjectStorage *intern = spl_object_storage_from_obj(object); + if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_READ_DIMENSION))) { + /* Can't optimize it if getHash, offsetExists, or offsetGet is overridden */ + return zend_std_read_dimension(object, offset, type, rv); + } + spl_SplObjectStorageElement *element = zend_hash_index_find_ptr(&intern->storage, Z_OBJ_HANDLE_P(offset)); + + if (!element) { + if (type == BP_VAR_IS) { + return &EG(uninitialized_zval); + } + zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0, "Object not found"); + return NULL; + } else { + /* This deliberately returns a non-reference, even for BP_VAR_W and BP_VAR_RW, to behave the same way as SplObjectStorage did when using the default zend_std_read_dimension behavior. + * i.e. This prevents taking a reference to an entry of SplObjectStorage because offsetGet would return a non-reference. */ + ZEND_ASSERT(Z_TYPE(element->inf) != IS_REFERENCE); + ZVAL_COPY_DEREF(rv, &element->inf); + return rv; + } +} + +static void spl_object_storage_write_dimension(zend_object *object, zval *offset, zval *inf) +{ + spl_SplObjectStorage *intern = spl_object_storage_from_obj(object); + if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION))) { + zend_std_write_dimension(object, offset, inf); + return; + } + spl_object_storage_attach_handle(intern, Z_OBJ_P(offset), inf); +} + +static void spl_object_storage_unset_dimension(zend_object *object, zval *offset) +{ + spl_SplObjectStorage *intern = spl_object_storage_from_obj(object); + if (UNEXPECTED(Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_UNSET_DIMENSION))) { + zend_std_unset_dimension(object, offset); + return; + } + zend_hash_index_del(&intern->storage, Z_OBJ_HANDLE_P(offset)); +} + /* {{{ Detaches an object from the storage */ PHP_METHOD(SplObjectStorage, detach) { @@ -851,6 +994,7 @@ PHP_METHOD(SplObjectStorage, __unserialize) RETURN_THROWS(); } + ZVAL_DEREF(val); spl_object_storage_attach(intern, Z_OBJ_P(key), val); key = NULL; } else { @@ -1201,6 +1345,10 @@ PHP_MINIT_FUNCTION(spl_observer) spl_handler_SplObjectStorage.clone_obj = spl_object_storage_clone; spl_handler_SplObjectStorage.get_gc = spl_object_storage_get_gc; spl_handler_SplObjectStorage.free_obj = spl_SplObjectStorage_free_storage; + spl_handler_SplObjectStorage.read_dimension = spl_object_storage_read_dimension; + spl_handler_SplObjectStorage.write_dimension = spl_object_storage_write_dimension; + spl_handler_SplObjectStorage.has_dimension = spl_object_storage_has_dimension; + spl_handler_SplObjectStorage.unset_dimension = spl_object_storage_unset_dimension; spl_ce_MultipleIterator = register_class_MultipleIterator(zend_ce_iterator); spl_ce_MultipleIterator->create_object = spl_SplObjectStorage_new; diff --git a/ext/spl/tests/SplObjectStorage_coalesce.phpt b/ext/spl/tests/SplObjectStorage_coalesce.phpt new file mode 100644 index 0000000000000..5a89b46be6b8f --- /dev/null +++ b/ext/spl/tests/SplObjectStorage_coalesce.phpt @@ -0,0 +1,91 @@ +--TEST-- +SplObjectStorage magic operators +--FILE-- +contains($o2)); +echo "check isset/empty/contains for false.\n"; +$s[$o2] = false; +var_dump(isset($s[$o2])); +var_dump(empty($s[$o2])); +var_dump($s->contains($o2)); +try { + $s['invalid'] = 123; +} catch (Error $e) { + printf("%s: %s\n", $e::class, $e->getMessage()); +} +try { + var_dump(isset($s['invalid'])); +} catch (Error $e) { + printf("%s: %s\n", $e::class, $e->getMessage()); +} +$a = &$s[$o1]; + +var_dump($s); + +?> +--EXPECTF-- +string(7) "default" +string(7) "dynamic" +string(7) "dynamic" +string(7) "dynamic" +string(7) "dynamic" +bool(true) +bool(false) +o2 +bool(false) +bool(true) +object(stdClass)#4 (0) { +} +check isset/empty/contains for null. offsetExists returns true as long as the entry is there. +bool(true) +bool(true) +bool(true) +check isset/empty/contains for false. +bool(true) +bool(true) +bool(true) +TypeError: SplObjectStorage::offsetSet(): Argument #1 ($object) must be of type object, string given +TypeError: SplObjectStorage::offsetExists(): Argument #1 ($object) must be of type object, string given + +Notice: Indirect modification of overloaded element of SplObjectStorage has no effect in %s on line 38 +object(SplObjectStorage)#1 (1) { + ["storage":"SplObjectStorage":private]=> + array(2) { + [0]=> + array(2) { + ["obj"]=> + object(stdClass)#2 (0) { + } + ["inf"]=> + string(7) "dynamic" + } + [1]=> + array(2) { + ["obj"]=> + object(stdClass)#3 (0) { + } + ["inf"]=> + bool(false) + } + } +} \ No newline at end of file diff --git a/ext/spl/tests/SplObjectStorage_unserialize_reference.phpt b/ext/spl/tests/SplObjectStorage_unserialize_reference.phpt new file mode 100644 index 0000000000000..de18a42aacdf8 --- /dev/null +++ b/ext/spl/tests/SplObjectStorage_unserialize_reference.phpt @@ -0,0 +1,42 @@ +--TEST-- +SPL: Test that __unserialize converts references to non-references +--FILE-- +__unserialize([$x, []]); +var_dump($s); +$val = $s[$o]; +$val = 123; +var_dump($s); +?> +--EXPECT-- +object(SplObjectStorage)#1 (1) { + ["storage":"SplObjectStorage":private]=> + array(1) { + [0]=> + array(2) { + ["obj"]=> + object(stdClass)#2 (0) { + } + ["inf"]=> + int(1) + } + } +} +object(SplObjectStorage)#1 (1) { + ["storage":"SplObjectStorage":private]=> + array(1) { + [0]=> + array(2) { + ["obj"]=> + object(stdClass)#2 (0) { + } + ["inf"]=> + int(1) + } + } +} diff --git a/ext/spl/tests/SplObjectStorage_unset.phpt b/ext/spl/tests/SplObjectStorage_unset.phpt new file mode 100644 index 0000000000000..2e3e58c837cb0 --- /dev/null +++ b/ext/spl/tests/SplObjectStorage_unset.phpt @@ -0,0 +1,55 @@ +--TEST-- +SplObjectStorage unset and destructor edge cases +--FILE-- +getMessage()}\n"; +} +var_dump($s); +$s[$o] = new HasDestructor(); +try { + $s->offsetUnset($o); +} catch (Exception $e) { + echo "Caught: {$e->getMessage()}\n"; +} + +var_dump($s); +?> +--EXPECT-- +In destructor. Should no longer be accessible in $s: +object(SplObjectStorage)#2 (1) { + ["storage":"SplObjectStorage":private]=> + array(0) { + } +} +Caught: thrown from destructor +object(SplObjectStorage)#2 (1) { + ["storage":"SplObjectStorage":private]=> + array(0) { + } +} +In destructor. Should no longer be accessible in $s: +object(SplObjectStorage)#2 (1) { + ["storage":"SplObjectStorage":private]=> + array(0) { + } +} +Caught: thrown from destructor +object(SplObjectStorage)#2 (1) { + ["storage":"SplObjectStorage":private]=> + array(0) { + } +} \ No newline at end of file