Skip to content

Conversation

@mhagstrand
Copy link
Contributor

No description provided.

@mhagstrand
Copy link
Contributor Author

get_object_vars hashes the numeric string when exporting it the symbols table

(gdb) print ((Bucket)$hash->arData[0]).h
$2 = 9223372043238033807

In the symbols table numeric strings are not hashed, There numeric value is used as the hash value:

(gdb) print ((Bucket)$hash2->arData[0]).h
$9 = 1234

@mhagstrand
Copy link
Contributor Author

It is causing a segfault in ext/spl/tests/array_017.phpt I need to look into

@mhagstrand
Copy link
Contributor Author

Fixed the issue with ext/spl/tests/array_017.phpt

@krakjoe krakjoe added the Bug label Jan 31, 2017
fast_copy = 1;
/* Check if the object has a numeric property, See Bug 73998 */
ZEND_HASH_FOREACH_STR_KEY(properties, key) {
if (ZEND_HANDLE_NUMERIC(key, index)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to check that key is non-null.

@mhagstrand
Copy link
Contributor Author

Thanks, I have added a check to make sure the key is not null.

}

if (fast_copy) {
/* fast copy */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment is redundant now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, I will remove it.

@@ -0,0 +1,19 @@
--TEST--
Bug #35239 (array_key_exists fails on arrays created by get_object_vars)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong bug ID here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I copied another test as a basic template! 😄 Thanks for catching that.

@nikic
Copy link
Member

nikic commented Feb 2, 2017

Merged via dd9cf23, thanks!

@nikic nikic closed this Feb 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants