ext/opcache/jit/zend_jit_trace: add missing lock for EXIT_INVALIDATE

[MaxKellermann]

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]

AppVeyor failure unrelated to this PR.

[arnaud-lb]

This looks good to me

@dstogov do you confirm?

[dstogov]

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.

[dstogov]

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.

[MaxKellermann]

After the lock we should check some race conditions

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.

[dstogov]

This looks fine.

[devnexen]

Merged via e217138b. Thank you