Skip to content

8.4 tracing JIT phpseclib failures #16996

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
danog opened this issue Nov 29, 2024 · 2 comments
Closed

8.4 tracing JIT phpseclib failures #16996

danog opened this issue Nov 29, 2024 · 2 comments

Comments

@danog
Copy link
Contributor

danog commented Nov 29, 2024

Description

The following phpseclib testsuites fail on x86-64-v4 with tracing JIT only:

  • tests/Unit/Crypt/RC2Test.php

Reproducer: https://github.com/danog/jit_bugs, bugs/phpseclib.sh tracing /app/tests/Unit/Crypt/RC2Test.php

ping @dstogov

PHP Version

nightly

Operating System

No response

@nielsdos
Copy link
Member

I debugged this for a while and it seems like values from different CVs are ending up into different CVs. I think it's related to spill conflicts of CVs with ZEND_ASSIGN, but it's complicated to understand. Always returning 1 for IS_CV in zend_jit_spilling_may_cause_conflict does prevent the issue, so that hints that it's maybe indeed spill related.

dstogov added a commit to dstogov/php-src that referenced this issue Dec 3, 2024
This prevents conflicts caused by spilling to bound PHP stack slots by
creating copies.
@dstogov
Copy link
Member

dstogov commented Dec 3, 2024

@nielsdos Thanks for the initial analyses. This is definitely related to spill conflicts (but related to VAR and POST_INC).

These kind of conflicts are caused by binding of spill slots to PHP VM stack locations. (See slide 53 at https://www.researchgate.net/publication/374470404_IR_JIT_Framework_a_base_for_the_next_generation_JIT_for_PHP). It's possible (especially after IR optimizations) that other variables are loades/stored from/to the same memory locations.

I think, I found a fix for this case, but it doesn't solve the problem in general.

@dstogov dstogov closed this as completed in 89b82ef Dec 3, 2024
dstogov added a commit that referenced this issue Dec 3, 2024
* PHP-8.4:
  Fix GH-16996: 8.4 tracing JIT phpseclib failures (#17030)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants