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
--- 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());