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

Fix WeakMap object reference offset causing TypeError #8995

Merged
merged 2 commits into from
Jul 15, 2022
Merged

Fix WeakMap object reference offset causing TypeError #8995

merged 2 commits into from
Jul 15, 2022

Conversation

Nevay
Copy link
Contributor

@Nevay Nevay commented Jul 13, 2022

Fix TypeError when using object reference as WeakMap offset.

$a = new stdClass();
$b = &$a;
$map = new WeakMap();
$map[$a] = true;
Fatal error: Uncaught TypeError: WeakMap key must be an object

Zend/zend_weakrefs.c Outdated Show resolved Hide resolved
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you! That looks about right, although I don't quite understand the use case (why referencing an object?)

@Nevay
Copy link
Contributor Author

Nevay commented Jul 13, 2022

I stumbled across this while using a by-ref parameter which is stored in a WeakMap afterwards (ref-1, ref-2):

function doStuff(?object &$obj = null): void {
    $obj = new stdClass();
}
doStuff($obj);
$map = new WeakMap();
$map[$obj] = null;

@cmb69
Copy link
Member

cmb69 commented Jul 13, 2022

Ah, yeah, that makes sense (although I wish PHP had out parameters for such purposes).

@arnaud-lb arnaud-lb merged commit ede92a8 into php:PHP-8.0 Jul 15, 2022
@arnaud-lb
Copy link
Member

Thank you !

arnaud-lb added a commit that referenced this pull request Jul 15, 2022
* PHP-8.1:
  [ci skip] NEWS
  Fix `WeakMap` object reference offset causing `TypeError` (#8995)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants