RFR: 8330105: SharedRuntime::resolve* should respect interpreter-only mode

[Patricio Chilano Mateo]

On Mon, 15 Apr 2024 14:24:29 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> JavaThread::set_interp_only_mode may be called while a thread is blocked waiting for a JIT compilation to complete. When interpreter-only mode is set, we should dispatch to interpreter instead of the returned compiled code.
>
> This is the same initial fix I proposed for JDK-8302351 but which I later changed when stumbling upon some exception cases where we cannot just return the c2i adapter entry: method handle intrinsics and enterSpecial/doYield methods.
> For method handle intrinsics, _linkToNative doesn't have an interpreter version so the c2i will lead to a i2c and we will crash because we cannot cascade those. For the other method handle intrinsics, although there is an interpreter version, I found another issue where generate_method_handle_interpreter_entry() can throw an exception before we create the interpreter frame, which will lead to crashes when walking the stack.
> Regarding enterSpecial/doYield, those also lack an interpreter version as _linkToNative(although enterSpecial has a hack here), but they are not really an issue today because we cannot switch to interpreter only mode while resolving those methods.

> @pchilano how about we return c2i only if callee is not a method handle intrinsic?
> 
> ```
> diff --git a/src/hotspot/share/runtime/sharedRuntime.cpp b/src/hotspot/share/runtime/sharedRuntime.cpp
> index 2b06859c96d..74d361a2b57 100644
> --- a/src/hotspot/share/runtime/sharedRuntime.cpp
> +++ b/src/hotspot/share/runtime/sharedRuntime.cpp
> @@ -1489,7 +1489,7 @@ JRT_END
>  // return verified_code_entry if interp_only_mode is not set for the current thread;
>  // otherwise return c2i entry.
>  address SharedRuntime::get_resolved_entry(JavaThread* current, methodHandle callee_method) {
> -  if (current->is_interp_only_mode()) {
> +  if (current->is_interp_only_mode()  && !callee_method->is_method_handle_intrinsic()) {
>      // In interp_only_mode we need to go to the interpreted entry
>      // The c2i won't patch in this mode -- see fixup_callers_callsite
>      return callee_method->get_c2i_entry();
> ```
> 
Sounds good. We can use is_special_native_intrinsic() instead just to avoid possible issues when testing Continuations alone. We can leave 8218403 open to fix these remaining cases.

> Btw how did you stress test this? [#14108 (comment)](https://github.com/openjdk/jdk/pull/14108#issuecomment-1574091628)
>
To see the issue about the cascading c2i -> i2c you can just always return the c2i entry. For the other issue I mentioned you can set `current->set_interp_only_mode(true);` upon entry in get_resolved_entry() to always return the c2i entry (exclude _linkToNative) and then remove the JvmtiExport::can_post_interpreter_events() check in jump_from_method_handle() (otherwise we might end up in the same cascading issue).

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

PR Comment: https://git.openjdk.org/jdk/pull/18741#issuecomment-2057449313