From b2f75e8cf86a72349c4bb0748a8425ca1cef3677 Mon Sep 17 00:00:00 2001 From: iveresov Date: Thu, 2 Dec 2010 17:21:12 -0800 Subject: [PATCH] 7003554: (tiered) assert(is_null_object() || handle() != NULL) failed: cannot embed null pointer Summary: C1 with profiling doesn't check whether the MDO has been really allocated, which can silently fail if the perm gen is full. The solution is to check if the allocation failed and bailout out of inlining or compilation. Reviewed-by: kvn, never --- src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp | 14 +++-------- src/cpu/x86/vm/c1_LIRAssembler_x86.cpp | 21 +++++----------- src/share/vm/c1/c1_Compilation.cpp | 10 ++++---- src/share/vm/c1/c1_GraphBuilder.cpp | 3 +++ src/share/vm/c1/c1_IR.cpp | 7 +++++- src/share/vm/c1/c1_LIRGenerator.cpp | 18 ++++++-------- src/share/vm/ci/ciMethod.cpp | 29 ++++++++++++++-------- src/share/vm/ci/ciMethod.hpp | 5 ++-- src/share/vm/opto/compile.cpp | 2 +- src/share/vm/opto/graphKit.cpp | 2 +- 10 files changed, 55 insertions(+), 56 deletions(-) diff --git a/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp b/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp index d933aa000..8304e4019 100644 --- a/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp +++ b/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp @@ -2419,11 +2419,8 @@ void LIR_Assembler::type_profile_helper(Register mdo, int mdo_offset_bias, void LIR_Assembler::setup_md_access(ciMethod* method, int bci, ciMethodData*& md, ciProfileData*& data, int& mdo_offset_bias) { - md = method->method_data(); - if (md == NULL) { - bailout("out of memory building methodDataOop"); - return; - } + md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); data = md->bci_to_data(bci); assert(data != NULL, "need data for checkcast"); assert(data->is_ReceiverTypeData(), "need ReceiverTypeData for type check"); @@ -2821,11 +2818,8 @@ void LIR_Assembler::emit_profile_call(LIR_OpProfileCall* op) { int bci = op->profiled_bci(); // Update counter for all call types - ciMethodData* md = method->method_data(); - if (md == NULL) { - bailout("out of memory building methodDataOop"); - return; - } + ciMethodData* md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); ciProfileData* data = md->bci_to_data(bci); assert(data->is_CounterData(), "need CounterData for calls"); assert(op->mdo()->is_single_cpu(), "mdo must be allocated"); diff --git a/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp b/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp index 4daac1aaf..25596b2c0 100644 --- a/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp +++ b/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp @@ -1716,11 +1716,8 @@ void LIR_Assembler::emit_typecheck_helper(LIR_OpTypeCheck *op, Label* success, L ciMethod* method = op->profiled_method(); assert(method != NULL, "Should have method"); int bci = op->profiled_bci(); - md = method->method_data(); - if (md == NULL) { - bailout("out of memory building methodDataOop"); - return; - } + md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); data = md->bci_to_data(bci); assert(data != NULL, "need data for type check"); assert(data->is_ReceiverTypeData(), "need ReceiverTypeData for type check"); @@ -1879,11 +1876,8 @@ void LIR_Assembler::emit_opTypeCheck(LIR_OpTypeCheck* op) { ciMethod* method = op->profiled_method(); assert(method != NULL, "Should have method"); int bci = op->profiled_bci(); - md = method->method_data(); - if (md == NULL) { - bailout("out of memory building methodDataOop"); - return; - } + md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); data = md->bci_to_data(bci); assert(data != NULL, "need data for type check"); assert(data->is_ReceiverTypeData(), "need ReceiverTypeData for type check"); @@ -3373,11 +3367,8 @@ void LIR_Assembler::emit_profile_call(LIR_OpProfileCall* op) { int bci = op->profiled_bci(); // Update counter for all call types - ciMethodData* md = method->method_data(); - if (md == NULL) { - bailout("out of memory building methodDataOop"); - return; - } + ciMethodData* md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); ciProfileData* data = md->bci_to_data(bci); assert(data->is_CounterData(), "need CounterData for calls"); assert(op->mdo()->is_single_cpu(), "mdo must be allocated"); diff --git a/src/share/vm/c1/c1_Compilation.cpp b/src/share/vm/c1/c1_Compilation.cpp index f4c353f45..9b55f915e 100644 --- a/src/share/vm/c1/c1_Compilation.cpp +++ b/src/share/vm/c1/c1_Compilation.cpp @@ -298,8 +298,8 @@ int Compilation::compile_java_method() { CHECK_BAILOUT_(no_frame_size); - if (is_profiling()) { - method()->build_method_data(); + if (is_profiling() && !method()->ensure_method_data()) { + BAILOUT_("mdo allocation failed", no_frame_size); } { @@ -484,11 +484,11 @@ Compilation::Compilation(AbstractCompiler* compiler, ciEnv* env, ciMethod* metho if (is_profiling()) { // Compilation failed, create MDO, which would signal the interpreter // to start profiling on its own. - _method->build_method_data(); + _method->ensure_method_data(); } } else if (is_profiling() && _would_profile) { - ciMethodData *md = method->method_data(); - assert (md != NULL, "Should have MDO"); + ciMethodData *md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); md->set_would_profile(_would_profile); } } diff --git a/src/share/vm/c1/c1_GraphBuilder.cpp b/src/share/vm/c1/c1_GraphBuilder.cpp index 492e21f4d..1aa2cc00a 100644 --- a/src/share/vm/c1/c1_GraphBuilder.cpp +++ b/src/share/vm/c1/c1_GraphBuilder.cpp @@ -3377,6 +3377,9 @@ bool GraphBuilder::try_inline_full(ciMethod* callee, bool holder_known) { INLINE_BAILOUT("total inlining greater than DesiredMethodLimit"); } + if (is_profiling() && !callee->ensure_method_data()) { + INLINE_BAILOUT("mdo allocation failed"); + } #ifndef PRODUCT // printing if (PrintInlining) { diff --git a/src/share/vm/c1/c1_IR.cpp b/src/share/vm/c1/c1_IR.cpp index 662b9ed06..917fc5bd5 100644 --- a/src/share/vm/c1/c1_IR.cpp +++ b/src/share/vm/c1/c1_IR.cpp @@ -504,7 +504,12 @@ ComputeLinearScanOrder::ComputeLinearScanOrder(Compilation* c, BlockBegin* start count_edges(start_block, NULL); if (compilation()->is_profiling()) { - compilation()->method()->method_data()->set_compilation_stats(_num_loops, _num_blocks); + ciMethod *method = compilation()->method(); + if (!method->is_accessor()) { + ciMethodData* md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); + md->set_compilation_stats(_num_loops, _num_blocks); + } } if (_num_loops > 0) { diff --git a/src/share/vm/c1/c1_LIRGenerator.cpp b/src/share/vm/c1/c1_LIRGenerator.cpp index b81d639f5..56b22f686 100644 --- a/src/share/vm/c1/c1_LIRGenerator.cpp +++ b/src/share/vm/c1/c1_LIRGenerator.cpp @@ -836,11 +836,8 @@ void LIRGenerator::profile_branch(If* if_instr, If::Condition cond) { if (if_instr->should_profile()) { ciMethod* method = if_instr->profiled_method(); assert(method != NULL, "method should be set if branch is profiled"); - ciMethodData* md = method->method_data(); - if (md == NULL) { - bailout("out of memory building methodDataOop"); - return; - } + ciMethodData* md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); ciProfileData* data = md->bci_to_data(if_instr->profiled_bci()); assert(data != NULL, "must have profiling data"); assert(data->is_BranchData(), "need BranchData for two-way branches"); @@ -2219,11 +2216,8 @@ void LIRGenerator::do_Goto(Goto* x) { if (x->should_profile()) { ciMethod* method = x->profiled_method(); assert(method != NULL, "method should be set if branch is profiled"); - ciMethodData* md = method->method_data(); - if (md == NULL) { - bailout("out of memory building methodDataOop"); - return; - } + ciMethodData* md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); ciProfileData* data = md->bci_to_data(x->profiled_bci()); assert(data != NULL, "must have profiling data"); int offset; @@ -2723,7 +2717,9 @@ void LIRGenerator::increment_event_counter_impl(CodeEmitInfo* info, } else if (level == CompLevel_full_profile) { offset = in_bytes(backedge ? methodDataOopDesc::backedge_counter_offset() : methodDataOopDesc::invocation_counter_offset()); - __ oop2reg(method->method_data()->constant_encoding(), counter_holder); + ciMethodData* md = method->method_data_or_null(); + assert(md != NULL, "Sanity"); + __ oop2reg(md->constant_encoding(), counter_holder); meth = new_register(T_OBJECT); __ oop2reg(method->constant_encoding(), meth); } else { diff --git a/src/share/vm/ci/ciMethod.cpp b/src/share/vm/ci/ciMethod.cpp index 9e27c07cc..d1d86923f 100644 --- a/src/share/vm/ci/ciMethod.cpp +++ b/src/share/vm/ci/ciMethod.cpp @@ -797,12 +797,13 @@ ciInstance* ciMethod::method_handle_type() { // ------------------------------------------------------------------ -// ciMethod::build_method_data +// ciMethod::ensure_method_data // // Generate new methodDataOop objects at compile time. -void ciMethod::build_method_data(methodHandle h_m) { +// Return true if allocation was successful or no MDO is required. +bool ciMethod::ensure_method_data(methodHandle h_m) { EXCEPTION_CONTEXT; - if (is_native() || is_abstract() || h_m()->is_accessor()) return; + if (is_native() || is_abstract() || h_m()->is_accessor()) return true; if (h_m()->method_data() == NULL) { methodOopDesc::build_interpreter_method_data(h_m, THREAD); if (HAS_PENDING_EXCEPTION) { @@ -812,18 +813,22 @@ void ciMethod::build_method_data(methodHandle h_m) { if (h_m()->method_data() != NULL) { _method_data = CURRENT_ENV->get_object(h_m()->method_data())->as_method_data(); _method_data->load_data(); + return true; } else { _method_data = CURRENT_ENV->get_empty_methodData(); + return false; } } // public, retroactive version -void ciMethod::build_method_data() { +bool ciMethod::ensure_method_data() { + bool result = true; if (_method_data == NULL || _method_data->is_empty()) { GUARDED_VM_ENTRY({ - build_method_data(get_methodOop()); + result = ensure_method_data(get_methodOop()); }); } + return result; } @@ -839,11 +844,6 @@ ciMethodData* ciMethod::method_data() { Thread* my_thread = JavaThread::current(); methodHandle h_m(my_thread, get_methodOop()); - // Create an MDO for the inlinee - if (TieredCompilation && is_c1_compile(env->comp_level())) { - build_method_data(h_m); - } - if (h_m()->method_data() != NULL) { _method_data = CURRENT_ENV->get_object(h_m()->method_data())->as_method_data(); _method_data->load_data(); @@ -854,6 +854,15 @@ ciMethodData* ciMethod::method_data() { } +// ------------------------------------------------------------------ +// ciMethod::method_data_or_null +// Returns a pointer to ciMethodData if MDO exists on the VM side, +// NULL otherwise. +ciMethodData* ciMethod::method_data_or_null() { + ciMethodData *md = method_data(); + if (md->is_empty()) return NULL; + return md; +} // ------------------------------------------------------------------ // ciMethod::will_link diff --git a/src/share/vm/ci/ciMethod.hpp b/src/share/vm/ci/ciMethod.hpp index 05df3ccc8..63d4bdc7c 100644 --- a/src/share/vm/ci/ciMethod.hpp +++ b/src/share/vm/ci/ciMethod.hpp @@ -106,7 +106,7 @@ class ciMethod : public ciObject { void check_is_loaded() const { assert(is_loaded(), "not loaded"); } - void build_method_data(methodHandle h_m); + bool ensure_method_data(methodHandle h_m); void code_at_put(int bci, Bytecodes::Code code) { Bytecodes::check(code); @@ -121,6 +121,7 @@ class ciMethod : public ciObject { ciSymbol* name() const { return _name; } ciInstanceKlass* holder() const { return _holder; } ciMethodData* method_data(); + ciMethodData* method_data_or_null(); // Signature information. ciSignature* signature() const { return _signature; } @@ -230,7 +231,7 @@ class ciMethod : public ciObject { bool has_unloaded_classes_in_signature(); bool is_klass_loaded(int refinfo_index, bool must_be_resolved) const; bool check_call(int refinfo_index, bool is_static) const; - void build_method_data(); // make sure it exists in the VM also + bool ensure_method_data(); // make sure it exists in the VM also int scale_count(int count, float prof_factor = 1.); // make MDO count commensurate with IIC // JSR 292 support diff --git a/src/share/vm/opto/compile.cpp b/src/share/vm/opto/compile.cpp index 677279c5d..972cc34ae 100644 --- a/src/share/vm/opto/compile.cpp +++ b/src/share/vm/opto/compile.cpp @@ -553,7 +553,7 @@ Compile::Compile( ciEnv* ci_env, C2Compiler* compiler, ciMethod* target, int osr if (ProfileTraps) { // Make sure the method being compiled gets its own MDO, // so we can at least track the decompile_count(). - method()->build_method_data(); + method()->ensure_method_data(); } Init(::AliasLevel); diff --git a/src/share/vm/opto/graphKit.cpp b/src/share/vm/opto/graphKit.cpp index 1a3333db7..80101d2da 100644 --- a/src/share/vm/opto/graphKit.cpp +++ b/src/share/vm/opto/graphKit.cpp @@ -1841,7 +1841,7 @@ void GraphKit::uncommon_trap(int trap_request, // Note: If ProfileTraps is true, and if a deopt. actually // occurs here, the runtime will make sure an MDO exists. There is - // no need to call method()->build_method_data() at this point. + // no need to call method()->ensure_method_data() at this point. #ifdef ASSERT if (!must_throw) { -- GitLab