Skip to content

Assertion `(flag & (1<<3)) == 0' failed. #10251

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

Closed
Changochen opened this issue Jan 6, 2023 · 5 comments
Closed

Assertion `(flag & (1<<3)) == 0' failed. #10251

Changochen opened this issue Jan 6, 2023 · 5 comments

Comments

@Changochen
Copy link

Description

The following code:

<?php
class A
{
    function __set($o, $l)
    {
        $this->$p = $v;
    }
}
$a = new A();
$pp = "";
$op = $pp & "";
$a->$op = 0;
?>

Resulted in this output:

/Zend/zend_hash.c:763: zval *_zend_hash_add_or_update_i(HashTable *, zend_string *, zval *, uint32_t): Assertion `(flag & (1<<3)) == 0' failed.

Build config:

./configure --disable-all --enable-address-sanitizer --disable-phpdbg --disable-cgi --with-pic --enable-debug-assertions
make -j

PHP Version

PHP 8.3.0-dev

Operating System

No response

@nielsdos
Copy link
Member

nielsdos commented Jan 6, 2023

I confirm this happens on 8.1 and higher.

Analysis:

Upon attempting to store the empty string in the magic set method we first reach this code

/* mark pointer as "special" using low bit */
zend_hash_add_new_ptr(guards, str,
(void*)(((zend_uintptr_t)&Z_PROPERTY_GUARD_P(zv)) | 1));

but the strings str and member are actually equal content-wise (their pointers are different, but the characters are the same: both the empty string). This shouldn't happen because of the string equality check above. Later we execute the following:
return (uint32_t*)zend_hash_add_new_ptr(guards, member, ptr);

And now a new entry isn't actually added because the first snippet I linked added it. Therefore the assertion fails.

Cause:

The following check should've prevented this:

if (EXPECTED(str == member) ||
/* "str" always has a pre-calculated hash value here */
(EXPECTED(ZSTR_H(str) == zend_string_hash_val(member)) &&
EXPECTED(zend_string_equal_content(str, member)))) {

But it turns out that the hash of str is zero here! I did a quick test by changing ZSTR_H(str) to zend_string_hash_val(str) and now the OP's code no longer crashes, indicating that the hash being zero is indeed the problem.

@Girgias
Copy link
Member

Girgias commented Jan 7, 2023

I would expect that any empty string to be "optimized" to the interned zend_empty_string zend_string which has the hash precomputed. So it seems the is an issue with the bitwise & operator (which might be generalized to other operators)

@nielsdos
Copy link
Member

nielsdos commented Jan 7, 2023

Yeah it also breaks with bitwise |.

@nikic
Copy link
Member

nikic commented Jan 7, 2023

The line comparing hashes should just be dropped. member is not guaranteed to have a hash, and member is assigned to str, so it doesn't necessarily have a hash either. (Or else we'd have to force a hash calculation when assigning str.)

@nielsdos
Copy link
Member

nielsdos commented Jan 7, 2023

I see, I'll make a PR for that change this afternoon.

nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 7, 2023
zend_get_property_guard previously assumed that at least "str" has a
pre-computed hash. This is not always the case, for example when a
string is created by bitwise operations, its hash is not set. Instead of
forcing a computation of the hashes, drop the hash comparison.
nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 7, 2023
Girgias added a commit that referenced this issue Jan 8, 2023
* PHP-8.1:
  Fix GH-10251: Assertion `(flag & (1<<3)) == 0' failed.
  Fix GH-9710: phpdbg memory leaks by option "-h"
@Girgias Girgias closed this as completed in d03025b Jan 8, 2023
Girgias added a commit that referenced this issue Jan 8, 2023
* PHP-8.2:
  Fix GH-10251: Assertion `(flag & (1<<3)) == 0' failed.
  Fix GH-9710: phpdbg memory leaks by option "-h"
nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
@nikic @nielsdos @Girgias @Changochen and others