8257965: [lworld] C2 compilation fails with assert(unc->peek_monitor_obj() == obj) failed: wrong monitor

src/hotspot/share/c1/c1_LIRAssembler.cpp

@@ -690,7 +690,7 @@ void LIR_Assembler::emit_std_entries() {
                            offsets()->value(ro_entry_type));
     }
   } else {
-    // All 3 entries are the same (no value-type packing)
+    // All 3 entries are the same (no inline type packing)
     offsets()->set_value(CodeOffsets::Entry, _masm->offset());
     offsets()->set_value(CodeOffsets::Inline_Entry, _masm->offset());
     if (needs_icache(method())) {

src/hotspot/share/opto/callnode.cpp

@@ -841,23 +841,22 @@ bool CallNode::may_modify(const TypeOopPtr *t_oop, PhaseTransform *phase) {
 }
 
 // Does this call have a direct reference to n other than debug information?
-bool CallNode::has_non_debug_use(Node *n) {
-  const TypeTuple * d = tf()->domain_cc();
+bool CallNode::has_non_debug_use(Node* n) {
+  const TypeTuple* d = tf()->domain_cc();
   for (uint i = TypeFunc::Parms; i < d->cnt(); i++) {
-    Node *arg = in(i);
-    if (arg == n) {
+    if (in(i) == n) {
       return true;
     }
   }
   return false;
 }
 
-bool CallNode::has_debug_use(Node *n) {
-  assert(jvms() != NULL, "jvms should not be null");
-  for (uint i = jvms()->debug_start(); i < jvms()->debug_end(); i++) {
-    Node *arg = in(i);
-    if (arg == n) {
-      return true;
+bool CallNode::has_debug_use(Node* n) {
+  if (jvms() != NULL) {
+    for (uint i = jvms()->debug_start(); i < jvms()->debug_end(); i++) {
+      if (in(i) == n) {
+        return true;
+      }
     }
   }
   return false;

src/hotspot/share/opto/callnode.hpp

@@ -642,8 +642,8 @@ class CallNode : public SafePointNode {
   // Returns true if the call may modify n
   virtual bool        may_modify(const TypeOopPtr *t_oop, PhaseTransform *phase);
   // Does this node have a use of n other than in debug information?
-  bool                has_non_debug_use(Node *n);
-  bool                has_debug_use(Node *n);
+  bool                has_non_debug_use(Node* n);
+  bool                has_debug_use(Node* n);
   // Returns the unique CheckCastPP of a call
   // or result projection is there are several CheckCastPP
   // or returns NULL if there is no one.

src/hotspot/share/opto/locknode.cpp

@@ -183,8 +183,8 @@ void Parse::do_monitor_enter() {
   kill_dead_locals();
 
   Node* obj = peek();
-
-  if (obj->is_InlineType()) {
+  const Type* obj_type = gvn().type(obj);
+  if (obj_type->isa_inlinetype() && !obj_type->is_inlinetypeptr()) {
     uncommon_trap(Deoptimization::Reason_class_check,
                   Deoptimization::Action_none);
     return;

src/hotspot/share/opto/macro.cpp

@@ -2227,6 +2227,48 @@ void PhaseMacroExpand::mark_eliminated_locking_nodes(AbstractLockNode *alock) {
   }
 }
 
+void PhaseMacroExpand::inline_type_guard(Node** ctrl, LockNode* lock) {
+  Node* obj = lock->obj_node();
+  const TypePtr* obj_type = _igvn.type(obj)->make_ptr();
+  if (!obj_type->can_be_inline_type()) {
+    return;
+  }
+  Node* mark = make_load(*ctrl, lock->memory(), obj, oopDesc::mark_offset_in_bytes(), TypeX_X, TypeX_X->basic_type());
+  Node* value_mask = _igvn.MakeConX(markWord::inline_type_pattern);
+  Node* is_value = _igvn.transform(new AndXNode(mark, value_mask));
+  Node* cmp = _igvn.transform(new CmpXNode(is_value, value_mask));
+  Node* bol = _igvn.transform(new BoolNode(cmp, BoolTest::eq));
+  Node* unc_ctrl = generate_slow_guard(ctrl, bol, NULL);
+
+  int trap_request = Deoptimization::make_trap_request(Deoptimization::Reason_class_check, Deoptimization::Action_none);
+  address call_addr = SharedRuntime::uncommon_trap_blob()->entry_point();
+  const TypePtr* no_memory_effects = NULL;
+  CallNode* unc = new CallStaticJavaNode(OptoRuntime::uncommon_trap_Type(), call_addr, "uncommon_trap",
+                                         lock->jvms()->bci(), no_memory_effects);
+  unc->init_req(TypeFunc::Control, unc_ctrl);
+  unc->init_req(TypeFunc::I_O, lock->i_o());
+  unc->init_req(TypeFunc::Memory, lock->memory());
+  unc->init_req(TypeFunc::FramePtr,  lock->in(TypeFunc::FramePtr));
+  unc->init_req(TypeFunc::ReturnAdr, lock->in(TypeFunc::ReturnAdr));
+  unc->init_req(TypeFunc::Parms+0, _igvn.intcon(trap_request));
+  unc->set_cnt(PROB_UNLIKELY_MAG(4));
+  unc->copy_call_debug_info(&_igvn, lock);
+
+  assert(unc->peek_monitor_box() == lock->box_node(), "wrong monitor");
+  assert((obj_type->is_inlinetypeptr() && unc->peek_monitor_obj()->is_SafePointScalarObject()) ||
+         (unc->peek_monitor_obj() == lock->obj_node()), "wrong monitor");
+
+  // pop monitor and push obj back on stack: we trap before the monitorenter
+  unc->pop_monitor();
+  unc->grow_stack(unc->jvms(), 1);
+  unc->set_stack(unc->jvms(), unc->jvms()->stk_size()-1, obj);
+  _igvn.register_new_node_with_optimizer(unc);
+
+  unc_ctrl = _igvn.transform(new ProjNode(unc, TypeFunc::Control));
+  Node* halt = _igvn.transform(new HaltNode(unc_ctrl, lock->in(TypeFunc::FramePtr), "monitor enter on inline type"));
+  C->root()->add_req(halt);
+}
+
 // we have determined that this lock/unlock can be eliminated, we simply
 // eliminate the node without expanding it.
 //
@@ -2239,8 +2281,6 @@ bool PhaseMacroExpand::eliminate_locking_node(AbstractLockNode *alock) {
     return false;
   }
 #ifdef ASSERT
-  const Type* obj_type = _igvn.type(alock->obj_node());
-  assert(!obj_type->isa_inlinetype() && !obj_type->is_inlinetypeptr(), "Eliminating lock on inline type");
   if (!alock->is_coarsened()) {
     // Check that new "eliminated" BoxLock node is created.
     BoxLockNode* oldbox = alock->box_node()->as_BoxLock();
@@ -2279,6 +2319,9 @@ bool PhaseMacroExpand::eliminate_locking_node(AbstractLockNode *alock) {
   // The input to a Lock is merged memory, so extract its RawMem input
   // (unless the MergeMem has been optimized away.)
   if (alock->is_Lock()) {
+    // Deoptimize and re-execute if object is an inline type
+    inline_type_guard(&ctrl, alock->as_Lock());
+
     // Seach for MemBarAcquireLock node and delete it also.
     MemBarNode* membar = fallthroughproj->unique_ctrl_out()->as_MemBar();
     assert(membar != NULL && membar->Opcode() == Op_MemBarAcquireLock, "");
@@ -2522,47 +2565,8 @@ void PhaseMacroExpand::expand_lock_node(LockNode *lock) {
     mem_phi->init_req(2, mem);
   }
 
-  const TypeOopPtr* objptr = _igvn.type(obj)->make_oopptr();
-  if (objptr->can_be_inline_type()) {
-    // Deoptimize and re-execute if a value
-    assert(EnableValhalla, "should only be used if inline types are enabled");
-    Node* mark = make_load(slow_path, mem, obj, oopDesc::mark_offset_in_bytes(), TypeX_X, TypeX_X->basic_type());
-    Node* value_mask = _igvn.MakeConX(markWord::inline_type_pattern);
-    Node* is_value = _igvn.transform(new AndXNode(mark, value_mask));
-    Node* cmp = _igvn.transform(new CmpXNode(is_value, value_mask));
-    Node* bol = _igvn.transform(new BoolNode(cmp, BoolTest::eq));
-    Node* unc_ctrl = generate_slow_guard(&slow_path, bol, NULL);
-
-    int trap_request = Deoptimization::make_trap_request(Deoptimization::Reason_class_check, Deoptimization::Action_none);
-    address call_addr = SharedRuntime::uncommon_trap_blob()->entry_point();
-    const TypePtr* no_memory_effects = NULL;
-    JVMState* jvms = lock->jvms();
-    CallNode* unc = new CallStaticJavaNode(OptoRuntime::uncommon_trap_Type(), call_addr, "uncommon_trap",
-                                           jvms->bci(), no_memory_effects);
-
-    unc->init_req(TypeFunc::Control, unc_ctrl);
-    unc->init_req(TypeFunc::I_O, lock->i_o());
-    unc->init_req(TypeFunc::Memory, mem); // may gc ptrs
-    unc->init_req(TypeFunc::FramePtr,  lock->in(TypeFunc::FramePtr));
-    unc->init_req(TypeFunc::ReturnAdr, lock->in(TypeFunc::ReturnAdr));
-    unc->init_req(TypeFunc::Parms+0, _igvn.intcon(trap_request));
-    unc->set_cnt(PROB_UNLIKELY_MAG(4));
-    unc->copy_call_debug_info(&_igvn, lock);
-
-    assert(unc->peek_monitor_box() == box, "wrong monitor");
-    assert(unc->peek_monitor_obj() == obj, "wrong monitor");
-
-    // pop monitor and push obj back on stack: we trap before the monitorenter
-    unc->pop_monitor();
-    unc->grow_stack(unc->jvms(), 1);
-    unc->set_stack(unc->jvms(), unc->jvms()->stk_size()-1, obj);
-
-    _igvn.register_new_node_with_optimizer(unc);
-
-    Node* ctrl = _igvn.transform(new ProjNode(unc, TypeFunc::Control));
-    Node* halt = _igvn.transform(new HaltNode(ctrl, lock->in(TypeFunc::FramePtr), "monitor enter on value-type"));
-    C->root()->add_req(halt);
-  }
+  // Deoptimize and re-execute if object is an inline type
+  inline_type_guard(&slow_path, lock);
 
   // Make slow path call
   CallNode *call = make_slow_call((CallNode *) lock, OptoRuntime::complete_monitor_enter_Type(),

src/hotspot/share/opto/macro.hpp

@@ -117,6 +117,7 @@ class PhaseMacroExpand : public Phase {
   void mark_eliminated_locking_nodes(AbstractLockNode *alock);
   bool eliminate_locking_node(AbstractLockNode *alock);
   void expand_lock_node(LockNode *lock);
+  void inline_type_guard(Node** ctrl, LockNode* lock);
   void expand_unlock_node(UnlockNode *unlock);
   void expand_mh_intrinsic_return(CallStaticJavaNode* call);
 

test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java

@@ -3527,4 +3527,101 @@ public void test129_verifier(boolean warmup) {
             Asserts.assertEquals(res, testValue2.hash());
         }
     }
+
+    // Lock on inline type (known after inlining)
+    @ForceInline
+    public Object test130_inlinee() {
+        return MyValue1.createWithFieldsInline(rI, rL);
+    }
+
+    @Test()
+    public void test130() {
+        Object obj = test130_inlinee();
+        synchronized (obj) {
+            throw new RuntimeException("test130 failed: synchronization on inline type should not succeed");
+        }
+    }
+
+    @DontCompile
+    public void test130_verifier(boolean warmup) {
+        try {
+            test130();
+            throw new RuntimeException("test130 failed: no exception thrown");
+        } catch (IllegalMonitorStateException ex) {
+            // Expected
+        }
+    }
+
+    // Same as test130 but with field load instead of allocation
+    @ForceInline
+    public Object test131_inlinee() {
+        return testValue1;
+    }
+
+    @Test()
+    public void test131() {
+        Object obj = test131_inlinee();
+        synchronized (obj) {
+            throw new RuntimeException("test131 failed: synchronization on inline type should not succeed");
+        }
+    }
+
+    @DontCompile
+    public void test131_verifier(boolean warmup) {
+        try {
+            test131();
+            throw new RuntimeException("test131 failed: no exception thrown");
+        } catch (IllegalMonitorStateException ex) {
+            // Expected
+        }
+    }
+
+    // Test locking on object that is known to be an inline type only after CCP
+    @Test()
+    @Warmup(10000)
+    public void test132() {
+        MyValue2 vt = MyValue2.createWithFieldsInline(rI, rD);
+        Object obj = new Integer(42);
+
+        int limit = 2;
+        for (; limit < 4; limit *= 2);
+        for (int i = 2; i < limit; i++) {
+            obj = vt;
+        }
+        synchronized (obj) {
+            throw new RuntimeException("test132 failed: synchronization on inline type should not succeed");
+        }
+    }
+
+    @DontCompile
+    public void test132_verifier(boolean warmup) {
+        try {
+            test132();
+            throw new RuntimeException("test132 failed: no exception thrown");
+        } catch (IllegalMonitorStateException ex) {
+            // Expected
+        }
+    }
+
+    // Test conditional locking on inline type and non-escaping object
+    @Test()
+    public void test133(boolean b) {
+        Object obj = b ? new Integer(42) : MyValue2.createWithFieldsInline(rI, rD);
+        synchronized (obj) {
+            if (!b) {
+                throw new RuntimeException("test133 failed: synchronization on inline type should not succeed");
+            }
+        }
+    }
+
+    @DontCompile
+    public void test133_verifier(boolean warmup) {
+        test133(true);
+        try {
+            test133(false);
+            throw new RuntimeException("test133 failed: no exception thrown");
+        } catch (IllegalMonitorStateException ex) {
+            // Expected
+        }
+    }
 }