Skip to content

Commit d653646

Browse files
iluuu1994iluuu1994
committed
Fix iterator position resetting
Previously, when an array was converted from packed to hashed, iterators would not be correctly reset to 0. Similarly, removing the last element from an array would decrease nNumUsed but not actually fix the iterator position, causing the element to be skipped in the next iteration. Some code was also removed that skips over IS_UNDEF elements for nInternalPointer and iterator positions. This is unnecessary, as this already happens during iteration. Closes GH-13178 Closes GH-13188
1 parent 6fa4286 commit d653646

File tree

8 files changed

+166
-35
lines changed

8 files changed

+166
-35
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ Core:
99
. Changed the type of PHP_DEBUG and PHP_ZTS constants to bool. (haszi)
1010
. Fixed bug GH-13142 (Undefined variable name is shortened when contains \0).
1111
(nielsdos)
12+
. Fixed bug GH-13178 (Iterator positions incorrect when converting packed
13+
array to hashed). (ilutov)
1214

1315
Curl:
1416
. Deprecated the CURLOPT_BINARYTRANSFER constant. (divinity76)

‎Zend/tests/gh13178_1.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-13178: Packed to hash must reset iterator position
3+
--FILE--
4+
<?php
5+
$array = ['foo'];
6+
foreach ($array as $key => &$value) {
7+
var_dump($key);
8+
unset($array[$key]);
9+
$array[] = 'foo';
10+
if ($key === 10) {
11+
break;
12+
}
13+
}
14+
?>
15+
--EXPECT--
16+
int(0)
17+
int(1)
18+
int(2)
19+
int(3)
20+
int(4)
21+
int(5)
22+
int(6)
23+
int(7)
24+
int(8)
25+
int(9)
26+
int(10)

‎Zend/tests/gh13178_2.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-13178: Unsetting last offset must floor iterator position
3+
--FILE--
4+
<?php
5+
$array = [100 => 'foo'];
6+
foreach ($array as $key => &$value) {
7+
var_dump($key);
8+
unset($array[$key]);
9+
$array[] = 'foo';
10+
if ($key === 110) {
11+
break;
12+
}
13+
}
14+
?>
15+
--EXPECT--
16+
int(100)
17+
int(101)
18+
int(102)
19+
int(103)
20+
int(104)
21+
int(105)
22+
int(106)
23+
int(107)
24+
int(108)
25+
int(109)
26+
int(110)

‎Zend/tests/gh13178_3.phpt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
GH-13178: Unsetting last offset variation with hashed array
3+
--FILE--
4+
<?php
5+
6+
$data = ['foo' => 'foo', 'bar' => 'bar', 'baz' => 'baz'];
7+
8+
foreach ($data as $key => &$value) {
9+
var_dump($value);
10+
if ($value === 'baz') {
11+
unset($data['bar']);
12+
unset($data['baz']);
13+
$data['qux'] = 'qux';
14+
$data['quux'] = 'quux';
15+
}
16+
}
17+
18+
?>
19+
--EXPECT--
20+
string(3) "foo"
21+
string(3) "bar"
22+
string(3) "baz"
23+
string(3) "qux"
24+
string(4) "quux"

‎Zend/tests/gh13178_4.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GH-13178: Packed to hash must reset nInternalPointer
3+
--FILE--
4+
<?php
5+
$array = ['foo'];
6+
reset($array);
7+
while (true) {
8+
$key = key($array);
9+
next($array);
10+
var_dump($key);
11+
unset($array[$key]);
12+
$array[] = 'foo';
13+
if ($key === 10) {
14+
break;
15+
}
16+
}
17+
?>
18+
--EXPECT--
19+
int(0)
20+
int(1)
21+
int(2)
22+
int(3)
23+
int(4)
24+
int(5)
25+
int(6)
26+
int(7)
27+
int(8)
28+
int(9)
29+
int(10)

‎Zend/tests/gh13178_5.phpt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
GH-13178: Packed array with last elements removed must reset iterator positions
3+
--FILE--
4+
<?php
5+
$array = [0, 1, 2];
6+
foreach ($array as &$value) {
7+
var_dump($value);
8+
if ($value === 2) {
9+
unset($array[2]);
10+
unset($array[1]);
11+
$array[1] = 3;
12+
$array[2] = 4;
13+
}
14+
}
15+
?>
16+
--EXPECT--
17+
int(0)
18+
int(1)
19+
int(2)
20+
int(3)
21+
int(4)

‎Zend/zend_hash.c

Lines changed: 28 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,6 +1331,18 @@ ZEND_API void ZEND_FASTCALL zend_hash_rehash(HashTable *ht)
13311331
if (!(HT_FLAGS(ht) & HASH_FLAG_UNINITIALIZED)) {
13321332
ht->nNumUsed = 0;
13331333
HT_HASH_RESET(ht);
1334+
/* Even if the array is empty, we still need to reset the iterator positions. */
1335+
ht->nInternalPointer = 0;
1336+
if (UNEXPECTED(HT_HAS_ITERATORS(ht))) {
1337+
HashTableIterator *iter = EG(ht_iterators);
1338+
HashTableIterator *end = iter + EG(ht_iterators_used);
1339+
while (iter != end) {
1340+
if (iter->ht == ht) {
1341+
iter->pos = 0;
1342+
}
1343+
iter++;
1344+
}
1345+
}
13341346
}
13351347
return;
13361348
}
@@ -1412,32 +1424,30 @@ ZEND_API void ZEND_FASTCALL zend_hash_rehash(HashTable *ht)
14121424
}
14131425
}
14141426

1415-
static zend_always_inline void _zend_hash_packed_del_val(HashTable *ht, uint32_t idx, zval *zv)
1427+
static zend_always_inline void zend_hash_iterators_clamp_max(HashTable *ht, uint32_t max)
14161428
{
1417-
idx = HT_HASH_TO_IDX(idx);
1418-
ht->nNumOfElements--;
1419-
if (ht->nInternalPointer == idx || UNEXPECTED(HT_HAS_ITERATORS(ht))) {
1420-
uint32_t new_idx;
1421-
1422-
new_idx = idx;
1423-
while (1) {
1424-
new_idx++;
1425-
if (new_idx >= ht->nNumUsed) {
1426-
break;
1427-
} else if (Z_TYPE(ht->arPacked[new_idx]) != IS_UNDEF) {
1428-
break;
1429+
if (UNEXPECTED(HT_HAS_ITERATORS(ht))) {
1430+
HashTableIterator *iter = EG(ht_iterators);
1431+
HashTableIterator *end = iter + EG(ht_iterators_used);
1432+
while (iter != end) {
1433+
if (iter->ht == ht) {
1434+
iter->pos = MIN(iter->pos, max);
14291435
}
1436+
iter++;
14301437
}
1431-
if (ht->nInternalPointer == idx) {
1432-
ht->nInternalPointer = new_idx;
1433-
}
1434-
zend_hash_iterators_update(ht, idx, new_idx);
14351438
}
1439+
}
1440+
1441+
static zend_always_inline void _zend_hash_packed_del_val(HashTable *ht, uint32_t idx, zval *zv)
1442+
{
1443+
idx = HT_HASH_TO_IDX(idx);
1444+
ht->nNumOfElements--;
14361445
if (ht->nNumUsed - 1 == idx) {
14371446
do {
14381447
ht->nNumUsed--;
14391448
} while (ht->nNumUsed > 0 && (UNEXPECTED(Z_TYPE(ht->arPacked[ht->nNumUsed-1]) == IS_UNDEF)));
14401449
ht->nInternalPointer = MIN(ht->nInternalPointer, ht->nNumUsed);
1450+
zend_hash_iterators_clamp_max(ht, ht->nNumUsed);
14411451
}
14421452
if (ht->pDestructor) {
14431453
zval tmp;
@@ -1458,28 +1468,12 @@ static zend_always_inline void _zend_hash_del_el_ex(HashTable *ht, uint32_t idx,
14581468
}
14591469
idx = HT_HASH_TO_IDX(idx);
14601470
ht->nNumOfElements--;
1461-
if (ht->nInternalPointer == idx || UNEXPECTED(HT_HAS_ITERATORS(ht))) {
1462-
uint32_t new_idx;
1463-
1464-
new_idx = idx;
1465-
while (1) {
1466-
new_idx++;
1467-
if (new_idx >= ht->nNumUsed) {
1468-
break;
1469-
} else if (Z_TYPE(ht->arData[new_idx].val) != IS_UNDEF) {
1470-
break;
1471-
}
1472-
}
1473-
if (ht->nInternalPointer == idx) {
1474-
ht->nInternalPointer = new_idx;
1475-
}
1476-
zend_hash_iterators_update(ht, idx, new_idx);
1477-
}
14781471
if (ht->nNumUsed - 1 == idx) {
14791472
do {
14801473
ht->nNumUsed--;
14811474
} while (ht->nNumUsed > 0 && (UNEXPECTED(Z_TYPE(ht->arData[ht->nNumUsed-1].val) == IS_UNDEF)));
14821475
ht->nInternalPointer = MIN(ht->nInternalPointer, ht->nNumUsed);
1476+
zend_hash_iterators_clamp_max(ht, ht->nNumUsed);
14831477
}
14841478
if (ht->pDestructor) {
14851479
zval tmp;

‎ext/standard/array.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3262,6 +3262,11 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
32623262
Z_TRY_ADDREF_P(entry);
32633263
zend_hash_next_index_insert_new(removed, entry);
32643264
zend_hash_packed_del_val(in_hash, entry);
3265+
/* Bump iterator positions to the element after replacement. */
3266+
if (idx == iter_pos) {
3267+
zend_hash_iterators_update(in_hash, idx, offset + length);
3268+
iter_pos = zend_hash_iterators_lower_pos(in_hash, iter_pos + 1);
3269+
}
32653270
}
32663271
} else { /* otherwise just skip those entries */
32673272
int pos2 = pos;
@@ -3270,9 +3275,13 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
32703275
if (Z_TYPE_P(entry) == IS_UNDEF) continue;
32713276
pos2++;
32723277
zend_hash_packed_del_val(in_hash, entry);
3278+
/* Bump iterator positions to the element after replacement. */
3279+
if (idx == iter_pos) {
3280+
zend_hash_iterators_update(in_hash, idx, offset + length);
3281+
iter_pos = zend_hash_iterators_lower_pos(in_hash, iter_pos + 1);
3282+
}
32733283
}
32743284
}
3275-
iter_pos = zend_hash_iterators_lower_pos(in_hash, iter_pos);
32763285

32773286
/* If there are entries to insert.. */
32783287
if (replace) {

0 commit comments

Comments
 (0)