-
Notifications
You must be signed in to change notification settings - Fork 8k
Fix GH-13433: Segmentation Fault in zend_class_init_statics when using opcache.preload #13794
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
Conversation
...sing opcache.preload This regressed in 9a250cc, which allowed static properties to get overridden by a trait during inheritance. In particular, because of the change to the loop in zend_update_parent_ce(), it's not guaranteed that all indirects are after one another. This means that during persisting the zvals of the static members table, some static properties may be skipped. In case of the test code, this means that the array in the trait will keep referring to the old, new freed, stale value. To solve this, we check the type for IS_INDIRECT, which is the same as what zend_persist_calc() is already doing anyway. Since 2543e61 we can check for IS_INDIRECT to see if it should be persisted or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good to me, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this included again? Shouldn't preloading take care of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking the PR.
You are right, this is not necessary, I don't remember why I did this. I checked to be sure: PHP without this PR still reproduces the case without these includes.
Uh oh!
There was an error while loading. Please reload this page.
This regressed in 9a250cc, which allowed static properties to get overridden by a trait during inheritance. In particular, because of the change to the loop in zend_update_parent_ce(), it's not guaranteed that all indirects are after one another.
This means that during persisting the zvals of the static members table, some static properties may be skipped by the current code, which is wrong. In case of the test code, this means that the array property in the class
TestClasswill keep referring to the old, new freed, stale value. The static properties inTraitAandParentClassare however updated. To solve this, we check the type for IS_INDIRECT, which is the same as what zend_persist_calc() is already doing anyway.Since 2543e61 we can check for IS_INDIRECT to see if it should be persisted or not.
Test with ASAN to see the bug.