Skip to content

Fix iterator position resetting #13188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Fix iterator position resetting #13188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a3f4e0a
Fix iterator position resetting
iluuu1994 Jan 18, 2024
f053c8e
Same adjustment for packed arrays
iluuu1994 Jan 18, 2024
1446223
Avoid redundancy
iluuu1994 Jan 18, 2024
dede210
Fix php_slice
iluuu1994 Jan 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions Zend/tests/gh13178_1.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
--TEST--
GH-13178: Packed to hash must reset iterator position
--FILE--
<?php
$array = ['foo'];
foreach ($array as $key => &$value) {
var_dump($key);
unset($array[$key]);
$array[] = 'foo';
if ($key === 10) {
break;
}
}
?>
--EXPECT--
int(0)
int(1)
int(2)
int(3)
int(4)
int(5)
int(6)
int(7)
int(8)
int(9)
int(10)
26 changes: 26 additions & 0 deletions Zend/tests/gh13178_2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
--TEST--
GH-13178: Unsetting last offset must floor iterator position
--FILE--
<?php
$array = [100 => 'foo'];
foreach ($array as $key => &$value) {
var_dump($key);
unset($array[$key]);
$array[] = 'foo';
if ($key === 110) {
break;
}
}
?>
--EXPECT--
int(100)
int(101)
int(102)
int(103)
int(104)
int(105)
int(106)
int(107)
int(108)
int(109)
int(110)
24 changes: 24 additions & 0 deletions Zend/tests/gh13178_3.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--TEST--
GH-13178: Unsetting last offset variation with hashed array
--FILE--
<?php

$data = ['foo' => 'foo', 'bar' => 'bar', 'baz' => 'baz'];

foreach ($data as $key => &$value) {
var_dump($value);
if ($value === 'baz') {
unset($data['bar']);
unset($data['baz']);
$data['qux'] = 'qux';
$data['quux'] = 'quux';
}
}

?>
--EXPECT--
string(3) "foo"
string(3) "bar"
string(3) "baz"
string(3) "qux"
string(4) "quux"
29 changes: 29 additions & 0 deletions Zend/tests/gh13178_4.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
GH-13178: Packed to hash must reset nInternalPointer
--FILE--
<?php
$array = ['foo'];
reset($array);
while (true) {
$key = key($array);
next($array);
var_dump($key);
unset($array[$key]);
$array[] = 'foo';
if ($key === 10) {
break;
}
}
?>
--EXPECT--
int(0)
int(1)
int(2)
int(3)
int(4)
int(5)
int(6)
int(7)
int(8)
int(9)
int(10)
21 changes: 21 additions & 0 deletions Zend/tests/gh13178_5.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
GH-13178: Packed array with last elements removed must reset iterator positions
--FILE--
<?php
$array = [0, 1, 2];
foreach ($array as &$value) {
var_dump($value);
if ($value === 2) {
unset($array[2]);
unset($array[1]);
$array[1] = 3;
$array[2] = 4;
}
}
?>
--EXPECT--
int(0)
int(1)
int(2)
int(3)
int(4)
62 changes: 28 additions & 34 deletions Zend/zend_hash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1331,6 +1331,18 @@ ZEND_API void ZEND_FASTCALL zend_hash_rehash(HashTable *ht)
if (!(HT_FLAGS(ht) & HASH_FLAG_UNINITIALIZED)) {
ht->nNumUsed = 0;
HT_HASH_RESET(ht);
/* Even if the array is empty, we still need to reset the iterator positions. */
ht->nInternalPointer = 0;
if (UNEXPECTED(HT_HAS_ITERATORS(ht))) {
HashTableIterator *iter = EG(ht_iterators);
HashTableIterator *end = iter + EG(ht_iterators_used);
while (iter != end) {
if (iter->ht == ht) {
iter->pos = 0;
}
iter++;
}
}
}
return;
}
Expand Down Expand Up @@ -1412,32 +1424,30 @@ ZEND_API void ZEND_FASTCALL zend_hash_rehash(HashTable *ht)
}
}

static zend_always_inline void _zend_hash_packed_del_val(HashTable *ht, uint32_t idx, zval *zv)
static zend_always_inline void zend_hash_iterators_clamp_max(HashTable *ht, uint32_t max)
{
idx = HT_HASH_TO_IDX(idx);
ht->nNumOfElements--;
if (ht->nInternalPointer == idx || UNEXPECTED(HT_HAS_ITERATORS(ht))) {
uint32_t new_idx;

new_idx = idx;
while (1) {
new_idx++;
if (new_idx >= ht->nNumUsed) {
break;
} else if (Z_TYPE(ht->arPacked[new_idx]) != IS_UNDEF) {
break;
if (UNEXPECTED(HT_HAS_ITERATORS(ht))) {
HashTableIterator *iter = EG(ht_iterators);
HashTableIterator *end = iter + EG(ht_iterators_used);
while (iter != end) {
if (iter->ht == ht) {
iter->pos = MIN(iter->pos, max);
}
iter++;
}
if (ht->nInternalPointer == idx) {
ht->nInternalPointer = new_idx;
}
zend_hash_iterators_update(ht, idx, new_idx);
}
}

static zend_always_inline void _zend_hash_packed_del_val(HashTable *ht, uint32_t idx, zval *zv)
{
idx = HT_HASH_TO_IDX(idx);
ht->nNumOfElements--;
if (ht->nNumUsed - 1 == idx) {
do {
ht->nNumUsed--;
} while (ht->nNumUsed > 0 && (UNEXPECTED(Z_TYPE(ht->arPacked[ht->nNumUsed-1]) == IS_UNDEF)));
ht->nInternalPointer = MIN(ht->nInternalPointer, ht->nNumUsed);
zend_hash_iterators_clamp_max(ht, ht->nNumUsed);
}
if (ht->pDestructor) {
zval tmp;
Expand All @@ -1458,28 +1468,12 @@ static zend_always_inline void _zend_hash_del_el_ex(HashTable *ht, uint32_t idx,
}
idx = HT_HASH_TO_IDX(idx);
ht->nNumOfElements--;
if (ht->nInternalPointer == idx || UNEXPECTED(HT_HAS_ITERATORS(ht))) {
uint32_t new_idx;

new_idx = idx;
while (1) {
new_idx++;
if (new_idx >= ht->nNumUsed) {
break;
} else if (Z_TYPE(ht->arData[new_idx].val) != IS_UNDEF) {
break;
}
}
if (ht->nInternalPointer == idx) {
ht->nInternalPointer = new_idx;
}
zend_hash_iterators_update(ht, idx, new_idx);
}
if (ht->nNumUsed - 1 == idx) {
do {
ht->nNumUsed--;
} while (ht->nNumUsed > 0 && (UNEXPECTED(Z_TYPE(ht->arData[ht->nNumUsed-1].val) == IS_UNDEF)));
ht->nInternalPointer = MIN(ht->nInternalPointer, ht->nNumUsed);
zend_hash_iterators_clamp_max(ht, ht->nNumUsed);
}
if (ht->pDestructor) {
zval tmp;
Expand Down
11 changes: 10 additions & 1 deletion ext/standard/array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3262,6 +3262,11 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
Z_TRY_ADDREF_P(entry);
zend_hash_next_index_insert_new(removed, entry);
zend_hash_packed_del_val(in_hash, entry);
/* Bump iterator positions to the element after replacement. */
if (idx == iter_pos) {
zend_hash_iterators_update(in_hash, idx, offset + length);
iter_pos = zend_hash_iterators_lower_pos(in_hash, iter_pos + 1);
}
}
} else { /* otherwise just skip those entries */
int pos2 = pos;
Expand All @@ -3270,9 +3275,13 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
if (Z_TYPE_P(entry) == IS_UNDEF) continue;
pos2++;
zend_hash_packed_del_val(in_hash, entry);
/* Bump iterator positions to the element after replacement. */
if (idx == iter_pos) {
zend_hash_iterators_update(in_hash, idx, offset + length);
iter_pos = zend_hash_iterators_lower_pos(in_hash, iter_pos + 1);
}
}
}
iter_pos = zend_hash_iterators_lower_pos(in_hash, iter_pos);

/* If there are entries to insert.. */
if (replace) {
Expand Down