diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index c94b605c5c96..03b107af17f7 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -53,6 +53,8 @@ PHP 8.7 INTERNALS UPGRADE NOTES freed with zend_hash_destroy() + pefree(). - The ZTS symbols compiler_globals_offset and executor_globals_offset have been removed: CG()/EG() now reach the globals through a __thread struct. +- Removed zend_hash_graceful_destroy(). Use zend_hash_graceful_reverse_destroy() + or zend_hash_destroy() instead. - Removed zend_error_noreturn_unchecked(). ======================== diff --git a/Zend/tests/gh8438.phpt b/Zend/tests/gh8438.phpt new file mode 100644 index 000000000000..84b38898b221 --- /dev/null +++ b/Zend/tests/gh8438.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-8438 (HashTableIterators of empty hashtables are not poisoned upon destruction) +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECT-- +zend_hash_destroy, uninitialized: bool(true) +zend_hash_destroy, packed: bool(true) +zend_hash_destroy, packed_emptied: bool(true) +zend_hash_destroy, hash: bool(true) +zend_hash_destroy, hash_emptied: bool(true) +zend_hash_graceful_reverse_destroy, uninitialized: bool(true) +zend_hash_graceful_reverse_destroy, packed: bool(true) +zend_hash_graceful_reverse_destroy, packed_emptied: bool(true) +zend_hash_graceful_reverse_destroy, hash: bool(true) +zend_hash_graceful_reverse_destroy, hash_emptied: bool(true) diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 0b2ed1c8cbd3..2776cc5b251d 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -1986,7 +1986,6 @@ ZEND_API void ZEND_FASTCALL zend_hash_destroy(HashTable *ht) } SET_INCONSISTENT(HT_DESTROYED); } - zend_hash_iterators_remove(ht); } else { Bucket *p = ht->arData; Bucket *end = p + ht->nNumUsed; @@ -2034,12 +2033,13 @@ ZEND_API void ZEND_FASTCALL zend_hash_destroy(HashTable *ht) } while (++p != end); } } - zend_hash_iterators_remove(ht); } - } else if (EXPECTED(HT_FLAGS(ht) & HASH_FLAG_UNINITIALIZED)) { - return; } - pefree(HT_GET_DATA_ADDR(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); + + zend_hash_iterators_remove(ht); + if (!(HT_FLAGS(ht) & HASH_FLAG_UNINITIALIZED)) { + pefree(HT_GET_DATA_ADDR(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); + } } ZEND_API void ZEND_FASTCALL zend_array_destroy(HashTable *ht) @@ -2223,35 +2223,6 @@ ZEND_API void ZEND_FASTCALL zend_symtable_clean(HashTable *ht) ht->nInternalPointer = 0; } -ZEND_API void ZEND_FASTCALL zend_hash_graceful_destroy(HashTable *ht) -{ - uint32_t idx; - - IS_CONSISTENT(ht); - HT_ASSERT_RC1(ht); - - if (HT_IS_PACKED(ht)) { - zval *zv = ht->arPacked; - - for (idx = 0; idx < ht->nNumUsed; idx++, zv++) { - if (UNEXPECTED(Z_TYPE_P(zv) == IS_UNDEF)) continue; - _zend_hash_packed_del_val(ht, HT_IDX_TO_HASH(idx), zv); - } - } else { - Bucket *p = ht->arData; - - for (idx = 0; idx < ht->nNumUsed; idx++, p++) { - if (UNEXPECTED(Z_TYPE(p->val) == IS_UNDEF)) continue; - _zend_hash_del_el(ht, HT_IDX_TO_HASH(idx), p); - } - } - if (!(HT_FLAGS(ht) & HASH_FLAG_UNINITIALIZED)) { - pefree(HT_GET_DATA_ADDR(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); - } - - SET_INCONSISTENT(HT_DESTROYED); -} - ZEND_API void ZEND_FASTCALL zend_hash_graceful_reverse_destroy(HashTable *ht) { uint32_t idx; @@ -2280,6 +2251,7 @@ ZEND_API void ZEND_FASTCALL zend_hash_graceful_reverse_destroy(HashTable *ht) } } + zend_hash_iterators_remove(ht); if (!(HT_FLAGS(ht) & HASH_FLAG_UNINITIALIZED)) { pefree(HT_GET_DATA_ADDR(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); } diff --git a/Zend/zend_hash.h b/Zend/zend_hash.h index cb8a2256aa63..561c7a82e4a1 100644 --- a/Zend/zend_hash.h +++ b/Zend/zend_hash.h @@ -166,7 +166,6 @@ typedef int (*apply_func_t)(zval *pDest); typedef int (*apply_func_arg_t)(zval *pDest, void *argument); typedef int (*apply_func_args_t)(zval *pDest, int num_args, va_list args, zend_hash_key *hash_key); -ZEND_API void ZEND_FASTCALL zend_hash_graceful_destroy(HashTable *ht); ZEND_API void ZEND_FASTCALL zend_hash_graceful_reverse_destroy(HashTable *ht); ZEND_API void ZEND_FASTCALL zend_hash_apply(HashTable *ht, apply_func_t apply_func); ZEND_API void ZEND_FASTCALL zend_hash_apply_with_argument(HashTable *ht, apply_func_arg_t apply_func, void *); diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 0ca19544d022..dc89ebc837b4 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -2793,3 +2793,80 @@ static PHP_FUNCTION(zend_test_gh19792) zend_error(E_WARNING, "a warning"); zend_throw_error(NULL, "an exception"); } + +/* Returns whether destroying a HashTable detaches the iterators on it. */ +static PHP_FUNCTION(zend_test_hash_destroy_detaches_iterator) +{ + zend_string *destroy, *state; + + ZEND_PARSE_PARAMETERS_START(2, 2) + Z_PARAM_STR(destroy) + Z_PARAM_STR(state) + ZEND_PARSE_PARAMETERS_END(); + + void (ZEND_FASTCALL *destroy_func)(HashTable *); + if (zend_string_equals_literal(destroy, "zend_hash_destroy")) { + destroy_func = zend_hash_destroy; + } else if (zend_string_equals_literal(destroy, "zend_hash_graceful_reverse_destroy")) { + destroy_func = zend_hash_graceful_reverse_destroy; + } else { + zend_argument_value_error(1, "must be \"zend_hash_destroy\" or \"zend_hash_graceful_reverse_destroy\""); + RETURN_THROWS(); + } + + bool packed; + bool emptied; + if (zend_string_equals_literal(state, "uninitialized")) { + packed = false; + emptied = false; + } else if (zend_string_equals_literal(state, "packed")) { + packed = true; + emptied = false; + } else if (zend_string_equals_literal(state, "packed_emptied")) { + packed = true; + emptied = true; + } else if (zend_string_equals_literal(state, "hash")) { + packed = false; + emptied = false; + } else if (zend_string_equals_literal(state, "hash_emptied")) { + packed = false; + emptied = true; + } else { + zend_argument_value_error(2, "must be \"uninitialized\", \"packed\", \"packed_emptied\", \"hash\", or \"hash_emptied\""); + RETURN_THROWS(); + } + + HashTable *ht = emalloc(sizeof(HashTable)); + zend_hash_init(ht, 8, NULL, ZVAL_PTR_DTOR, false); + + if (!zend_string_equals_literal(state, "uninitialized")) { + zval value; + ZVAL_STR(&value, zend_string_init("value", strlen("value"), false)); + if (packed) { + zend_hash_index_add_new(ht, 0, &value); + ZEND_ASSERT(HT_IS_PACKED(ht)); + } else { + zend_hash_str_add_new(ht, "key", strlen("key"), &value); + ZEND_ASSERT(!HT_IS_PACKED(ht)); + } + if (emptied) { + if (packed) { + zend_hash_index_del(ht, 0); + } else { + zend_hash_str_del(ht, "key", strlen("key")); + } + ZEND_ASSERT(ht->nNumUsed == 0); + } + } + + uint32_t iter_idx = zend_hash_iterator_add(ht, 0); + destroy_func(ht); + bool detached = EG(ht_iterators)[iter_idx].ht != ht; + + /* Delete the iterator before ht is freed, so that this helper stays memory safe + * when the iterator was not detached. */ + zend_hash_iterator_del(iter_idx); + efree(ht); + + RETURN_BOOL(detached); +} diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index 62cc78cbe241..7bfcf2987f35 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -472,6 +472,8 @@ function zend_test_uri_parser(string $uri, string $parser): array { } /** @compile-time-eval */ function zend_test_gh19792(): void {} + function zend_test_hash_destroy_detaches_iterator(string $destroy, string $state): bool {} + /* @param resource $stream */ function zend_test_handle_post_data(mixed $stream, string $content_type, int $content_length): void {} diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index 4d7bced1402d..40b9bf97e5d1 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit test.stub.php instead. - * Stub hash: a541ec4d43e8dfb243636d2517436196e337047e + * Stub hash: 23669ad6e9407e4851abc2f350efd7672a82c98a * Has decl header: yes */ #include "zend_attributes.h" @@ -362,6 +362,11 @@ ZEND_END_ARG_INFO() #define arginfo_zend_test_gh19792 arginfo_zend_test_void_return +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_hash_destroy_detaches_iterator, 0, 2, _IS_BOOL, 0) + ZEND_ARG_TYPE_INFO(0, destroy, IS_STRING, 0) + ZEND_ARG_TYPE_INFO(0, state, IS_STRING, 0) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_handle_post_data, 0, 3, IS_VOID, 0) ZEND_ARG_TYPE_INFO(0, stream, IS_MIXED, 0) ZEND_ARG_TYPE_INFO(0, content_type, IS_STRING, 0) @@ -573,6 +578,7 @@ static ZEND_FUNCTION(zend_test_gh18756); static ZEND_FUNCTION(zend_test_opcache_preloading); static ZEND_FUNCTION(zend_test_uri_parser); static ZEND_FUNCTION(zend_test_gh19792); +static ZEND_FUNCTION(zend_test_hash_destroy_detaches_iterator); static ZEND_FUNCTION(zend_test_handle_post_data); static ZEND_FUNCTION(zend_new_singleton_array); static ZEND_FUNCTION(zend_test_set_local_var); @@ -767,6 +773,7 @@ static const zend_function_entry ext_functions[] = { ZEND_RAW_FENTRY("zend_test_gh19792", zif_zend_test_gh19792, arginfo_zend_test_gh19792, 0) #endif #endif + ZEND_FE(zend_test_hash_destroy_detaches_iterator, arginfo_zend_test_hash_destroy_detaches_iterator) ZEND_FE(zend_test_handle_post_data, arginfo_zend_test_handle_post_data) ZEND_FE(zend_new_singleton_array, arginfo_zend_new_singleton_array) ZEND_FE(zend_test_set_local_var, arginfo_zend_test_set_local_var) diff --git a/ext/zend_test/test_decl.h b/ext/zend_test/test_decl.h index 8fc579dd6879..278b3b6b0fe0 100644 --- a/ext/zend_test/test_decl.h +++ b/ext/zend_test/test_decl.h @@ -1,8 +1,8 @@ /* This is a generated file, edit test.stub.php instead. - * Stub hash: a541ec4d43e8dfb243636d2517436196e337047e */ + * Stub hash: 23669ad6e9407e4851abc2f350efd7672a82c98a */ -#ifndef ZEND_TEST_DECL_a541ec4d43e8dfb243636d2517436196e337047e_H -#define ZEND_TEST_DECL_a541ec4d43e8dfb243636d2517436196e337047e_H +#ifndef ZEND_TEST_DECL_23669ad6e9407e4851abc2f350efd7672a82c98a_H +#define ZEND_TEST_DECL_23669ad6e9407e4851abc2f350efd7672a82c98a_H typedef enum zend_enum_ZendTestUnitEnum { ZEND_ENUM_ZendTestUnitEnum_Foo = 1, @@ -27,4 +27,4 @@ typedef enum zend_enum_ZendTestEnumWithInterface { ZEND_ENUM_ZendTestEnumWithInterface_Bar = 2, } zend_enum_ZendTestEnumWithInterface; -#endif /* ZEND_TEST_DECL_a541ec4d43e8dfb243636d2517436196e337047e_H */ +#endif /* ZEND_TEST_DECL_23669ad6e9407e4851abc2f350efd7672a82c98a_H */ diff --git a/ext/zend_test/test_legacy_arginfo.h b/ext/zend_test/test_legacy_arginfo.h index f654203e2a60..c60228579591 100644 --- a/ext/zend_test/test_legacy_arginfo.h +++ b/ext/zend_test/test_legacy_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit test.stub.php instead. - * Stub hash: a541ec4d43e8dfb243636d2517436196e337047e + * Stub hash: 23669ad6e9407e4851abc2f350efd7672a82c98a * Has decl header: yes */ #include "zend_constants.h" @@ -299,6 +299,11 @@ ZEND_END_ARG_INFO() #define arginfo_zend_test_gh19792 arginfo_zend_trigger_bailout +ZEND_BEGIN_ARG_INFO_EX(arginfo_zend_test_hash_destroy_detaches_iterator, 0, 0, 2) + ZEND_ARG_INFO(0, destroy) + ZEND_ARG_INFO(0, state) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_INFO_EX(arginfo_zend_test_handle_post_data, 0, 0, 3) ZEND_ARG_INFO(0, stream) ZEND_ARG_INFO(0, content_type) @@ -497,6 +502,7 @@ static ZEND_FUNCTION(zend_test_gh18756); static ZEND_FUNCTION(zend_test_opcache_preloading); static ZEND_FUNCTION(zend_test_uri_parser); static ZEND_FUNCTION(zend_test_gh19792); +static ZEND_FUNCTION(zend_test_hash_destroy_detaches_iterator); static ZEND_FUNCTION(zend_test_handle_post_data); static ZEND_FUNCTION(zend_new_singleton_array); static ZEND_FUNCTION(zend_test_set_local_var); @@ -655,6 +661,7 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(zend_test_opcache_preloading, arginfo_zend_test_opcache_preloading) ZEND_FE(zend_test_uri_parser, arginfo_zend_test_uri_parser) ZEND_FE(zend_test_gh19792, arginfo_zend_test_gh19792) + ZEND_FE(zend_test_hash_destroy_detaches_iterator, arginfo_zend_test_hash_destroy_detaches_iterator) ZEND_FE(zend_test_handle_post_data, arginfo_zend_test_handle_post_data) ZEND_FE(zend_new_singleton_array, arginfo_zend_new_singleton_array) ZEND_FE(zend_test_set_local_var, arginfo_zend_test_set_local_var)