Skip to content

Fix unsound SCCP for partial objects with hooks#21233

Open
iluuu1994 wants to merge 1 commit intophp:PHP-8.4from
iluuu1994:gh-21231
Open

Fix unsound SCCP for partial objects with hooks#21233
iluuu1994 wants to merge 1 commit intophp:PHP-8.4from
iluuu1994:gh-21231

Conversation

@iluuu1994
Copy link
Member

Fixes GH-21231

Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

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

I was wondering if __set would cause unsoundness as well, but it doesn't. Looks like that objects with __set are considered escaped in zend_ssa_escape_analysis(), and as a result we skip this optimization. Should we do the same for hooks?

Comment on lines +992 to +994
// FIXME: With dynamic property warnings and hooks this has
// become decreasingly useful. Maybe partial objects should just
// be removed.
Copy link
Member

Choose a reason for hiding this comment

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

I would expect the optimization to be useful, especially if we can enable it on typed props, when the value assigns cleanly.

@arnaud-lb
Copy link
Member

A similar issue happens with typed references:

#[AllowDynamicProperties]
class Foo {
}
class Bar {
    public object $x;
}

function test() {
    $bar = new Bar;
    $bar->x = new stdClass;
    $obj = new Foo;
    $obj->prop = &$bar->x;
    $obj->prop = 42;
    return $obj->prop;
}

var_dump(test()); // int(42); expected: TypeError: Cannot assign int to reference held by property Bar::$x of type object

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants