changeset 7268:e0048254cc82

8058093: Test nsk/stress/jck60/jck60014: assert in src/share/vm/oops/constantPool.cpp: should not be resolved otherwise 8044209: nsk/split_verifier/security/coglio06 fails with exit code 97 - missing 'prohibited package name' Summary: Fix resolution error saving. Reviewed-by: lfoltan, sspitsyn, hseigel, ctornqvi
author coleenp
date Tue, 28 Oct 2014 17:02:08 -0400
parents 64c09126ad3c
children 253150035d8f 6fc3b8261368
files src/share/vm/oops/constantPool.cpp src/share/vm/oops/constantPool.hpp src/share/vm/runtime/deoptimization.cpp
diffstat 3 files changed, 41 insertions(+), 21 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/oops/constantPool.cpp	Mon Oct 27 15:09:23 2014 -0700
+++ b/src/share/vm/oops/constantPool.cpp	Tue Oct 28 17:02:08 2014 -0400
@@ -206,7 +206,8 @@
   }
 }
 
-Klass* ConstantPool::klass_at_impl(constantPoolHandle this_cp, int which, TRAPS) {
+Klass* ConstantPool::klass_at_impl(constantPoolHandle this_cp, int which,
+                                   bool save_resolution_error, TRAPS) {
   assert(THREAD->is_Java_thread(), "must be a Java thread");
 
   // A resolved constantPool entry will contain a Klass*, otherwise a Symbol*.
@@ -249,7 +250,18 @@
   // Failed to resolve class. We must record the errors so that subsequent attempts
   // to resolve this constant pool entry fail with the same error (JVMS 5.4.3).
   if (HAS_PENDING_EXCEPTION) {
-    save_and_throw_exception(this_cp, which, constantTag(JVM_CONSTANT_UnresolvedClass), CHECK_0);
+    if (save_resolution_error) {
+      save_and_throw_exception(this_cp, which, constantTag(JVM_CONSTANT_UnresolvedClass), CHECK_NULL);
+      // If CHECK_NULL above doesn't return the exception, that means that
+      // some other thread has beaten us and has resolved the class.
+      // To preserve old behavior, we return the resolved class.
+      entry = this_cp->resolved_klass_at(which);
+      assert(entry.is_resolved(), "must be resolved if exception was cleared");
+      assert(entry.get_klass()->is_klass(), "must be resolved to a klass");
+      return entry.get_klass();
+    } else {
+      return NULL;  // return the pending exception
+    }
   }
 
   // Make this class loader depend upon the class loader owning the class reference
@@ -260,10 +272,10 @@
     // skip resolving the constant pool so that this code gets
     // called the next time some bytecodes refer to this class.
     trace_class_resolution(this_cp, k);
-      return k();
-    } else {
-        this_cp->klass_at_put(which, k());
-      }
+    return k();
+  } else {
+    this_cp->klass_at_put(which, k());
+  }
 
   entry = this_cp->resolved_klass_at(which);
   assert(entry.is_resolved() && entry.get_klass()->is_klass(), "must be resolved at this point");
@@ -573,24 +585,25 @@
     Symbol* message = exception_message(this_cp, which, tag, PENDING_EXCEPTION);
     SystemDictionary::add_resolution_error(this_cp, which, error, message);
     // CAS in the tag.  If a thread beat us to registering this error that's fine.
-    // If another thread resolved the reference, this is an error.  The resolution
-    // must deterministically get an error.   So why do we save this?
-    // We save this because jvmti can add classes to the bootclass path after this
-    // error, so it needs to get the same error if the error is first.
+    // If another thread resolved the reference, this is a race condition. This
+    // thread may have had a security manager or something temporary.
+    // This doesn't deterministically get an error.   So why do we save this?
+    // We save this because jvmti can add classes to the bootclass path after
+    // this error, so it needs to get the same error if the error is first.
     jbyte old_tag = Atomic::cmpxchg((jbyte)error_tag,
                             (jbyte*)this_cp->tag_addr_at(which), (jbyte)tag.value());
-    assert(old_tag == error_tag || old_tag == tag.value(), "should not be resolved otherwise");
+    if (old_tag != error_tag && old_tag != tag.value()) {
+      // MethodHandles and MethodType doesn't change to resolved version.
+      assert(this_cp->tag_at(which).is_klass(), "Wrong tag value");
+      // Forget the exception and use the resolved class.
+      CLEAR_PENDING_EXCEPTION;
+    }
   } else {
     // some other thread put this in error state
     throw_resolution_error(this_cp, which, CHECK);
   }
-
-  // This exits with some pending exception
-  assert(HAS_PENDING_EXCEPTION, "should not be cleared");
 }
 
-
-
 // Called to resolve constants in the constant pool and return an oop.
 // Some constant pool entries cache their resolved oop. This is also
 // called to create oops from constants to use in arguments for invokedynamic
@@ -627,7 +640,7 @@
   case JVM_CONSTANT_Class:
     {
       assert(cache_index == _no_index_sentinel, "should not have been set");
-      Klass* resolved = klass_at_impl(this_cp, index, CHECK_NULL);
+      Klass* resolved = klass_at_impl(this_cp, index, true, CHECK_NULL);
       // ldc wants the java mirror.
       result_oop = resolved->java_mirror();
       break;
@@ -660,7 +673,7 @@
                       ref_kind, index, this_cp->method_handle_index_at(index),
                       callee_index, name->as_C_string(), signature->as_C_string());
       KlassHandle callee;
-      { Klass* k = klass_at_impl(this_cp, callee_index, CHECK_NULL);
+      { Klass* k = klass_at_impl(this_cp, callee_index, true, CHECK_NULL);
         callee = KlassHandle(THREAD, k);
       }
       KlassHandle klass(THREAD, this_cp->pool_holder());
--- a/src/share/vm/oops/constantPool.hpp	Mon Oct 27 15:09:23 2014 -0700
+++ b/src/share/vm/oops/constantPool.hpp	Tue Oct 28 17:02:08 2014 -0400
@@ -336,7 +336,13 @@
 
   Klass* klass_at(int which, TRAPS) {
     constantPoolHandle h_this(THREAD, this);
-    return klass_at_impl(h_this, which, CHECK_NULL);
+    return klass_at_impl(h_this, which, true, CHECK_NULL);
+  }
+
+  // Version of klass_at that doesn't save the resolution error, called during deopt
+  Klass* klass_at_ignore_error(int which, TRAPS) {
+    constantPoolHandle h_this(THREAD, this);
+    return klass_at_impl(h_this, which, false, CHECK_NULL);
   }
 
   Symbol* klass_name_at(int which);  // Returns the name, w/o resolving.
@@ -793,7 +799,8 @@
 
   // Implementation of methods that needs an exposed 'this' pointer, in order to
   // handle GC while executing the method
-  static Klass* klass_at_impl(constantPoolHandle this_cp, int which, TRAPS);
+  static Klass* klass_at_impl(constantPoolHandle this_cp, int which,
+                              bool save_resolution_error, TRAPS);
   static oop string_at_impl(constantPoolHandle this_cp, int which, int obj_index, TRAPS);
 
   static void trace_class_resolution(constantPoolHandle this_cp, KlassHandle k);
--- a/src/share/vm/runtime/deoptimization.cpp	Mon Oct 27 15:09:23 2014 -0700
+++ b/src/share/vm/runtime/deoptimization.cpp	Tue Oct 28 17:02:08 2014 -0400
@@ -1173,7 +1173,7 @@
 void Deoptimization::load_class_by_index(constantPoolHandle constant_pool, int index, TRAPS) {
   // in case of an unresolved klass entry, load the class.
   if (constant_pool->tag_at(index).is_unresolved_klass()) {
-    Klass* tk = constant_pool->klass_at(index, CHECK);
+    Klass* tk = constant_pool->klass_at_ignore_error(index, CHECK);
     return;
   }