From fbaba5d685f3e3cdb655c6ff578e01d3586afec2 Mon Sep 17 00:00:00 2001 From: Doug Simon Date: Thu, 25 Jun 2020 14:56:06 +0200 Subject: [PATCH] HotSpotNmethod.executeVarargs can try execute a zombie nmethod --- src/hotspot/share/jvmci/jvmciCompilerToVM.cpp | 4 +- src/hotspot/share/runtime/javaCalls.cpp | 46 ++++++++----------- src/hotspot/share/runtime/javaCalls.hpp | 11 +++-- .../src/jdk/vm/ci/hotspot/HotSpotNmethod.java | 12 +++++ 4 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp index 61151b21906..9abfa32d259 100644 --- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp +++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp @@ -1027,7 +1027,7 @@ C2V_VMENTRY_NULL(jobject, executeHotSpotNmethod, (JNIEnv* env, jobject, jobject JVMCIObject nmethod_mirror = JVMCIENV->wrap(hs_nmethod); nmethodLocker locker; nmethod* nm = JVMCIENV->get_nmethod(nmethod_mirror, locker); - if (nm == NULL) { + if (nm == NULL || !nm->is_in_use()) { JVMCI_THROW_NULL(InvalidInstalledCodeException); } methodHandle mh = nm->method(); @@ -1036,7 +1036,7 @@ C2V_VMENTRY_NULL(jobject, executeHotSpotNmethod, (JNIEnv* env, jobject, jobject JavaArgumentUnboxer jap(signature, &jca, (arrayOop) JNIHandles::resolve(args), mh->is_static()); JavaValue result(jap.get_ret_type()); - jca.set_alternative_target(nm); + jca.set_alternative_target(Handle(THREAD, JNIHandles::resolve(nmethod_mirror.as_jobject()))); JavaCalls::call(&result, mh, &jca, CHECK_NULL); if (jap.get_ret_type() == T_VOID) { diff --git a/src/hotspot/share/runtime/javaCalls.cpp b/src/hotspot/share/runtime/javaCalls.cpp index 4db7bf26dce..55d525a13dc 100644 --- a/src/hotspot/share/runtime/javaCalls.cpp +++ b/src/hotspot/share/runtime/javaCalls.cpp @@ -29,6 +29,9 @@ #include "compiler/compileBroker.hpp" #include "interpreter/interpreter.hpp" #include "interpreter/linkResolver.hpp" +#if INCLUDE_JVMCI +#include "jvmci/jvmciJavaClasses.hpp" +#endif #include "memory/universe.hpp" #include "oops/method.inline.hpp" #include "oops/oop.inline.hpp" @@ -348,27 +351,15 @@ void JavaCalls::call_helper(JavaValue* result, const methodHandle& method, JavaC CHECK_UNHANDLED_OOPS_ONLY(thread->clear_unhandled_oops();) -#if INCLUDE_JVMCI - // Gets the nmethod (if any) that should be called instead of normal target - nmethod* alternative_target = args->alternative_target(); - if (alternative_target == NULL) { -#endif -// Verify the arguments - - if (CheckJNICalls) { + // Verify the arguments + if (JVMCI_ONLY(args->alternative_target().is_null() &&) (DEBUG_ONLY(true ||) CheckJNICalls)) { args->verify(method, result->get_type()); } - else debug_only(args->verify(method, result->get_type())); -#if INCLUDE_JVMCI - } -#else - // Ignore call if method is empty - if (method->is_empty_method()) { + if (JVMCI_ONLY(args->alternative_target().is_null() &&) method->is_empty_method()) { assert(result->get_type() == T_VOID, "an empty method must return a void value"); return; } -#endif #ifdef ASSERT { InstanceKlass* holder = method->method_holder(); @@ -420,21 +411,24 @@ void JavaCalls::call_helper(JavaValue* result, const methodHandle& method, JavaC os::map_stack_shadow_pages(sp); } -#if INCLUDE_JVMCI - if (alternative_target != NULL) { - if (alternative_target->is_alive()) { - thread->set_jvmci_alternate_call_target(alternative_target->verified_entry_point()); - entry_point = method->adapter()->get_i2c_entry(); - } else { - THROW(vmSymbols::jdk_vm_ci_code_InvalidInstalledCodeException()); - } - } -#endif - // do call { JavaCallWrapper link(method, receiver, result, CHECK); { HandleMark hm(thread); // HandleMark used by HandleMarkCleaner +#if INCLUDE_JVMCI + // Gets the alternative target (if any) that should be called + Handle alternative_target = args->alternative_target(); + if (!alternative_target.is_null()) { + // Must extract verified entry point from HotSpotNmethod after VM to Java + // transition in JavaCallWrapper constructor so that it is safe with + // respect to nmethod sweeping. + address verified_entry_point = (address) HotSpotJVMCI::InstalledCode::entryPoint(NULL, alternative_target()); + if (verified_entry_point != NULL) { + thread->set_jvmci_alternate_call_target(verified_entry_point); + entry_point = method->adapter()->get_i2c_entry(); + } + } +#endif StubRoutines::call_stub()( (address)&link, // (intptr_t*)&(result->_value), // see NOTE above (compiler problem) diff --git a/src/hotspot/share/runtime/javaCalls.hpp b/src/hotspot/share/runtime/javaCalls.hpp index 07f1e133741..bddf86eebf3 100644 --- a/src/hotspot/share/runtime/javaCalls.hpp +++ b/src/hotspot/share/runtime/javaCalls.hpp @@ -88,7 +88,10 @@ class JavaCallArguments : public StackObj { int _size; int _max_size; bool _start_at_zero; // Support late setting of receiver - JVMCI_ONLY(nmethod* _alternative_target;) // Nmethod that should be called instead of normal target +#if INCLUDE_JVMCI + Handle _alternative_target; // HotSpotNmethod wrapping an nmethod whose verified entry point + // should be called instead of the normal target +#endif void initialize() { // Starts at first element to support set_receiver. @@ -98,7 +101,6 @@ class JavaCallArguments : public StackObj { _max_size = _default_size; _size = 0; _start_at_zero = false; - JVMCI_ONLY(_alternative_target = NULL;) } // Helper for push_oop and the like. The value argument is a @@ -138,18 +140,17 @@ class JavaCallArguments : public StackObj { _max_size = max_size; _size = 0; _start_at_zero = false; - JVMCI_ONLY(_alternative_target = NULL;) } else { initialize(); } } #if INCLUDE_JVMCI - void set_alternative_target(nmethod* target) { + void set_alternative_target(Handle target) { _alternative_target = target; } - nmethod* alternative_target() { + Handle alternative_target() { return _alternative_target; } #endif diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotNmethod.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotNmethod.java index ae8dcbc259a..99fd3016f98 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotNmethod.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotNmethod.java @@ -146,6 +146,18 @@ private boolean checkArgs(Object... args) { return true; } + /** + * {@inheritDoc} + * + * It's possible for the HotSpot runtime to sweep nmethods at any point in time. As a result, + * there is no guarantee that calling this method will execute the wrapped nmethod. Instead, it + * may end up executing the bytecode of the associated {@link #getMethod() Java method}. Only if + * {@link #isValid()} is {@code true} after returning can the caller be sure that the nmethod + * was executed. If {@link #isValid()} is {@code false}, then the only way to determine if the + * nmethod was executed is to test for some side-effect specific to the nmethod (e.g., update to + * a field) that is not performed by the bytecode of the associated {@link #getMethod() Java + * method}. + */ @Override public Object executeVarargs(Object... args) throws InvalidInstalledCodeException { if (IS_IN_NATIVE_IMAGE) {