8198909: [Graal] compiler/codecache/stress/UnexpectedDeoptimizationTest.java crashed with SIGSEGV
authornever
Sun, 24 Jun 2018 21:46:11 -0700
changeset 50746 85789fb05154
parent 50745 a390cbb82d47
child 50747 66aa2e3ffcbc
8198909: [Graal] compiler/codecache/stress/UnexpectedDeoptimizationTest.java crashed with SIGSEGV Reviewed-by: eosterlund, iveresov
src/hotspot/share/classfile/classLoaderData.hpp
src/hotspot/share/jvmci/jvmciCodeInstaller.cpp
src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
src/hotspot/share/jvmci/jvmciCompilerToVM.hpp
src/hotspot/share/oops/instanceKlass.cpp
src/hotspot/share/oops/instanceKlass.hpp
src/hotspot/share/oops/klass.cpp
src/hotspot/share/oops/klass.hpp
--- a/src/hotspot/share/classfile/classLoaderData.hpp	Mon Jun 25 02:07:42 2018 +0200
+++ b/src/hotspot/share/classfile/classLoaderData.hpp	Sun Jun 24 21:46:11 2018 -0700
@@ -210,7 +210,7 @@
   friend class ClassLoaderDataGraphKlassIteratorAtomic;
   friend class ClassLoaderDataGraphKlassIteratorStatic;
   friend class ClassLoaderDataGraphMetaspaceIterator;
-  friend class InstanceKlass;
+  friend class Klass;
   friend class MetaDataFactory;
   friend class Method;
 
--- a/src/hotspot/share/jvmci/jvmciCodeInstaller.cpp	Mon Jun 25 02:07:42 2018 +0200
+++ b/src/hotspot/share/jvmci/jvmciCodeInstaller.cpp	Sun Jun 24 21:46:11 2018 -0700
@@ -234,7 +234,7 @@
 
   vmassert(index + 1 == newCount, "must be last");
 
-  Klass* klass = NULL;
+  JVMCIKlassHandle klass(THREAD);
   oop result = NULL;
   if (h->is_klass()) {
     klass = (Klass*) h;
--- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp	Mon Jun 25 02:07:42 2018 +0200
+++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp	Sun Jun 24 21:46:11 2018 -0700
@@ -44,6 +44,21 @@
 #include "runtime/timerTrace.hpp"
 #include "runtime/vframe_hp.hpp"
 
+JVMCIKlassHandle::JVMCIKlassHandle(Thread* thread, Klass* klass) {
+  _thread = thread;
+  _klass = klass;
+  if (klass != NULL) {
+    _holder = Handle(_thread, klass->holder_phantom());
+  }
+}
+
+JVMCIKlassHandle& JVMCIKlassHandle::operator=(Klass* klass) {
+  _klass = klass;
+  if (klass != NULL) {
+    _holder = Handle(_thread, klass->holder_phantom());
+  }
+  return *this;
+}
 
 void JNIHandleMark::push_jni_handle_block() {
   JavaThread* thread = JavaThread::current();
@@ -91,8 +106,8 @@
   return NULL;
 }
 
-oop CompilerToVM::get_jvmci_type(Klass* klass, TRAPS) {
-  if (klass != NULL) {
+oop CompilerToVM::get_jvmci_type(JVMCIKlassHandle& klass, TRAPS) {
+  if (!klass.is_null()) {
     JavaValue result(T_OBJECT);
     JavaCallArguments args;
     args.push_oop(Handle(THREAD, klass->java_mirror()));
@@ -311,7 +326,7 @@
 }
 
 C2V_VMENTRY(jobject, getResolvedJavaType, (JNIEnv *, jobject, jobject base, jlong offset, jboolean compressed))
-  Klass* klass = NULL;
+  JVMCIKlassHandle klass(THREAD);
   oop base_object = JNIHandles::resolve(base);
   jlong base_address = 0;
   if (base_object != NULL && offset == oopDesc::klass_offset_in_bytes()) {
@@ -365,7 +380,8 @@
         err_msg("Expected interface type, got %s", klass->external_name()));
   }
   InstanceKlass* iklass = InstanceKlass::cast(klass);
-  oop implementor = CompilerToVM::get_jvmci_type(iklass->implementor(), CHECK_NULL);
+  JVMCIKlassHandle handle(THREAD, iklass->implementor());
+  oop implementor = CompilerToVM::get_jvmci_type(handle, CHECK_NULL);
   return JNIHandles::make_local(THREAD, implementor);
 C2V_END
 
@@ -400,7 +416,7 @@
     THROW_MSG_0(vmSymbols::java_lang_InternalError(), err_msg("Primitive type %s should be handled in Java code", class_name->as_C_string()));
   }
 
-  Klass* resolved_klass = NULL;
+  JVMCIKlassHandle resolved_klass(THREAD);
   if (JNIHandles::resolve(accessing_class) == NULL) {
     THROW_0(vmSymbols::java_lang_NullPointerException());
   }
@@ -433,12 +449,11 @@
                                                              class_loader,
                                                              protection_domain,
                                                              CHECK_0);
-        if (resolved_klass != NULL) {
+        if (!resolved_klass.is_null()) {
           resolved_klass = resolved_klass->array_klass(fd.dimension(), CHECK_0);
         }
       } else {
-        resolved_klass = Universe::typeArrayKlassObj(t);
-        resolved_klass = TypeArrayKlass::cast(resolved_klass)->array_klass(fd.dimension(), CHECK_0);
+        resolved_klass = TypeArrayKlass::cast(Universe::typeArrayKlassObj(t))->array_klass(fd.dimension(), CHECK_0);
       }
     }
   }
@@ -482,25 +497,26 @@
 
 C2V_VMENTRY(jobject, resolveTypeInPool, (JNIEnv*, jobject, jobject jvmci_constant_pool, jint index))
   constantPoolHandle cp = CompilerToVM::asConstantPool(jvmci_constant_pool);
-  Klass* resolved_klass = cp->klass_at(index, CHECK_NULL);
+  Klass* klass = cp->klass_at(index, CHECK_NULL);
+  JVMCIKlassHandle resolved_klass(THREAD, klass);
   if (resolved_klass->is_instance_klass()) {
-    InstanceKlass::cast(resolved_klass)->link_class_or_fail(THREAD);
+    InstanceKlass::cast(resolved_klass())->link_class_or_fail(THREAD);
   }
-  oop klass = CompilerToVM::get_jvmci_type(resolved_klass, CHECK_NULL);
-  return JNIHandles::make_local(THREAD, klass);
+  oop jvmci_type = CompilerToVM::get_jvmci_type(resolved_klass, CHECK_NULL);
+  return JNIHandles::make_local(THREAD, jvmci_type);
 C2V_END
 
 C2V_VMENTRY(jobject, lookupKlassInPool, (JNIEnv*, jobject, jobject jvmci_constant_pool, jint index, jbyte opcode))
   constantPoolHandle cp = CompilerToVM::asConstantPool(jvmci_constant_pool);
   Klass* loading_klass = cp->pool_holder();
   bool is_accessible = false;
-  Klass* klass = JVMCIEnv::get_klass_by_index(cp, index, is_accessible, loading_klass);
+  JVMCIKlassHandle klass(THREAD, JVMCIEnv::get_klass_by_index(cp, index, is_accessible, loading_klass));
   Symbol* symbol = NULL;
   if (klass == NULL) {
     symbol = cp->klass_name_at(index);
   }
   oop result_oop;
-  if (klass != NULL) {
+  if (!klass.is_null()) {
     result_oop = CompilerToVM::get_jvmci_type(klass, CHECK_NULL);
   } else {
     Handle result = java_lang_String::create_from_symbol(symbol, CHECK_NULL);
@@ -543,7 +559,8 @@
   info->int_at_put(0, fd.access_flags().as_int());
   info->int_at_put(1, fd.offset());
   info->int_at_put(2, fd.index());
-  oop field_holder = CompilerToVM::get_jvmci_type(fd.field_holder(), CHECK_NULL);
+  JVMCIKlassHandle handle(THREAD, fd.field_holder());
+  oop field_holder = CompilerToVM::get_jvmci_type(handle, CHECK_NULL);
   return JNIHandles::make_local(THREAD, field_holder);
 C2V_END
 
@@ -1413,7 +1430,8 @@
 C2V_VMENTRY(jobject, getHostClass, (JNIEnv*, jobject, jobject jvmci_type))
   InstanceKlass* k = InstanceKlass::cast(CompilerToVM::asKlass(jvmci_type));
   InstanceKlass* host = k->host_klass();
-  oop result = CompilerToVM::get_jvmci_type(host, CHECK_NULL);
+  JVMCIKlassHandle handle(THREAD, host);
+  oop result = CompilerToVM::get_jvmci_type(handle, CHECK_NULL);
   return JNIHandles::make_local(THREAD, result);
 C2V_END
 
--- a/src/hotspot/share/jvmci/jvmciCompilerToVM.hpp	Mon Jun 25 02:07:42 2018 +0200
+++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.hpp	Sun Jun 24 21:46:11 2018 -0700
@@ -28,6 +28,37 @@
 #include "runtime/javaCalls.hpp"
 #include "jvmci/jvmciJavaClasses.hpp"
 
+// Helper class to ensure that references to Klass* are kept alive for G1
+class JVMCIKlassHandle : public StackObj {
+ private:
+  Klass*     _klass;
+  Handle     _holder;
+  Thread*    _thread;
+
+  Klass*        klass() const                     { return _klass; }
+  Klass*        non_null_klass() const            { assert(_klass != NULL, "resolving NULL _klass"); return _klass; }
+
+ public:
+  /* Constructors */
+  JVMCIKlassHandle (Thread* thread) : _klass(NULL), _thread(thread) {}
+  JVMCIKlassHandle (Thread* thread, Klass* klass);
+
+  JVMCIKlassHandle (const JVMCIKlassHandle &h): _klass(h._klass), _holder(h._holder), _thread(h._thread) {}
+  JVMCIKlassHandle& operator=(const JVMCIKlassHandle &s);
+  JVMCIKlassHandle& operator=(Klass* klass);
+
+  /* Operators for ease of use */
+  Klass*        operator () () const            { return klass(); }
+  Klass*        operator -> () const            { return non_null_klass(); }
+
+  bool    operator == (Klass* o) const          { return klass() == o; }
+  bool    operator == (const JVMCIKlassHandle& h) const  { return klass() == h.klass(); }
+
+  /* Null checks */
+  bool    is_null() const                      { return _klass == NULL; }
+  bool    not_null() const                     { return _klass != NULL; }
+};
+
 class CompilerToVM {
  public:
   class Data {
@@ -161,7 +192,7 @@
 
   static oop get_jvmci_method(const methodHandle& method, TRAPS);
 
-  static oop get_jvmci_type(Klass* klass, TRAPS);
+  static oop get_jvmci_type(JVMCIKlassHandle& klass, TRAPS);
 };
 
 class JavaArgumentUnboxer : public SignatureIterator {
--- a/src/hotspot/share/oops/instanceKlass.cpp	Mon Jun 25 02:07:42 2018 +0200
+++ b/src/hotspot/share/oops/instanceKlass.cpp	Sun Jun 24 21:46:11 2018 -0700
@@ -3625,10 +3625,6 @@
   }
 }
 
-oop InstanceKlass::holder_phantom() const {
-  return class_loader_data()->holder_phantom();
-}
-
 #ifdef ASSERT
 void InstanceKlass::set_init_state(ClassState state) {
   bool good_state = is_shared() ? (_init_state <= state)
--- a/src/hotspot/share/oops/instanceKlass.hpp	Mon Jun 25 02:07:42 2018 +0200
+++ b/src/hotspot/share/oops/instanceKlass.hpp	Sun Jun 24 21:46:11 2018 -0700
@@ -687,11 +687,6 @@
     return is_anonymous() ? java_mirror() : class_loader();
   }
 
-  // Load the klass's holder as a phantom. This is useful when a weak Klass
-  // pointer has been "peeked" and then must be kept alive before it may
-  // be used safely.
-  oop holder_phantom() const;
-
   bool is_contended() const                {
     return (_misc_flags & _misc_is_contended) != 0;
   }
--- a/src/hotspot/share/oops/klass.cpp	Mon Jun 25 02:07:42 2018 +0200
+++ b/src/hotspot/share/oops/klass.cpp	Sun Jun 24 21:46:11 2018 -0700
@@ -388,6 +388,10 @@
   debug_only(verify();)
 }
 
+oop Klass::holder_phantom() const {
+  return class_loader_data()->holder_phantom();
+}
+
 void Klass::clean_weak_klass_links(bool unloading_occurred, bool clean_alive_klasses) {
   if (!ClassUnloading || !unloading_occurred) {
     return;
--- a/src/hotspot/share/oops/klass.hpp	Mon Jun 25 02:07:42 2018 +0200
+++ b/src/hotspot/share/oops/klass.hpp	Sun Jun 24 21:46:11 2018 -0700
@@ -640,6 +640,11 @@
   // Klass is considered alive.  Has already been marked as unloading.
   bool is_loader_alive() const { return !class_loader_data()->is_unloading(); }
 
+  // Load the klass's holder as a phantom. This is useful when a weak Klass
+  // pointer has been "peeked" and then must be kept alive before it may
+  // be used safely.
+  oop holder_phantom() const;
+
   static void clean_weak_klass_links(bool unloading_occurred, bool clean_alive_klasses = true);
   static void clean_subklass_tree() {
     clean_weak_klass_links(/*unloading_occurred*/ true , /* clean_alive_klasses */ false);