8160543: C1: Crash in java.lang.String.indexOf in some java.sql tests
authorneliasso
Tue, 01 Nov 2016 14:22:38 +0100
changeset 42544 58de8aaf9365
parent 42543 9e7ac7aff2d1
child 42545 af9f4bf4d6d8
8160543: C1: Crash in java.lang.String.indexOf in some java.sql tests Summary: C1 must use unverified entry point for unloaded methods. Reviewed-by: vlivanov, goetz
hotspot/src/share/vm/c1/c1_GraphBuilder.cpp
hotspot/src/share/vm/c1/c1_LIRGenerator.cpp
hotspot/src/share/vm/code/compiledIC.cpp
--- a/hotspot/src/share/vm/c1/c1_GraphBuilder.cpp	Mon Oct 31 11:36:45 2016 +0100
+++ b/hotspot/src/share/vm/c1/c1_GraphBuilder.cpp	Tue Nov 01 14:22:38 2016 +0100
@@ -1813,14 +1813,10 @@
   ciKlass*              holder = stream()->get_declared_method_holder();
   const Bytecodes::Code bc_raw = stream()->cur_bc_raw();
   assert(declared_signature != NULL, "cannot be null");
+  assert(will_link == target->is_loaded(), "");
 
   ciInstanceKlass* klass = target->holder();
-
-  // Make sure there are no evident problems with linking the instruction.
-  bool is_resolved = true;
-  if (klass->is_loaded() && !target->is_loaded()) {
-    is_resolved = false; // method not found
-  }
+  assert(!target->is_loaded() || klass->is_loaded(), "loaded target must imply loaded klass");
 
   // check if CHA possible: if so, change the code to invoke_special
   ciInstanceKlass* calling_klass = method()->holder();
@@ -1868,7 +1864,7 @@
   ciMethod* cha_monomorphic_target = NULL;
   ciMethod* exact_target = NULL;
   Value better_receiver = NULL;
-  if (UseCHA && DeoptC1 && klass->is_loaded() && target->is_loaded() &&
+  if (UseCHA && DeoptC1 && target->is_loaded() &&
       !(// %%% FIXME: Are both of these relevant?
         target->is_method_handle_intrinsic() ||
         target->is_compiled_lambda_form()) &&
@@ -1988,8 +1984,7 @@
   }
 
   // check if we could do inlining
-  if (!PatchALot && Inline && is_resolved &&
-      klass->is_loaded() && target->is_loaded() &&
+  if (!PatchALot && Inline && target->is_loaded() &&
       (klass->is_initialized() || klass->is_interface() && target->holder()->is_initialized())
       && !patch_for_appendix) {
     // callee is known => check if we have static binding
@@ -2032,7 +2027,6 @@
   CHECK_BAILOUT();
 
   // inlining not successful => standard invoke
-  bool is_loaded = target->is_loaded();
   ValueType* result_type = as_ValueType(declared_signature->return_type());
   ValueStack* state_before = copy_state_exhandling();
 
@@ -2049,7 +2043,7 @@
   // Currently only supported on Sparc.
   // The UseInlineCaches only controls dispatch to invokevirtuals for
   // loaded classes which we weren't able to statically bind.
-  if (!UseInlineCaches && is_resolved && is_loaded && code == Bytecodes::_invokevirtual
+  if (!UseInlineCaches && target->is_loaded() && code == Bytecodes::_invokevirtual
       && !target->can_be_statically_bound()) {
     // Find a vtable index if one is available
     // For arrays, callee_holder is Object. Resolving the call with
@@ -2062,18 +2056,17 @@
   }
 #endif
 
-  if (is_resolved) {
-    // invokespecial always needs a NULL check. invokevirtual where the target is
-    // final or where it's not known whether the target is final requires a NULL check.
-    // Otherwise normal invokevirtual will perform the null check during the lookup
-    // logic or the unverified entry point.  Profiling of calls requires that
-    // the null check is performed in all cases.
-    bool do_null_check = (recv != NULL) &&
-        (code == Bytecodes::_invokespecial || !is_loaded || target->is_final() || (is_profiling() && profile_calls()));
-
-    if (do_null_check) {
-      null_check(recv);
-    }
+  // invokespecial always needs a NULL check. invokevirtual where the target is
+  // final or where it's not known whether the target is final requires a NULL check.
+  // Otherwise normal invokevirtual will perform the null check during the lookup
+  // logic or the unverified entry point.  Profiling of calls requires that
+  // the null check is performed in all cases.
+
+  bool do_null_check = (recv != NULL) &&
+        (code == Bytecodes::_invokespecial || (target->is_loaded() && (target->is_final() || (is_profiling() && profile_calls()))));
+
+  if (do_null_check) {
+    null_check(recv);
 
     if (is_profiling()) {
       // Note that we'd collect profile data in this method if we wanted it.
@@ -2090,9 +2083,6 @@
         profile_call(target, recv, target_klass, collect_args_for_profiling(args, NULL, false), false);
       }
     }
-  } else {
-    // No need in null check or profiling: linkage error will be thrown at runtime
-    // during resolution.
   }
 
   Invoke* result = new Invoke(code, result_type, recv, args, vtable_index, target, state_before);
--- a/hotspot/src/share/vm/c1/c1_LIRGenerator.cpp	Mon Oct 31 11:36:45 2016 +0100
+++ b/hotspot/src/share/vm/c1/c1_LIRGenerator.cpp	Tue Nov 01 14:22:38 2016 +0100
@@ -2976,7 +2976,6 @@
   }
 
   // emit invoke code
-  bool optimized = x->target_is_loaded() && x->target_is_final();
   assert(receiver->is_illegal() || receiver->is_equal(LIR_Assembler::receiverOpr()), "must match");
 
   // JSR 292
@@ -3001,9 +3000,9 @@
     case Bytecodes::_invokespecial:
     case Bytecodes::_invokevirtual:
     case Bytecodes::_invokeinterface:
-      // for final target we still produce an inline cache, in order
-      // to be able to call mixed mode
-      if (x->code() == Bytecodes::_invokespecial || optimized) {
+      // for loaded and final (method or class) target we still produce an inline cache,
+      // in order to be able to call mixed mode
+      if (x->code() == Bytecodes::_invokespecial || x->target_is_final()) {
         __ call_opt_virtual(target, receiver, result_register,
                             SharedRuntime::get_resolve_opt_virtual_call_stub(),
                             arg_list, info);
--- a/hotspot/src/share/vm/code/compiledIC.cpp	Mon Oct 31 11:36:45 2016 +0100
+++ b/hotspot/src/share/vm/code/compiledIC.cpp	Tue Nov 01 14:22:38 2016 +0100
@@ -460,9 +460,11 @@
 }
 
 
-// is_optimized: Compiler has generated an optimized call (i.e., no inline
-// cache) static_bound: The call can be static bound (i.e, no need to use
-// inline cache)
+// is_optimized: Compiler has generated an optimized call (i.e. fixed, no inline cache)
+// static_bound: The call can be static bound. If it isn't also optimized, the property
+// wasn't provable at time of compilation. An optimized call will have any necessary
+// null check, while a static_bound won't. A static_bound (but not optimized) must
+// therefore use the unverified entry point.
 void CompiledIC::compute_monomorphic_entry(const methodHandle& method,
                                            KlassHandle receiver_klass,
                                            bool is_optimized,
@@ -475,7 +477,23 @@
   if (method_code != NULL && method_code->is_in_use()) {
     assert(method_code->is_compiled(), "must be compiled");
     // Call to compiled code
-    if (static_bound || is_optimized) {
+    //
+    // Note: the following problem exists with Compiler1:
+    //   - at compile time we may or may not know if the destination is final
+    //   - if we know that the destination is final (is_optimized), we will emit
+    //     an optimized virtual call (no inline cache), and need a Method* to make
+    //     a call to the interpreter
+    //   - if we don't know if the destination is final, we emit a standard
+    //     virtual call, and use CompiledICHolder to call interpreted code
+    //     (no static call stub has been generated)
+    //   - In the case that we here notice the call is static bound we
+    //     convert the call into what looks to be an optimized virtual call,
+    //     but we must use the unverified entry point (since there will be no
+    //     null check on a call when the target isn't loaded).
+    //     This causes problems when verifying the IC because
+    //     it looks vanilla but is optimized. Code in is_call_to_interpreted
+    //     is aware of this and weakens its asserts.
+    if (is_optimized) {
       entry      = method_code->verified_entry_point();
     } else {
       entry      = method_code->entry_point();
@@ -485,38 +503,6 @@
     // Call to compiled code
     info.set_compiled_entry(entry, (static_bound || is_optimized) ? NULL : receiver_klass(), is_optimized);
   } else {
-    // Note: the following problem exists with Compiler1:
-    //   - at compile time we may or may not know if the destination is final
-    //   - if we know that the destination is final, we will emit an optimized
-    //     virtual call (no inline cache), and need a Method* to make a call
-    //     to the interpreter
-    //   - if we do not know if the destination is final, we emit a standard
-    //     virtual call, and use CompiledICHolder to call interpreted code
-    //     (no static call stub has been generated)
-    //     However in that case we will now notice it is static_bound
-    //     and convert the call into what looks to be an optimized
-    //     virtual call. This causes problems in verifying the IC because
-    //     it look vanilla but is optimized. Code in is_call_to_interpreted
-    //     is aware of this and weakens its asserts.
-
-    // static_bound should imply is_optimized -- otherwise we have a
-    // performance bug (statically-bindable method is called via
-    // dynamically-dispatched call note: the reverse implication isn't
-    // necessarily true -- the call may have been optimized based on compiler
-    // analysis (static_bound is only based on "final" etc.)
-#ifdef COMPILER2
-#ifdef TIERED
-#if defined(ASSERT)
-    // can't check the assert because we don't have the CompiledIC with which to
-    // find the address if the call instruction.
-    //
-    // CodeBlob* cb = find_blob_unsafe(instruction_address());
-    // assert(cb->is_compiled_by_c1() || !static_bound || is_optimized, "static_bound should imply is_optimized");
-#endif // ASSERT
-#else
-    assert(!static_bound || is_optimized, "static_bound should imply is_optimized");
-#endif // TIERED
-#endif // COMPILER2
     if (is_optimized) {
       // Use stub entry
       info.set_interpreter_entry(method()->get_c2i_entry(), method());