Skip to content

Disable JIT when pthread protection is unavailable#22853

Merged
iluuu1994 merged 11 commits into
php:masterfrom
realFlowControl:florian/fix-apple-silicon-zts-jit-runtime-check
Jul 24, 2026
Merged

Disable JIT when pthread protection is unavailable#22853
iluuu1994 merged 11 commits into
php:masterfrom
realFlowControl:florian/fix-apple-silicon-zts-jit-runtime-check

Conversation

@realFlowControl

@realFlowControl realFlowControl commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Follow up to #22712 (comment)

This moves the runtime capability check into zend_jit_check_support(). When pthread_jit_write_protect_supported_np() reports that per-thread JIT write protection is unavailable, OPcache now logs an ACCEL_LOG_WARNING, disables JIT, and continues using the interpreter. This matches the existing unsupported-JIT behavior.

JIT tests that modify opcache.jit at runtime now do so only when OPcache is inactive or its runtime status reports JIT as enabled. This preserves their original coverage on supported platforms without attempting to re-enable a JIT that the platform capability check disabled.

Testing

  • Clean Apple Silicon ZTS build.
  • Supported Apple Silicon JIT fork test passes.
  • Forced-unsupported OPcache suites matching the CI Test and Test OpCache configurations: 928 tests each, 0 failures.

@iluuu1994 iluuu1994 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you Florian! Looks correct to me. The PR is still marked as draft, let me know when you think it's ok to merge.

Comment thread ext/opcache/tests/jit/gh14267_001.phpt Outdated
===DONE===
--EXPECTREGEX--
^(?:Warning: Cannot change opcache\.jit setting at run-time \(JIT is disabled\) in .+ on line 2
)?===DONE===$

@iluuu1994 iluuu1994 Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't quite understand why this is a new problem? 🤔

Edit: Oh, I guess the JIT was just fully disabled at compile time and the INI setting didn't do anything.

@realFlowControl realFlowControl Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Exactly. After #22712 this behavior changed, but CI did not expose this particular follow-up problem because the initial test step printed a fatal error and failed. Consequently, the job never reached the later tracing/function JIT stages. See https://github.com/php/php-src/actions/runs/29793770728/job/88520997977

So the problem was already there with #22712, we just did not see it because CI failed before those stages were executed. 😉

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am also unsure whether this warning represents a practical user-facing problem. It can occur when the configure-time check confirms that the pthread_jit_write_protect_np() symbol is available, but the runtime capability probe pthread_jit_write_protect_supported_np() returns false.

So far, I have only observed this on GitHub’s virtualized macOS runners. On a physical M4 Mac, the runtime capability is available and JIT works as expected.

@iluuu1994

Copy link
Copy Markdown
Member

Okay, given pthread_jit_write_protect_supported_np() returns false in CI, I don't think actually running it makes sense. I think fa47ae1 should just be reverted. @arnaud-lb @realFlowControl Are you ok with that?

@arnaud-lb

Copy link
Copy Markdown
Member

Ok for me

@realFlowControl

Copy link
Copy Markdown
Contributor Author

Commited the revert in d84ae5b

@iluuu1994

Copy link
Copy Markdown
Member

Thanks, but I need to commit this to 8.2+ separately anyway.

@iluuu1994

iluuu1994 commented Jul 23, 2026

Copy link
Copy Markdown
Member

I reverted the change directly. I believe, once you rebase, the changes of the test files directly in ext/opcache/tests/ are no longer necessary.

@realFlowControl
realFlowControl force-pushed the florian/fix-apple-silicon-zts-jit-runtime-check branch from d84ae5b to 09aaa34 Compare July 24, 2026 05:48
@TimWolla
TimWolla removed their request for review July 24, 2026 11:07
Avoid regex sections by conditionally enabling JIT.

@iluuu1994 iluuu1994 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you Florian! Will merge once CI has finished.

<?php
// Skip when JIT was completely disabled at runtime.
if (ini_get('opcache.jit') !== 'disable') {
if (($status = opcache_get_status()) === false || $status['jit']['enabled']) {

@iluuu1994 iluuu1994 Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be:

if (($status = opcache_get_status()) !== false && $status['jit']['enabled']) {

? I guess it's fine to try to set it even with opcache disabled.

@iluuu1994
iluuu1994 merged commit 16da1f5 into php:master Jul 24, 2026
24 checks passed
@iluuu1994

Copy link
Copy Markdown
Member

Thanks again!

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.

3 participants