Skip to content

Handle trampolines correctly in new FCC API + usages #9877

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

Merged
merged 3 commits into from
Nov 22, 2022

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Nov 2, 2022

So I finally understand what php-src calls trampolines, and they are extremely annoying to deal with.

I've taken the code to handle them from the SPL class autoloading mechanism, however I haven't yet figured out why SPL copies the function handler when fcc->function_handler == &EG(trampoline).

This makes me wonder if this is the reason why no FCC API was introduced previously as you would need to carry the zval callable around which would would recreate the trampoline each time.

Moreover, the refetching of the trampoline which is freed by ZPP should probably be part of some explicit API that we provide if in the end we thing providing an FCC API is sensible.

@arnaud-lb
Copy link
Member

I haven't yet figured out why SPL copies the function handler when fcc->function_handler == &EG(trampoline)

My understanding is that EG(trampoline) is an allocation cache slot to store a trampoline. It is used to avoid an allocation in zend_get_call_trampoline_func. The slot is considered to be free to use when EG(trampoline).common.function_name == NULL.

SPL copies EG(trampoline) and resets EG(trampoline).common.function_name to make the slot available again to future zend_get_call_trampoline_func calls.

@Girgias
Copy link
Member Author

Girgias commented Nov 4, 2022

I haven't yet figured out why SPL copies the function handler when fcc->function_handler == &EG(trampoline)

My understanding is that EG(trampoline) is an allocation cache slot to store a trampoline. It is used to avoid an allocation in zend_get_call_trampoline_func. The slot is considered to be free to use when EG(trampoline).common.function_name == NULL.

SPL copies EG(trampoline) and resets EG(trampoline).common.function_name to make the slot available again to future zend_get_call_trampoline_func calls.

Ah, so I suppose it's a wise idea to free this when doing the copy. :)

@Girgias Girgias force-pushed the fcc-api-trampolines-fixes branch 2 times, most recently from c322a0f to 1699bd7 Compare November 15, 2022 09:14
Zend/zend_API.h Outdated
memcpy(copy, fcc->function_handler, sizeof(zend_function));
fcc->function_handler->common.function_name = NULL;
fcc->function_handler = copy;
memset(&EG(trampoline), 0, sizeof(zend_function));
Copy link
Member

Choose a reason for hiding this comment

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

This is redundant with the fcc->function_handler->common.function_name = NULL above.

It may be enough to just set EG(trampoline).common.function_name to NULL (this is what zend_free_trampoline does, and EG(trampoline).common.function_name only looks at EG(trampoline).common.function_name to check that the slot is free).

@Girgias Girgias force-pushed the fcc-api-trampolines-fixes branch from 1699bd7 to d369627 Compare November 22, 2022 13:07
@Girgias Girgias merged commit 32d3cae into php:master Nov 22, 2022
@Girgias Girgias deleted the fcc-api-trampolines-fixes branch November 22, 2022 17:12
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