8317368: [JVMCI] SIGSEGV in JVMCIEnv::initialize_installed_code on libgraal

Reviewed-by: dnsimon, kvn, eosterlund

src/hotspot/share/jvmci/jvmciCodeInstaller.cpp

@@ -704,6 +704,7 @@ JVMCI::CodeInstallResult CodeInstaller::install(JVMCICompiler* compiler,
     JVMCIObject compiled_code,
     objArrayHandle object_pool,
     CodeBlob*& cb,
+    JVMCINMethodHandle& nmethod_handle,
     JVMCIObject installed_code,
     FailedSpeculation** failed_speculations,
     char* speculations,
@@ -805,6 +806,8 @@ JVMCI::CodeInstallResult CodeInstaller::install(JVMCICompiler* compiler,
                                         speculations_len,
                                         _nmethod_entry_patch_offset);
     if (result == JVMCI::ok) {
+      guarantee(nm != nullptr, "successful compile must produce an nmethod");
+      nmethod_handle.set_nmethod(nm);
       cb = nm;
       if (compile_state == nullptr) {
         // This compile didn't come through the CompileBroker so perform the printing here
@@ -813,14 +816,13 @@ JVMCI::CodeInstallResult CodeInstaller::install(JVMCICompiler* compiler,
         DirectivesStack::release(directive);
       }
 
-      if (nm != nullptr) {
-        if (_nmethod_entry_patch_offset != -1) {
-          err_msg msg("");
-          BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod();
+      if (_nmethod_entry_patch_offset != -1) {
+        BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod();
 
-          if (!bs_nm->verify_barrier(nm, msg)) {
-            JVMCI_THROW_MSG_(IllegalArgumentException, err_msg("nmethod entry barrier is malformed: %s", msg.buffer()), JVMCI::ok);
-          }
+        // an empty error buffer for use by the verify_barrier code
+        err_msg msg("");
+        if (!bs_nm->verify_barrier(nm, msg)) {
+          JVMCI_THROW_MSG_(IllegalArgumentException, err_msg("nmethod entry barrier is malformed: %s", msg.buffer()), JVMCI::ok);
         }
       }
     }

src/hotspot/share/jvmci/jvmciCodeInstaller.hpp

@@ -332,6 +332,7 @@ public:
                                    JVMCIObject compiled_code,
                                    objArrayHandle object_pool,
                                    CodeBlob*& cb,
+                                   JVMCINMethodHandle& nmethod_handle,
                                    JVMCIObject installed_code,
                                    FailedSpeculation** failed_speculations,
                                    char* speculations,

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -61,6 +61,7 @@
 #include "runtime/globals_extension.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
 #include "runtime/jniHandles.inline.hpp"
+#include "runtime/keepStackGCProcessed.hpp"
 #include "runtime/reflection.hpp"
 #include "runtime/stackFrameStream.inline.hpp"
 #include "runtime/timerTrace.hpp"
@@ -1106,6 +1107,7 @@ C2V_VMENTRY_0(jint, installCode0, (JNIEnv *env, jobject,
   TraceTime install_time("installCode", timer);
 
   CodeInstaller installer(JVMCIENV);
+  JVMCINMethodHandle nmethod_handle(THREAD);
 
   JVMCI::CodeInstallResult result = installer.install(compiler,
       compiled_code_buffer,
@@ -1113,6 +1115,7 @@ C2V_VMENTRY_0(jint, installCode0, (JNIEnv *env, jobject,
       compiled_code_handle,
       object_pool_handle,
       cb,
+      nmethod_handle,
       installed_code_handle,
       (FailedSpeculation**)(address) failed_speculations_address,
       speculations,
@@ -1204,7 +1207,8 @@ C2V_VMENTRY_NULL(jobject, executeHotSpotNmethod, (JNIEnv* env, jobject, jobject
   HandleMark hm(THREAD);
 
   JVMCIObject nmethod_mirror = JVMCIENV->wrap(hs_nmethod);
-  nmethod* nm = JVMCIENV->get_nmethod(nmethod_mirror);
+  JVMCINMethodHandle nmethod_handle(THREAD);
+  nmethod* nm = JVMCIENV->get_nmethod(nmethod_mirror, nmethod_handle);
   if (nm == nullptr || !nm->is_in_use()) {
     JVMCI_THROW_NULL(InvalidInstalledCodeException);
   }
@@ -1478,6 +1482,7 @@ C2V_VMENTRY_NULL(jobject, iterateFrames, (JNIEnv* env, jobject compilerToVM, job
     return nullptr;
   }
   Handle visitor(THREAD, JNIHandles::resolve_non_null(visitor_handle));
+  KeepStackGCProcessedMark keep_stack(THREAD);
 
   requireInHotSpot("iterateFrames", JVMCI_CHECK_NULL);
 
@@ -2762,7 +2767,8 @@ C2V_VMENTRY_0(jlong, translate, (JNIEnv* env, jobject, jobject obj_handle, jbool
     result = PEER_JVMCIENV->get_object_constant(constant());
   } else if (thisEnv->isa_HotSpotNmethod(obj)) {
     if (PEER_JVMCIENV->is_hotspot()) {
-      nmethod* nm = JVMCIENV->get_nmethod(obj);
+      JVMCINMethodHandle nmethod_handle(THREAD);
+      nmethod* nm = JVMCIENV->get_nmethod(obj, nmethod_handle);
       if (nm != nullptr) {
         JVMCINMethodData* data = nm->jvmci_nmethod_data();
         if (data != nullptr) {
@@ -2785,7 +2791,8 @@ C2V_VMENTRY_0(jlong, translate, (JNIEnv* env, jobject, jobject obj_handle, jbool
       const char* cstring = name_string.is_null() ? nullptr : thisEnv->as_utf8_string(name_string);
       // Create a new HotSpotNmethod instance in the peer runtime
       result = PEER_JVMCIENV->new_HotSpotNmethod(mh, cstring, isDefault, compileIdSnapshot, JVMCI_CHECK_0);
-      nmethod* nm = JVMCIENV->get_nmethod(obj);
+      JVMCINMethodHandle nmethod_handle(THREAD);
+      nmethod* nm = JVMCIENV->get_nmethod(obj, nmethod_handle);
       if (result.is_null()) {
         // exception occurred (e.g. OOME) creating a new HotSpotNmethod
       } else if (nm == nullptr) {
@@ -2837,7 +2844,8 @@ C2V_END
 C2V_VMENTRY(void, updateHotSpotNmethod, (JNIEnv* env, jobject, jobject code_handle))
   JVMCIObject code = JVMCIENV->wrap(code_handle);
   // Execute this operation for the side effect of updating the InstalledCode state
-  JVMCIENV->get_nmethod(code);
+  JVMCINMethodHandle nmethod_handle(THREAD);
+  JVMCIENV->get_nmethod(code, nmethod_handle);
 C2V_END
 
 C2V_VMENTRY_NULL(jbyteArray, getCode, (JNIEnv* env, jobject, jobject code_handle))

src/hotspot/share/jvmci/jvmciEnv.cpp

@@ -29,6 +29,8 @@
 #include "code/codeCache.hpp"
 #include "compiler/compilerOracle.hpp"
 #include "compiler/compileTask.hpp"
+#include "gc/shared/barrierSet.hpp"
+#include "gc/shared/barrierSetNMethod.hpp"
 #include "jvm_io.h"
 #include "memory/oopFactory.hpp"
 #include "memory/resourceArea.hpp"
@@ -1722,12 +1724,6 @@ void JVMCIEnv::invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimize, JV
     JVMCI_THROW(NullPointerException);
   }
 
-  nmethod* nm = JVMCIENV->get_nmethod(mirror);
-  if (nm == nullptr) {
-    // Nothing to do
-    return;
-  }
-
   Thread* current = Thread::current();
   if (!mirror.is_hotspot() && !current->is_Java_thread()) {
     // Calling back into native might cause the execution to block, so only allow this when calling
@@ -1736,6 +1732,14 @@ void JVMCIEnv::invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimize, JV
                     "Cannot invalidate HotSpotNmethod object in shared library VM heap from non-JavaThread");
   }
 
+  JavaThread* thread = JavaThread::cast(current);
+  JVMCINMethodHandle nmethod_handle(thread);
+  nmethod* nm = JVMCIENV->get_nmethod(mirror, nmethod_handle);
+  if (nm == nullptr) {
+    // Nothing to do
+    return;
+  }
+
   if (!deoptimize) {
     // Prevent future executions of the nmethod but let current executions complete.
     nm->make_not_entrant();
@@ -1825,10 +1829,22 @@ CodeBlob* JVMCIEnv::get_code_blob(JVMCIObject obj) {
   return cb;
 }
 
-nmethod* JVMCIEnv::get_nmethod(JVMCIObject obj) {
+void JVMCINMethodHandle::set_nmethod(nmethod* nm) {
+  BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod();
+  if (bs_nm != nullptr) {
+    bs_nm->nmethod_entry_barrier(nm);
+  }
+  _thread->set_live_nmethod(nm);
+}
+
+nmethod* JVMCIEnv::get_nmethod(JVMCIObject obj, JVMCINMethodHandle& nmethod_handle) {
   CodeBlob* cb = get_code_blob(obj);
   if (cb != nullptr) {
-    return cb->as_nmethod_or_null();
+    nmethod* nm = cb->as_nmethod_or_null();
+    if (nm != nullptr) {
+      nmethod_handle.set_nmethod(nm);
+      return nm;
+    }
   }
   return nullptr;
 }

src/hotspot/share/jvmci/jvmciEnv.hpp

@@ -88,6 +88,21 @@ class JVMCIKlassHandle : public StackObj {
   bool    not_null() const                     { return _klass != nullptr; }
 };
 
+// A helper class to main a strong link to an nmethod that might not otherwise be referenced.  Only
+// one nmethod can be kept alive in this manner.
+class JVMCINMethodHandle : public StackObj {
+  JavaThread* _thread;
+
+ public:
+  JVMCINMethodHandle(JavaThread* thread): _thread(thread) {}
+
+  void set_nmethod(nmethod* nm);
+
+  ~JVMCINMethodHandle() {
+    _thread->clear_live_nmethod();
+  }
+};
+
 // A class that maintains the state needed for compilations requested
 // by the CompileBroker.  It is created in the broker and passed through
 // into the code installation step.
@@ -370,11 +385,11 @@ public:
 
   void fthrow_error(const char* file, int line, const char* format, ...) ATTRIBUTE_PRINTF(4, 5);
 
-  // Given an instance of HotSpotInstalledCode return the corresponding CodeBlob*.
+  // Given an instance of HotSpotInstalledCode, return the corresponding CodeBlob*.
   CodeBlob* get_code_blob(JVMCIObject code);
 
-  // Given an instance of HotSpotInstalledCode return the corresponding nmethod.
-  nmethod* get_nmethod(JVMCIObject code);
+  // Given an instance of HotSpotInstalledCode, return the corresponding nmethod.
+  nmethod* get_nmethod(JVMCIObject code, JVMCINMethodHandle& nmethod_handle);
 
   const char* klass_name(JVMCIObject object);
 

src/hotspot/share/runtime/javaThread.cpp

@@ -467,6 +467,7 @@ JavaThread::JavaThread() :
   _jvmci_reserved0(0),
   _jvmci_reserved1(0),
   _jvmci_reserved_oop0(nullptr),
+  _live_nmethod(nullptr),
 #endif // INCLUDE_JVMCI
 
   _exception_oop(oop()),
@@ -1429,6 +1430,10 @@ void JavaThread::oops_do_no_frames(OopClosure* f, NMethodClosure* cf) {
   f->do_oop((oop*) &_exception_oop);
 #if INCLUDE_JVMCI
   f->do_oop((oop*) &_jvmci_reserved_oop0);
+
+  if (_live_nmethod != nullptr && cf != nullptr) {
+    cf->do_nmethod(_live_nmethod);
+  }
 #endif
 
   if (jvmti_thread_state() != nullptr) {
@@ -1484,6 +1489,12 @@ void JavaThread::nmethods_do(NMethodClosure* cf) {
   if (jvmti_thread_state() != nullptr) {
     jvmti_thread_state()->nmethods_do(cf);
   }
+
+#if INCLUDE_JVMCI
+  if (_live_nmethod != nullptr) {
+    cf->do_nmethod(_live_nmethod);
+  }
+#endif
 }
 
 void JavaThread::metadata_do(MetadataClosure* f) {

src/hotspot/share/runtime/javaThread.hpp

@@ -379,6 +379,10 @@ class JavaThread: public Thread {
   jlong      _jvmci_reserved1;
   oop        _jvmci_reserved_oop0;
 
+  // This field is used to keep an nmethod visible to the GC so that it and its contained oops can
+  // be kept alive
+  nmethod*  _live_nmethod;
+
  public:
   static jlong* _jvmci_old_thread_counters;
   static void collect_counters(jlong* array, int length);
@@ -411,6 +415,15 @@ class JavaThread: public Thread {
     return _jvmci_reserved1;
   }
 
+  void set_live_nmethod(nmethod* nm) {
+    assert(_live_nmethod == nullptr, "only one");
+    _live_nmethod = nm;
+  }
+
+  void clear_live_nmethod() {
+    _live_nmethod = nullptr;
+  }
+
  private:
 #endif // INCLUDE_JVMCI