php / php-src

The PHP Interpreter
https://www.php.net
Other
38.33k stars 7.76k forks source link

Assertion failure with tracing JIT #9011

Open cmb69 opened 2 years ago

cmb69 commented 2 years ago

Description

The following code (script provided by @zeriyoshi):

<?php

$foo = [];
$foo[] = new \Exception(); /* Native interface implemented Native instance */
$foo[] = new class () implements \Stringable /* Native interface implemented User instance */
{
    public function __toString(): string
    {
        return "bar";
    }
};

foreach ($foo as $baz) {
    for ($i = 0; $i < 64; $i++) {
        $baz->__toString();
    }
}

Resulted in this output:

Assertion failed: ((execute_data)->opline) >= ((execute_data)->func)->op_array.opcodes && ((execute_data)->opline) < ((execute_data)->func)->op_array.opcodes + ((execute_data)->func)->op_array.last, file C:\php-sdk\phpdev\vs16\x64\php-src-8.0\ext\opcache\jit/zend_jit_trace.c, line 7679

https://github.com/php/php-src/blob/789a37f14405e2d1a05a76c9fb4ed2d49d4580d5/ext/opcache/jit/zend_jit_trace.c#L7678-L7679

But I expected this output instead:

PHP Version

= PHP-8.0

Operating System

Windows, maybe others as well

nielsdos commented 1 day ago

I've debugged this for a while and can only reproduce this on Windows. It seemed related to the handling of side exits with internal functions. I noticed side exit generation code here: https://github.com/php/php-src/blob/b06f2bc67c3e47dfb686a8ef4e1e7a3447b9cd5d/ext/opcache/jit/zend_jit_ir.c#L9020-L9026 The last part of the check looked suspicious to me. Why do we skip internal functions on Windows? I would expect the opposite: because of ASLR we do have to make a guard that checks if the function address is unchanged. Therefore, we must execute this code for internal functions as well on Windows to get the potential side exit.

The reason this fixes the issue here is because in the trace we initially inferred the function to execute being Exception::__toString, but in the second outer iteration we must execute a userland function. Because the initial trace used an internal function, the guard is not generated and so the generated code will use the same function again instead of the userland function, leading to the assertion failure.

Patch made on branch PHP-8.4. (On master, more places may need changes, I could do this if this is the right approach)

diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c
index e4b68d23520..a1612948782 100644
--- a/ext/opcache/jit/zend_jit_ir.c
+++ b/ext/opcache/jit/zend_jit_ir.c
@@ -9006,9 +9006,6 @@ static int zend_jit_init_method_call(zend_jit_ctx         *jit,
     && trace
     && trace->op == ZEND_JIT_TRACE_INIT_CALL
     && trace->func
-#ifdef _WIN32
-    && trace->func->type != ZEND_INTERNAL_FUNCTION
-#endif
    ) {
        int32_t exit_point;
        const void *exit_addr;

I thought first, an alternative solution is maybe to check if the function type is still the same, but that would not be enough probably because of ASLR.

I could be wrong. cc @dstogov Also, this would need to be fixed in 8.2 too, but I wanted to show the patch for IR JIT because IR JIT is easier to understand.