Skip to content

Fix #81679: Tracing JIT crashes on reattaching #7776

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

cmb69
Copy link
Member

@cmb69 cmb69 commented Dec 14, 2021

When a new process reattaches to OPcache, tracing JIT causes segfaults,
because each new process allocates its own zend_jit_traces and
zend_jit_exit_groups in SHM, although these need to be shared between
all processes.

We solve that by only allocating these structs for the first process,
and store the pointers in accel_shared_globals, so we can reassign
them when a new process reattaches.


Well, this is more like a request for help than an actual pull request:

  • It is unclear to me where to store the pointers; accel_shared_globals seems viable, but there is likely a better solution. Anyway, the void *jit_traces should get the proper type.
  • That shared memory would need to be locked, and to be (un)protected.
  • Is it correct that JIT_G(exit_counters) is still allocated in process memory?

@dstogov, you may want to have a look at this. As is, tracing JIT apparently can't work in "reattaching environments" (i.e. most Windows stacks, except for Apache mpm_winnt). An alternative solution to let zend_jit_startup() fail when reattaching, won't work, since the OPcode JIT handlers are still going to be executed.

When a new process reattaches to OPcache, tracing JIT causes segfaults,
because each new process allocates its own `zend_jit_traces` and
`zend_jit_exit_groups` in SHM, although these need to be shared between
all processes.

We solve that by only allocating these structs for the first process,
and store the pointers in `accel_shared_globals`, so we can reassign
them when a new process reattaches.
@cmb69
Copy link
Member Author

cmb69 commented Dec 15, 2021

Is this really good for PHP-8.0?

@dstogov
Copy link
Member

dstogov commented Dec 15, 2021

Is this really good for PHP-8.0?

Yeah. It's OK for PHP-8.0. opcache shared globals are not exported to other extensions.

@cmb69 cmb69 marked this pull request as ready for review December 15, 2021 14:41
@cmb69 cmb69 closed this in 49380b5 Dec 15, 2021
@cmb69 cmb69 deleted the cmb/81679 branch December 15, 2021 14:47
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