From 943c48013b5877ad58adb9f9df28a03fd23899c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Magalh=C3=A3es?= Date: Tue, 7 Mar 2017 23:29:21 +0100 Subject: [PATCH 1/7] If the first index is negative let the next one stay negative --- Zend/zend_hash.c | 22 +++++++++++++------ ext/standard/array.c | 2 +- .../tests/array/array_fill_variation1.phpt | 2 +- .../array/array_fill_variation1_64bit.phpt | 2 +- ext/standard/tests/array/bug67693.phpt | 4 ++-- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index f5670dd3709fa..b7acf0ea5c229 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -181,7 +181,7 @@ static zend_always_inline void _zend_hash_init_int(HashTable *ht, uint32_t nSize ht->nNumUsed = 0; ht->nNumOfElements = 0; ht->nInternalPointer = HT_INVALID_IDX; - ht->nNextFreeElement = 0; + ht->nNextFreeElement = ZEND_LONG_MIN; ht->pDestructor = pDestructor; ht->nTableSize = zend_hash_check_size(nSize); } @@ -789,6 +789,10 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, IS_CONSISTENT(ht); HT_ASSERT_RC1(ht); + if (h == ZEND_LONG_MIN && flag & HASH_ADD_NEXT) { + h = 0; + } + if (UNEXPECTED(!(HT_FLAGS(ht) & HASH_FLAG_INITIALIZED))) { CHECK_INIT(ht, h < ht->nTableSize); if (h < ht->nTableSize) { @@ -841,7 +845,7 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, } zend_hash_iterators_update(ht, HT_INVALID_IDX, h); if ((zend_long)h >= (zend_long)ht->nNextFreeElement) { - ht->nNextFreeElement = h < ZEND_LONG_MAX ? h + 1 : ZEND_LONG_MAX; + ht->nNextFreeElement = (zend_long)h < ZEND_LONG_MAX ? h + 1 : ZEND_LONG_MAX; } p->h = h; p->key = NULL; @@ -863,7 +867,7 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, } ZVAL_COPY_VALUE(&p->val, pData); if ((zend_long)h >= (zend_long)ht->nNextFreeElement) { - ht->nNextFreeElement = h < ZEND_LONG_MAX ? h + 1 : ZEND_LONG_MAX; + ht->nNextFreeElement = (zend_long)h < ZEND_LONG_MAX ? h + 1 : ZEND_LONG_MAX; } return &p->val; } @@ -872,6 +876,10 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, ZEND_HASH_IF_FULL_DO_RESIZE(ht); /* If the Hash table is full, resize it */ add_to_hash: + /* Initialize nNextFreeElement with the value of the first numeric index */ + if (ht->nNextFreeElement == ZEND_LONG_MIN) { + ht->nNextFreeElement = h; + } idx = ht->nNumUsed++; ht->nNumOfElements++; if (ht->nInternalPointer == HT_INVALID_IDX) { @@ -879,7 +887,7 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, } zend_hash_iterators_update(ht, HT_INVALID_IDX, idx); if ((zend_long)h >= (zend_long)ht->nNextFreeElement) { - ht->nNextFreeElement = h < ZEND_LONG_MAX ? h + 1 : ZEND_LONG_MAX; + ht->nNextFreeElement = (zend_long)h < ZEND_LONG_MAX ? h + 1 : ZEND_LONG_MAX; } p = ht->arData + idx; p->h = h; @@ -1471,7 +1479,7 @@ ZEND_API void ZEND_FASTCALL zend_hash_clean(HashTable *ht) } ht->nNumUsed = 0; ht->nNumOfElements = 0; - ht->nNextFreeElement = 0; + ht->nNextFreeElement = ZEND_LONG_MIN; ht->nInternalPointer = HT_INVALID_IDX; } @@ -1510,7 +1518,7 @@ ZEND_API void ZEND_FASTCALL zend_symtable_clean(HashTable *ht) } ht->nNumUsed = 0; ht->nNumOfElements = 0; - ht->nNextFreeElement = 0; + ht->nNextFreeElement = ZEND_LONG_MIN; ht->nInternalPointer = HT_INVALID_IDX; } @@ -1837,7 +1845,7 @@ ZEND_API HashTable* ZEND_FASTCALL zend_array_dup(HashTable *source) target->nTableMask = HT_MIN_MASK; target->nNumUsed = 0; target->nNumOfElements = 0; - target->nNextFreeElement = 0; + target->nNextFreeElement = ZEND_LONG_MIN; target->nInternalPointer = HT_INVALID_IDX; HT_SET_DATA_ADDR(target, &uninitialized_bucket); } else if (GC_FLAGS(source) & IS_ARRAY_IMMUTABLE) { diff --git a/ext/standard/array.c b/ext/standard/array.c index 50ea6802ae301..26c67060698db 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3275,7 +3275,7 @@ PHP_FUNCTION(array_pop) ZVAL_DEREF(val); ZVAL_COPY(return_value, val); - if (!p->key && Z_ARRVAL_P(stack)->nNextFreeElement > 0 && p->h >= (zend_ulong)(Z_ARRVAL_P(stack)->nNextFreeElement - 1)) { + if (!p->key && p->h >= (zend_ulong)(Z_ARRVAL_P(stack)->nNextFreeElement - 1)) { Z_ARRVAL_P(stack)->nNextFreeElement = Z_ARRVAL_P(stack)->nNextFreeElement - 1; } diff --git a/ext/standard/tests/array/array_fill_variation1.phpt b/ext/standard/tests/array/array_fill_variation1.phpt index 684949fe95905..38c2703d0211d 100644 --- a/ext/standard/tests/array/array_fill_variation1.phpt +++ b/ext/standard/tests/array/array_fill_variation1.phpt @@ -120,7 +120,7 @@ array(2) { array(2) { [-10]=> int(100) - [0]=> + [-9]=> int(100) } -- Iteration 3 -- diff --git a/ext/standard/tests/array/array_fill_variation1_64bit.phpt b/ext/standard/tests/array/array_fill_variation1_64bit.phpt index 6cbec86f60843..adac4a175f275 100644 --- a/ext/standard/tests/array/array_fill_variation1_64bit.phpt +++ b/ext/standard/tests/array/array_fill_variation1_64bit.phpt @@ -120,7 +120,7 @@ array(2) { array(2) { [-10]=> int(100) - [0]=> + [-9]=> int(100) } -- Iteration 3 -- diff --git a/ext/standard/tests/array/bug67693.phpt b/ext/standard/tests/array/bug67693.phpt index 516436c511388..c9aa2d86fab6b 100644 --- a/ext/standard/tests/array/bug67693.phpt +++ b/ext/standard/tests/array/bug67693.phpt @@ -16,9 +16,9 @@ echo"\nDone"; ?> --EXPECT-- array(2) { - [0]=> + [-1]=> int(0) - [1]=> + [0]=> int(0) } From 0250615e0a2969be2660e452a75ff84a7f55de82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Magalh=C3=A3es?= Date: Sat, 10 Jun 2017 15:21:36 +0200 Subject: [PATCH 2/7] Added deprecated notice for cases where the behavior will change --- Zend/zend_hash.c | 8 ++++++++ ext/standard/tests/array/array_fill_variation1.phpt | 4 +++- ext/standard/tests/array/array_fill_variation1_64bit.phpt | 4 +++- ext/standard/tests/array/bug67693.phpt | 7 ++++--- ext/standard/tests/array/count_recursive.phpt | 1 + 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index b7acf0ea5c229..6aec5effbf4a5 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -789,6 +789,14 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, IS_CONSISTENT(ht); HT_ASSERT_RC1(ht); + /* Removing this block will end the deprecation phase and allow the new behavior */ + if (UNEXPECTED(flag & HASH_ADD_NEXT + && ht->nNextFreeElement > ZEND_LONG_MIN + && ht->nNextFreeElement < 0)) { + zend_error(E_DEPRECATED, "In the next major version of PHP the implicit keys of this array will start from %d instead of 0", ht->nNextFreeElement); + h = 0; + } + if (h == ZEND_LONG_MIN && flag & HASH_ADD_NEXT) { h = 0; } diff --git a/ext/standard/tests/array/array_fill_variation1.phpt b/ext/standard/tests/array/array_fill_variation1.phpt index 38c2703d0211d..9001e998b01b2 100644 --- a/ext/standard/tests/array/array_fill_variation1.phpt +++ b/ext/standard/tests/array/array_fill_variation1.phpt @@ -117,10 +117,12 @@ array(2) { int(100) } -- Iteration 2 -- + +Deprecated: In the next major version of PHP the implicit keys of this array will start from -9 instead of 0 in %s array(2) { [-10]=> int(100) - [-9]=> + [0]=> int(100) } -- Iteration 3 -- diff --git a/ext/standard/tests/array/array_fill_variation1_64bit.phpt b/ext/standard/tests/array/array_fill_variation1_64bit.phpt index adac4a175f275..2283d10c17856 100644 --- a/ext/standard/tests/array/array_fill_variation1_64bit.phpt +++ b/ext/standard/tests/array/array_fill_variation1_64bit.phpt @@ -117,10 +117,12 @@ array(2) { int(100) } -- Iteration 2 -- + +Deprecated: In the next major version of PHP the implicit keys of this array will start from -9 instead of 0 in %s array(2) { [-10]=> int(100) - [-9]=> + [0]=> int(100) } -- Iteration 3 -- diff --git a/ext/standard/tests/array/bug67693.phpt b/ext/standard/tests/array/bug67693.phpt index c9aa2d86fab6b..bc8236fefee6e 100644 --- a/ext/standard/tests/array/bug67693.phpt +++ b/ext/standard/tests/array/bug67693.phpt @@ -14,12 +14,13 @@ var_dump($array); echo"\nDone"; ?> ---EXPECT-- +--EXPECTF-- +Deprecated: In the next major version of PHP the implicit keys of this array will start from -1 instead of 0 in %s array(2) { - [-1]=> - int(0) [0]=> int(0) + [1]=> + int(0) } Done diff --git a/ext/standard/tests/array/count_recursive.phpt b/ext/standard/tests/array/count_recursive.phpt index b903d8b189236..ec2b1e7c377ea 100644 --- a/ext/standard/tests/array/count_recursive.phpt +++ b/ext/standard/tests/array/count_recursive.phpt @@ -130,6 +130,7 @@ fclose( $resource1 ); closedir( $resource2 ); ?> --EXPECTF-- +Deprecated: In the next major version of PHP the implicit keys of this array will start from -1 instead of 0 in %s *** Testing basic functionality of count() function *** -- Testing NULL -- From 2722a32cab05859e34138a980569ff853a5943a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Magalh=C3=A3es?= Date: Tue, 27 Feb 2018 23:19:45 +0000 Subject: [PATCH 3/7] Fix notice fmt and remove redundant block --- Zend/zend_hash.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 6aec5effbf4a5..89376b9ee0cbf 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -793,7 +793,8 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, if (UNEXPECTED(flag & HASH_ADD_NEXT && ht->nNextFreeElement > ZEND_LONG_MIN && ht->nNextFreeElement < 0)) { - zend_error(E_DEPRECATED, "In the next major version of PHP the implicit keys of this array will start from %d instead of 0", ht->nNextFreeElement); + zend_error(E_DEPRECATED, "In the next major version of PHP the implicit keys of this array will start from " ZEND_LONG_FMT " instead of 0", + ht->nNextFreeElement); h = 0; } @@ -884,10 +885,6 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, ZEND_HASH_IF_FULL_DO_RESIZE(ht); /* If the Hash table is full, resize it */ add_to_hash: - /* Initialize nNextFreeElement with the value of the first numeric index */ - if (ht->nNextFreeElement == ZEND_LONG_MIN) { - ht->nNextFreeElement = h; - } idx = ht->nNumUsed++; ht->nNumOfElements++; if (ht->nInternalPointer == HT_INVALID_IDX) { From 92fd7872eeb8f060942a6f66dced15e93e68738b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Magalh=C3=A3es?= Date: Wed, 28 Feb 2018 21:29:58 +0000 Subject: [PATCH 4/7] Avoid the deprecation notice when registering globals --- Zend/zend_hash.c | 2 +- main/php_variables.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 89376b9ee0cbf..0f02b439b23d5 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -789,7 +789,7 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, IS_CONSISTENT(ht); HT_ASSERT_RC1(ht); - /* Removing this block will end the deprecation phase and allow the new behavior */ + /* [negative_array_index] Removing this block will end the deprecation phase and allow the new behavior */ if (UNEXPECTED(flag & HASH_ADD_NEXT && ht->nNextFreeElement > ZEND_LONG_MIN && ht->nNextFreeElement < 0)) { diff --git a/main/php_variables.c b/main/php_variables.c index 9e7f185bd9447..681145aaa2f6f 100644 --- a/main/php_variables.c +++ b/main/php_variables.c @@ -245,6 +245,10 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars } else { plain_var: if (!index) { + /* [negative_array_index] Avoid the deprecation notice during globals registration */ + if (symtable1->nNextFreeElement > ZEND_LONG_MIN && symtable1->nNextFreeElement < 0) { + symtable1->nNextFreeElement = 0; + } if (zend_hash_next_index_insert(symtable1, val) == NULL) { zval_ptr_dtor(val); } From 06b41cb5132aea047c7bca609bc77e92e011737b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Magalh=C3=A3es?= Date: Wed, 28 Feb 2018 21:49:06 +0000 Subject: [PATCH 5/7] Bail out of CT eval when the deprecation notice would be emitted --- Zend/zend_compile.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index e6a34371a3a33..d550e43fa75f2 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7048,7 +7048,9 @@ static zend_bool zend_try_ct_eval_array(zval *result, zend_ast *ast) /* {{{ */ break; } } else { - if (!zend_hash_next_index_insert(Z_ARRVAL_P(result), value)) { + /* [negative_array_index] Bail out of CT eval in case the deprecation notice would be emitted so it may be handled in RT */ + if ((Z_ARRVAL_P(result)->nNextFreeElement > ZEND_LONG_MIN && Z_ARRVAL_P(result)->nNextFreeElement < 0) || + !zend_hash_next_index_insert(Z_ARRVAL_P(result), value)) { zval_ptr_dtor_nogc(value); zval_ptr_dtor(result); return 0; From deed6de92e6edbf2468bf80378fa48dd77250bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Magalh=C3=A3es?= Date: Wed, 28 Feb 2018 22:26:00 +0000 Subject: [PATCH 6/7] Extract the check for the notice to a macro --- Zend/zend_compile.c | 4 ++-- Zend/zend_hash.c | 5 ++--- Zend/zend_hash.h | 3 +++ ext/standard/tests/array/count_recursive.phpt | 3 ++- main/php_variables.c | 4 ++-- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index d550e43fa75f2..9cd55b58806ec 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7048,8 +7048,8 @@ static zend_bool zend_try_ct_eval_array(zval *result, zend_ast *ast) /* {{{ */ break; } } else { - /* [negative_array_index] Bail out of CT eval in case the deprecation notice would be emitted so it may be handled in RT */ - if ((Z_ARRVAL_P(result)->nNextFreeElement > ZEND_LONG_MIN && Z_ARRVAL_P(result)->nNextFreeElement < 0) || + /* Bail out of CT eval in case the deprecation notice would be emitted so it may be handled in RT */ + if (HASH_ADD_NEXT_EMITS_DEPRECATED(Z_ARRVAL_P(result)) || !zend_hash_next_index_insert(Z_ARRVAL_P(result), value)) { zval_ptr_dtor_nogc(value); zval_ptr_dtor(result); diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 0f02b439b23d5..c9a14118238ab 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -789,10 +789,9 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, IS_CONSISTENT(ht); HT_ASSERT_RC1(ht); - /* [negative_array_index] Removing this block will end the deprecation phase and allow the new behavior */ + /* Removing this block will end the deprecation phase and allow the new behavior */ if (UNEXPECTED(flag & HASH_ADD_NEXT - && ht->nNextFreeElement > ZEND_LONG_MIN - && ht->nNextFreeElement < 0)) { + && HASH_ADD_NEXT_EMITS_DEPRECATED(ht))) { zend_error(E_DEPRECATED, "In the next major version of PHP the implicit keys of this array will start from " ZEND_LONG_FMT " instead of 0", ht->nNextFreeElement); h = 0; diff --git a/Zend/zend_hash.h b/Zend/zend_hash.h index af9c21a7ecc20..4339455a71126 100644 --- a/Zend/zend_hash.h +++ b/Zend/zend_hash.h @@ -35,6 +35,9 @@ #define HASH_ADD_NEW (1<<3) #define HASH_ADD_NEXT (1<<4) +#define HASH_ADD_NEXT_EMITS_DEPRECATED(ht) \ + ((ht)->nNextFreeElement > ZEND_LONG_MIN && (ht)->nNextFreeElement < 0) + #define HASH_FLAG_CONSISTENCY ((1<<0) | (1<<1)) #define HASH_FLAG_PACKED (1<<2) #define HASH_FLAG_INITIALIZED (1<<3) diff --git a/ext/standard/tests/array/count_recursive.phpt b/ext/standard/tests/array/count_recursive.phpt index ec2b1e7c377ea..a925e3cacedbb 100644 --- a/ext/standard/tests/array/count_recursive.phpt +++ b/ext/standard/tests/array/count_recursive.phpt @@ -130,7 +130,6 @@ fclose( $resource1 ); closedir( $resource2 ); ?> --EXPECTF-- -Deprecated: In the next major version of PHP the implicit keys of this array will start from -1 instead of 0 in %s *** Testing basic functionality of count() function *** -- Testing NULL -- @@ -162,6 +161,8 @@ COUNT_NORMAL: should be 3, is 3 COUNT_RECURSIVE: should be 13, is 13 *** Testing possible variations of count() function on arrays *** +Deprecated: In the next major version of PHP the implicit keys of this array will start from -1 instead of 0 in %s + -- Iteration 0 -- COUNT_NORMAL is 0 COUNT_RECURSIVE is 0 diff --git a/main/php_variables.c b/main/php_variables.c index 681145aaa2f6f..4bba9e9ee7557 100644 --- a/main/php_variables.c +++ b/main/php_variables.c @@ -245,8 +245,8 @@ PHPAPI void php_register_variable_ex(char *var_name, zval *val, zval *track_vars } else { plain_var: if (!index) { - /* [negative_array_index] Avoid the deprecation notice during globals registration */ - if (symtable1->nNextFreeElement > ZEND_LONG_MIN && symtable1->nNextFreeElement < 0) { + /* Avoid the deprecation notice during globals registration */ + if (HASH_ADD_NEXT_EMITS_DEPRECATED(symtable1)) { symtable1->nNextFreeElement = 0; } if (zend_hash_next_index_insert(symtable1, val) == NULL) { From 67776c0be400a04585b04cdeee9268067d1ef409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Magalh=C3=A3es?= Date: Thu, 1 Mar 2018 01:11:31 +0000 Subject: [PATCH 7/7] Also bail out of CT eval on SCCP --- ext/opcache/Optimizer/sccp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opcache/Optimizer/sccp.c b/ext/opcache/Optimizer/sccp.c index 07fdeee7089b0..8418254c08feb 100644 --- a/ext/opcache/Optimizer/sccp.c +++ b/ext/opcache/Optimizer/sccp.c @@ -475,7 +475,7 @@ static inline int ct_eval_del_array_elem(zval *result, zval *key) { static inline int ct_eval_add_array_elem(zval *result, zval *value, zval *key) { if (!key) { SEPARATE_ARRAY(result); - if ((value = zend_hash_next_index_insert(Z_ARR_P(result), value))) { + if (!HASH_ADD_NEXT_EMITS_DEPRECATED(Z_ARR_P(result)) && (value = zend_hash_next_index_insert(Z_ARR_P(result), value))) { Z_TRY_ADDREF_P(value); return SUCCESS; }