Skip to content
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.

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

Weird pointers issue in nested loops #12826

Closed
bloatware opened this issue Nov 29, 2023 · 2 comments
Closed

Weird pointers issue in nested loops #12826

bloatware opened this issue Nov 29, 2023 · 2 comments

Comments

@bloatware
Copy link

Description

Hi,

I observe a weird issue in php 8.3.0 with the following (weird) code:

<?php
$test = array(
  'a' => 1,
  'b' => 2,
  'c' => 3,
  'd' => 4,
);

//unset($test['a']);
//unset($test['b']);
//unset($test['c']);
//unset($test['d']);

foreach($test as $k => &$v) { // Mind the reference!
    echo "Pass $k : ";

    foreach($test as $kk => $vv) {
        echo $test[$kk];
        if ($kk == $k) $test[$kk] = 0;
    }

    echo '<br />';
}

unset($v);

As expected, it outputs:

Pass a : 1234
Pass b : 0234
Pass c : 0034
Pass d : 0004

But if I unset, say $test['a'] and $test['b'] before the loop, the server seems to enter some kind of infinite loop. Unsetting only one element or only few last elements does not seem to yield any problem. When looping by value $k => $v everything is fine of course.

The problem seems to come from $test[$kk] = 0 assignment. If I comment it out, everything is fine again, whichever elements are unset.

I've tested it on few servers, with the same result. The code works without any issue in php from 5.4 to 8.2.

PHP Version

PHP 8.3.0

Operating System

Various

@nielsdos
Copy link
Member

Can confirm, reverting cd53ce8 fixes this.

@nielsdos
Copy link
Member

This loops hangs forever: iter_pos is equal to nNumUsed (2) and can't move further:

php-src/Zend/zend_hash.c

Lines 2419 to 2422 in e3de478

do {
zend_hash_iterators_update(target, iter_pos, target_idx);
iter_pos = zend_hash_iterators_lower_pos(target, iter_pos + 1);
} while (iter_pos < idx);

However, I find it odd because we are looping from p to end, which is from p to p+4 in the example code.
Then I see

target->nNumUsed = source->nNumOfElements;

which seems strange because now the number of used slots is equal to the number of elements, but we are still looping over all the buckets including the undef ones.
When I change that line to target->nNumUsed = source->nNumUsed; instead it seems to work...

nielsdos added a commit to nielsdos/php-src that referenced this issue Nov 29, 2023
This regressed in cd53ce8.
The loop with `zend_hash_iterators_update` hangs forever because
`iter_pos` can't advance to idx. This is because the
`zend_hash_iterators_lower_pos` upper bound is `target->nNumUsed`,
but that is set to `source->nNumOfElements`.
That means that if there are holes in the array, we still loop over all
the buckets but the number of bucket slots will not match.
Fix it by changing the assignment.
@nielsdos nielsdos linked a pull request Nov 29, 2023 that will close this issue
nielsdos added a commit that referenced this issue Dec 1, 2023
* PHP-8.3:
  Fix GH-12826: Weird pointers issue in nested loops
  Fix GH-12838: [SOAP] Temporary WSDL cache files not being deleted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants