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

8 files changed, 45 insertions, 31 deletions

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) {