Fix invalid target opline with jit->reuse_ip active

[bwoebi]

When the JIT jumps out of tracing while reuse_ip is set, it will jump directly to the reused pointer, which might be e.g. EX(call) instead of EX(opline).

I honestly don't know how to properly give an isolated reproducer, but the gist is that the tracing JIT was bailing out on a DO_UCALL opcode...

I'm not sure whether that fix is correct, but at least I could no longer reproduce it later.

[dstogov]

I recently saw and fix the same problem, but I thought it's related to my new changes in #16365 (it's not merged yet).

I think your fix is right (it needs few minor tweaks).
It would be very useful to see the TRACE where the "invalid" zend_jit_trace_return() occurs.

[dstogov]

I think the check for original_handler is useless, but the check for jit->reuse_ip may be not enough.

[bwoebi]

Wanted to leave minimal impact on codepaths, but maybe it's useless. I don't understand the code well enough.

[dstogov]

This change is useless.

[bwoebi]

I thought so, but wasn't sure.

[dstogov]

#16365 fixed the similar problem by adding the following code at the end of zend_jit_init_method_call()

+	if (trace
+	 && trace->op == ZEND_JIT_TRACE_END
+	 && trace->stop >= ZEND_JIT_TRACE_STOP_INTERPRETER) {
+		if (!zend_jit_set_ip(jit, opline + 1)) {
+			return 0;
+		}
+	}

[bwoebi]

@dstogov But that means that you would have to put this code into every single zend_jit_* function where IP reuse is active, right?
At least #16365 doesn't fix the issue I encountered (just tested it).

[bwoebi]

And I also have a self-contained reproducer now:

<?php

function gen() {
    yield 1;
    yield 2;
    return 3;
};

opcache_jit_blacklist(gen(...));

for ($i = 0; $i < 2; ++$i) {
    foreach (gen() as $val) {
        var_dump($val);
    }
}

with

./sapi/cli/php -d zend_extension=$(pwd)/modules/opcache.so -d opcache.jit=tracing -d opcache.jit_hot_loop=1 -d opcache.enable_cli=1

[dstogov]

@dstogov But that means that you would have to put this code into every single zend_jit_* function where IP reuse is active, right? At least #16365 doesn't fix the issue I encountered (just tested it).

Right, this is the reason I liked to see the test case.
Now I can take care about a better fix.. (I'm going to do it on top of yours).

[dstogov]

Unfortunately the fix for this problem becomes more complex than I thought.
p->opline of the last trace record for ZEND_JIT_TRACE_STOP_INTERPRETER doesn't necessary correspond to the next executed instruction (in many cases the next executed opline is unknown), so we can't use it as argument for zend_jit_set_ip().

Probably, I'll have to extend the solution from #16365 Anyway, I would prefer to merge it first.

[bwoebi]

I see. It was fine for fixing my issue, but probably not in the general case then.
I'd like to review your PR, but as you see, my understanding of assumptions you can and can't do in jit code is ... very limited.

[bwoebi]

Closing as fixed by #16457.