Skip to content

[Observer] Save opline before calling begin/end handlers #6421

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
wants to merge 2 commits into from

Conversation

SammyK
Copy link
Contributor

@SammyK SammyK commented Nov 10, 2020

This PR fixes a SIGSEGV that can occur on builds that have support for global register variables (gcc-global-regs). On those builds, when an extension requires access to the opline from an observer begin or end handler, the opline on the execute_data will be NULL or the wrong opcode in many cases. This showed up for us when we called zend_call_function from a begin handler and it crashed when the null opline was accessed.

cc/ @dstogov

@SammyK
Copy link
Contributor Author

SammyK commented Nov 10, 2020

The compiler on mac does not have support for global register variables, so the original issue doesn't occur on macOS builds. But after adding this patch I had to fix the build on macOS via 2b65e5b. The build error got me wondering what we need to do to save the opline for uncaught exceptions and ZEND_CALL_TRAMPOLINE. I'm a bit out of my depth on this one so any guidance would be much appreciated. :)

@dstogov
Copy link
Member

dstogov commented Nov 11, 2020

It looks like, independently from this patch observer API doesn't work with SWITCH and GOTO VM.
I'll try to take a look

@dstogov
Copy link
Member

dstogov commented Nov 11, 2020

Merged as 855d8fa

@dstogov dstogov closed this Nov 11, 2020
@SammyK
Copy link
Contributor Author

SammyK commented Nov 11, 2020

Thanks @dstogov!

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.

2 participants