Skip to content
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

IR JIT: too long jmp distance in ir_add_veneer, ir_aarch64.dasc:6495. #17858

Open
danog opened this issue Feb 18, 2025 · 8 comments
Open

IR JIT: too long jmp distance in ir_add_veneer, ir_aarch64.dasc:6495. #17858

danog opened this issue Feb 18, 2025 · 8 comments

Comments

@danog
Copy link
Contributor

danog commented Feb 18, 2025

Description

Running ./psalm --no-cache in https://github.com/vimeo/psalm/tree/preload_all_classes_tmp on aarch64 (mac OS 15.3.1, not reproducible on linux aarch64 on a KVM in the same machine) triggers the following assertion:

Assertion failed: (0 && "too long jmp distance"), function ir_add_veneer, file ir_aarch64.dasc, line 6495.

On 3b23de3, the following assertion is triggered instead:

Assertion failed: (_blocks[use] == b || use_insn->op == IR_PARAM), function ir_schedule, file ir_gcm.c, line 948.

The issue is triggered by the large number of class_exists statements in src/Psalm/Internal/Preloader.php, reducing the number to < ~1020 fixes the issue.

PHP Version

PHP 8.4.4 or 3b23de3

Operating System

Mac OS 15.3.1

@danog
Copy link
Contributor Author

danog commented Feb 18, 2025

Ping @dstogov

danog added a commit to danog/psalm that referenced this issue Feb 18, 2025
@dstogov
Copy link
Member

dstogov commented Feb 19, 2025

Thanks for the report.
Can you reproduce the issue running php -d opcache.jit=1205 Preloader.php
The second assertion looks unrelated.

@dstogov
Copy link
Member

dstogov commented Feb 19, 2025

@danog could you please check the patch.
It should fix only the first problem, I need a reproduction case for the second one.

diff --git a/ext/opcache/jit/ir/ir_aarch64.dasc b/ext/opcache/jit/ir/ir_aarch64.dasc
index 772eea7a5d7..67bb0cdaea1 100644
--- a/ext/opcache/jit/ir/ir_aarch64.dasc
+++ b/ext/opcache/jit/ir/ir_aarch64.dasc
@@ -6441,6 +6441,8 @@ static int ir_add_veneer(dasm_State *Dst, void *buffer, uint32_t ins, int *b, ui
 		if (ctx->get_veneer) {
 			veneer = ctx->get_veneer(ctx, addr);
 		}
+	} else if ((ins >> 16) == DASM_REL_PC) {
+		addr = (char*)cp - 4 + offset;
 	} else {
 		IR_ASSERT(0 && "too long jmp distance");
 		return 0;
@@ -6525,7 +6527,9 @@ static int ir_add_veneer(dasm_State *Dst, void *buffer, uint32_t ins, int *b, ui
 		return 0;
 	}
 
-	if (!ctx->set_veneer || !ctx->set_veneer(ctx, addr, veneer)) {
+	if ((ins >> 16) == DASM_REL_PC) {
+	  *DASM_POS2PTR(Dst, *(b - 1)) = (int)((char*)addr - (char*)buffer);
+	} else if (!ctx->set_veneer || !ctx->set_veneer(ctx, addr, veneer)) {
 		IR_ASSERT(0 && "too long jmp distance");
 		return 0;
 	}
'''

@danog
Copy link
Contributor Author

danog commented Feb 19, 2025

@dstogov The second can be easily reproduced by running ./psalm --no-cache on 3b23de3, with the opcache extension loaded via php.ini.

Trying with the patch applied on top of PHP-8.4 now...

@danog
Copy link
Contributor Author

danog commented Feb 19, 2025

@dstogov your patch doesn't fix the issue when applied on PHP-8.4:

daniil@MBP-di-Daniil php-src % git diff
diff --git a/ext/opcache/jit/ir/ir_aarch64.dasc b/ext/opcache/jit/ir/ir_aarch64.dasc
index 772eea7a5d..67bb0cdaea 100644
--- a/ext/opcache/jit/ir/ir_aarch64.dasc
+++ b/ext/opcache/jit/ir/ir_aarch64.dasc
@@ -6441,6 +6441,8 @@ static int ir_add_veneer(dasm_State *Dst, void *buffer, uint32_t ins, int *b, ui
                if (ctx->get_veneer) {
                        veneer = ctx->get_veneer(ctx, addr);
                }
+       } else if ((ins >> 16) == DASM_REL_PC) {
+               addr = (char*)cp - 4 + offset;
        } else {
                IR_ASSERT(0 && "too long jmp distance");
                return 0;
@@ -6525,7 +6527,9 @@ static int ir_add_veneer(dasm_State *Dst, void *buffer, uint32_t ins, int *b, ui
                return 0;
        }

-       if (!ctx->set_veneer || !ctx->set_veneer(ctx, addr, veneer)) {
+       if ((ins >> 16) == DASM_REL_PC) {
+         *DASM_POS2PTR(Dst, *(b - 1)) = (int)((char*)addr - (char*)buffer);
+       } else if (!ctx->set_veneer || !ctx->set_veneer(ctx, addr, veneer)) {
                IR_ASSERT(0 && "too long jmp distance");
                return 0;
        }
daniil@MBP-di-Daniil psalm % git diff
diff --git a/src/Psalm/Internal/Fork/PsalmRestarter.php b/src/Psalm/Internal/Fork/PsalmRestarter.php
index 5a702e137..01808ffd7 100644
--- a/src/Psalm/Internal/Fork/PsalmRestarter.php
+++ b/src/Psalm/Internal/Fork/PsalmRestarter.php
@@ -166,7 +166,7 @@ final class PsalmRestarter extends XdebugHandler
         // executed in the parent process (before restart)
         // if it wasn't loaded then we apparently don't have opcache installed and there's no point trying
         // to tweak it
-        $additional_options = $opcache_loaded ? [] : ['-dzend_extension=opcache'];
+        $additional_options = $opcache_loaded ? [] : ['-dzend_extension=/Users/daniil/repos/php-src/./ext/opcache/.libs/opcache.so'];
         foreach (self::REQUIRED_OPCACHE_SETTINGS as $key => $value) {
             $additional_options []= "-dopcache.{$key}={$value}";
         }
daniil@MBP-di-Daniil psalm % ~/repos/php-src/sapi/cli/php ./psalm --no-cache
php(75602,0x1fa134840) malloc: nano zone abandoned due to inability to reserve vm space.
php(75605,0x1fa134840) malloc: nano zone abandoned due to inability to reserve vm space.

JIT acceleration: ON

JIT compilation in progress... Assertion failed: (0 && "too long jmp distance"), function ir_add_veneer, file ir_aarch64.dasc, line 6497.

@dstogov
Copy link
Member

dstogov commented Feb 19, 2025

It seems you have a different problem, that I can't reproduce (I don't have Mac).

@danog
Copy link
Contributor Author

danog commented Feb 19, 2025

https://github.com/sickcodes/Docker-OSX might help

@iluuu1994
Copy link
Member

@danog FWIW I tried that in the past and couldn't get it working.

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

4 participants