# HG changeset patch # User neliasso # Date 1478006558 -3600 # Node ID 58de8aaf93659c86f8cd35f8f8af61af9b75c565 # Parent 9e7ac7aff2d11de12b099fb8bf406c05556d48d0 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 diff -r 9e7ac7aff2d1 -r 58de8aaf9365 hotspot/src/share/vm/c1/c1_GraphBuilder.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); diff -r 9e7ac7aff2d1 -r 58de8aaf9365 hotspot/src/share/vm/c1/c1_LIRGenerator.cpp --- 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); diff -r 9e7ac7aff2d1 -r 58de8aaf9365 hotspot/src/share/vm/code/compiledIC.cpp --- 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());