RFR: 8302351: "assert(!JavaThread::current()->is_interp_only_mode() || !nm->method()->is_continuation_enter_intrinsic() || ContinuationEntry::is_interpreted_call(return_pc)) failed: interp_only_mode but not in enterSpecial interpreted entry" in fixup_callers_callsite

[Patricio Chilano Mateo]

On Tue, 23 May 2023 22:07:37 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

> Please review the following fix. Runtime methods called through the SharedRuntime::generate_resolve_blob() stub always return the value stored in _from_compiled_entry as the entry point to the callee method. This will either be the entry point to the compiled version of callee if there is one or the c2i adapter entry point. But this doesn't consider the case where an EnterInterpOnlyModeClosure handshake catches the JavaThread in the transition back to Java on those methods. In that case we should return the c2i adapter entry point even if there is a compiled entry point. Otherwise the JavaThread will continue calling the compiled versions of methods without noticing it's in interpreted only mode until it either calls a method that hasn't been compiled yet or it returns to the caller of that resolved callee where the change to interpreter only mode happened (since the EnterInterpOnlyModeClosure handshake marked all the frames on the stack for deoptimization). 
> 
> This is a long standing bug but has been made visible with the assert added as part of 8288949 where a related issue was fixed. There are more details in the bug comments about how this specific crash happens and its relation with 8288949. I also attached a reproducer.
> 
> These runtime methods are already using JRT_BLOCK_ENTRY/JRT_BLOCK so that the entry point to the callee is fetched only after the last possible safepoint in JRT_BLOCK_END. This guarantees that we will not return an entry point to compiled code that has already been removed. So the fix just adds a check to verify if the JavaThread entered into interpreted only mode in that transition back to Java and to return the c2i entry point instead.
> 
> I tested the patch in mach5 tiers 1-6. I also verified it with the reproducer I attached to the bug. I didn't include it as an additional test but I can do that if desired.
> 
> Thanks,
> Patricio

> Do we ever call these resolve stubs on method handle intrinsics (if we could force the compilers not to inline them)? I'm wondering what happens when we ask for the c2i adapter for intrinsics like _linkToNative, and also if these intrinsics do the right thing with is_interp_only_mode(). The fact that these intrinsics only have a compiled version came up before (JDK-8296336 and #10933). These intrinsics don't push a frame, but when they call the target do they check for is_interp_only_mode() and call the interpreted version instead of the compiled version?
>
I don't know how method handle intrinsics work, so not sure if they use this stubs. If they do and there is no c2i adapter then this approach won't work, even if we check and ignore that case returning the normal compiled entry. Because we will remain in compile code and the issue is the same. Interpreted only mode is not checked while in compiled code because the code assumes that won't happen. The switch to interpreted only mode happens in a handshake, and we deoptimize all frames, so we should switch to interpreted mode when going back to Java after the handshake either from vm or native. But this case is special because we are returning to Java but not back to the deopted caller, we are returning to the runtime stub and then jumping to the callee entry.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14108#issuecomment-1561857366