ext/opcache/jit/zend_jit_trace: add missing lock for EXIT_INVALIDATE#10138
ext/opcache/jit/zend_jit_trace: add missing lock for EXIT_INVALIDATE#10138MaxKellermann wants to merge 1 commit intophp:PHP-8.1from
Conversation
|
AppVeyor failure unrelated to this PR. |
|
This looks good to me @dstogov do you confirm? |
dstogov
left a comment
dstogov
left a comment
There was a problem hiding this comment.
The additional lock is right of course but probably not enough. After the lock we should check some race conditions. E.g.
/* Checks under lock */
if (!(ZEND_OP_TRACE_INFO(opline, offset)->trace_flags & (ZEND_JIT_TRACE_JITED|ZEND_JIT_TRACE_BLACKLISTED))) {
/* skip */;
} else if (ZEND_JIT_TRACE_NUM >= JIT_G(max_root_traces)) {
/* skip */;Also it seems like, invalidating doesn't reset JITED/BLACKLISTED flags that should prevent re-compilation. Could you please take care about this.
Oh, I missed. These flag are already properly reset. So we need just additional checks to prevent "double" invalidations. |
Commit 6c25413 added the flag ZEND_JIT_EXIT_INVALIDATE which resets the trace handlers in zend_jit_trace_exit(), but forgot to lock the shared memory section. This could cause another worker process who still saw the ZEND_JIT_TRACE_JITED flag to schedule ZEND_JIT_TRACE_STOP_LINK, but when it arrived at the ZEND_JIT_DEBUG_TRACE_STOP, the handler was already reverted by the first worker process and thus zend_jit_find_trace() fails. This in turn generated a bogus jump offset in the JITed code, crashing the PHP process.
MaxKellermann
force-pushed
the
exit_invalidate_lock
branch
from
feffcce to
c5cccc8
Compare
Thanks @dstogov, I added the checks you posted, but I'm not sure what else may be missing - I only recently read the JIT code for the first time, and those internals are still obscure to me. Please check if this is ok for merging. |
|
Merged via e217138b. Thank you |
4 participants
Commit 6c25413 added the flag ZEND_JIT_EXIT_INVALIDATE which resets the trace handlers in zend_jit_trace_exit(), but forgot to lock the shared memory section.
This could cause another worker process who still saw the ZEND_JIT_TRACE_JITED flag to schedule ZEND_JIT_TRACE_STOP_LINK, but when it arrived at the ZEND_JIT_DEBUG_TRACE_STOP, the handler was already reverted by the first worker process and thus zend_jit_find_trace() fails.
This in turn generated a bogus jump offset in the JITed code, crashing the PHP process.