8334421: assert(!oldbox->is_unbalanced()) failed: this should not be …

…called for unbalanced region

Reviewed-by: vlivanov, thartmann

src/hotspot/share/opto/callnode.cpp

@@ -1950,6 +1950,22 @@ bool AbstractLockNode::find_unlocks_for_region(const RegionNode* region, LockNod
 
 }
 
+// Check that all locks/unlocks associated with object come from balanced regions.
+bool AbstractLockNode::is_balanced() {
+  Node* obj = obj_node();
+  for (uint j = 0; j < obj->outcnt(); j++) {
+    Node* n = obj->raw_out(j);
+    if (n->is_AbstractLock() &&
+        n->as_AbstractLock()->obj_node()->eqv_uncast(obj)) {
+      BoxLockNode* n_box = n->as_AbstractLock()->box_node()->as_BoxLock();
+      if (n_box->is_unbalanced()) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
 const char* AbstractLockNode::_kind_names[] = {"Regular", "NonEscObj", "Coarsened", "Nested"};
 
 const char * AbstractLockNode::kind_as_string() const {
@@ -2056,6 +2072,8 @@ Node *LockNode::Ideal(PhaseGVN *phase, bool can_reshape) {
           int unlocks = 0;
           if (Verbose) {
             tty->print_cr("=== Locks coarsening ===");
+            tty->print("Obj: ");
+            obj_node()->dump();
           }
           for (int i = 0; i < lock_ops.length(); i++) {
             AbstractLockNode* lock = lock_ops.at(i);
@@ -2064,6 +2082,8 @@ Node *LockNode::Ideal(PhaseGVN *phase, bool can_reshape) {
             else
               unlocks++;
             if (Verbose) {
+              tty->print("Box %d: ", i);
+              box_node()->dump();
               tty->print(" %d: ", i);
               lock->dump();
             }

src/hotspot/share/opto/callnode.hpp

@@ -1154,6 +1154,10 @@ class AbstractLockNode: public CallNode {
   void set_coarsened()   { _kind = Coarsened; set_eliminated_lock_counter(); }
   void set_nested()      { _kind = Nested; set_eliminated_lock_counter(); }
 
+  // Check that all locks/unlocks associated with object come from balanced regions.
+  // They can become unbalanced after coarsening optimization or on OSR entry.
+  bool is_balanced();
+
   // locking does not modify its arguments
   virtual bool may_modify(const TypeOopPtr* t_oop, PhaseValues* phase){ return false; }
 

src/hotspot/share/opto/escape.cpp

@@ -3502,12 +3502,11 @@ bool ConnectionGraph::not_global_escape(Node *n) {
 // and locked code region (identified by BoxLockNode) is balanced:
 // all compiled code paths have corresponding Lock/Unlock pairs.
 bool ConnectionGraph::can_eliminate_lock(AbstractLockNode* alock) {
-  BoxLockNode* box = alock->box_node()->as_BoxLock();
-  if (!box->is_unbalanced() && not_global_escape(alock->obj_node())) {
+  if (alock->is_balanced() && not_global_escape(alock->obj_node())) {
     if (EliminateNestedLocks) {
       // We can mark whole locking region as Local only when only
       // one object is used for locking.
-      box->set_local();
+      alock->box_node()->as_BoxLock()->set_local();
     }
     return true;
   }

src/hotspot/share/opto/locknode.hpp

@@ -44,7 +44,7 @@ class BoxLockNode : public Node {
     Eliminated         // All lock/unlock in region were eliminated
   } _kind;
 
-#ifdef ASSERT
+#ifndef PRODUCT
   const char* _kind_name[6] = {
    "Regular",
    "Local",
@@ -122,7 +122,9 @@ class BoxLockNode : public Node {
 
 #ifndef PRODUCT
   virtual void format( PhaseRegAlloc *, outputStream *st ) const;
-  virtual void dump_spec(outputStream *st) const { st->print("  Lock %d",_slot); }
+  virtual void dump_spec(outputStream *st) const {
+    st->print("  Lock slot: %d, Kind: %s", _slot, _kind_name[(int)_kind]);
+  }
 #endif
 };
 

src/hotspot/share/opto/macro.cpp

@@ -2045,7 +2045,7 @@ void PhaseMacroExpand::mark_eliminated_box(Node* box, Node* obj) {
 
 //-----------------------mark_eliminated_locking_nodes-----------------------
 void PhaseMacroExpand::mark_eliminated_locking_nodes(AbstractLockNode *alock) {
-  if (alock->box_node()->as_BoxLock()->is_unbalanced()) {
+  if (!alock->is_balanced()) {
     return; // Can't do any more elimination for this locking region
   }
   if (EliminateNestedLocks) {

test/hotspot/jtreg/compiler/locks/TestCoarsenedAndNotEscapedLocksElimination.java

@@ -0,0 +1,205 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8334421
+ * @summary C2 incorrectly marks not-escaped locks for elimination after
+ *          coarsened locks were eliminated and created unbalanced regions.
+ * @requires vm.compMode != "Xint"
+ * @run main/othervm -XX:-TieredCompilation TestCoarsenedAndNotEscapedLocksElimination
+ * @run main TestCoarsenedAndNotEscapedLocksElimination
+ */
+
+import java.util.Vector;
+
+class TestVector extends Vector<Object> {
+
+    TestVector() {
+        super();
+    }
+
+    TestVector(int initialCapacity) {
+        super(initialCapacity);
+    }
+
+    TestVector(int initialCapacity, int capacityIncrement) {
+        super(initialCapacity, capacityIncrement);
+    }
+
+    Object[] getElementData () {
+        return elementData; // access protected field
+    }
+}
+
+public class TestCoarsenedAndNotEscapedLocksElimination {
+
+    public static void main(String[] strArr) {
+        TestCoarsenedAndNotEscapedLocksElimination tc = new TestCoarsenedAndNotEscapedLocksElimination();
+        String result = null;
+        for (int i = 0; i < 12000; ++i) {
+            result = tc.test();
+            if (result != null) break;
+        }
+        System.out.println(result == null? "passed" : result);
+    }
+
+    int [][] vector_types = {
+       {-1, -1},
+       {0, -1},
+       {1, -1},
+       {2, -1},
+       {1025, -1},
+       {0, -2},
+       {1, -2},
+       {2, -2},
+       {1025, -2},
+       {0, 0},
+       {1, 0},
+       {2, 0},
+       {1025, 0},
+       {0, 1},
+       {1, 1},
+       {2, 1},
+       {1025, 1},
+       {0, 1025 },
+       {1, 1025 },
+       {2, 1025 },
+       {1025, 1025 }
+    };
+
+    Object [] elems = {
+        null,
+        new Object(),
+        new Vector(),
+        new Object[0]
+    };
+
+    int cntr = 0, mode = 0;
+
+    void reset() {
+        cntr = 0;
+        mode = 0;
+    }
+
+    TestVector nextVector() {
+        if (cntr == vector_types.length) {
+            return null;
+        } else {
+            TestVector vect;
+            if (vector_types[cntr][0] < 0) {
+                vect = new TestVector();
+            } else if (vector_types[cntr][1] == -2) {
+                vect = new TestVector(vector_types[cntr][0]);
+            } else {
+                vect = new TestVector(vector_types[cntr][0], vector_types[cntr][1]);
+            }
+            if (mode == 1) {
+                vect.addElement(null);
+                vect.addElement(new Object());
+                vect.addElement(new Vector());
+                vect.addElement(new Object[0]);
+            } else if (mode == 2) {
+                int cap = vect.capacity();
+                vect.addElement(null);
+                for (int i = 0; i < cap; i++) {
+                    vect.addElement(new Object());
+                }
+            }
+            if (++mode == 3) {
+                mode = 0;
+                cntr++;
+            }
+            return vect;
+        }
+    }
+
+    public String test() {
+        reset();
+        TestVector vect = (TestVector)nextVector();
+        while (vect != null) {
+            Object [] backup_array = new Object[vect.size()];
+            System.arraycopy(vect.getElementData(),0,backup_array,0,vect.size());
+
+            int old_size = vect.size();
+            vect.setSize(vect.size());
+            if (vect.size() != old_size) {
+                return "Vector: "+vect+" size changed after setSize(size())";
+            }
+            for (int i = 0; i < vect.size(); i++) {
+                if (vect.elementAt(i) != backup_array[i]) {
+                    return "Vector: "+vect+" : "+i+"th element changed after setSize(size())";
+                }
+            }
+
+            old_size = vect.size();
+            vect.setSize(vect.size()*2);
+            if (vect.size() != old_size*2) {
+                return "Vector: "+vect+" size incorrectly changed after setSize(size()*2)";
+            }
+            for (int i = 0; i < old_size; i++) {
+                if (vect.elementAt(i) != backup_array[i]) {
+                    return "Vector: "+vect+" : "+i+"th element changed after setSize(size()*2)";
+                }
+            }
+            for (int i = old_size; i < old_size*2; i++) {
+                if (vect.elementAt(i) != null) {
+                    return "Vector: "+vect+" : "+i+"th element not null after setSize(size()*2)";
+                }
+            }
+
+            old_size = vect.size();
+            int old_cap = vect.capacity();
+            vect.setSize(vect.capacity()+1);
+            if (vect.size() != old_cap+1) {
+                return "Vector: "+vect+" size incorrectly changed after setSize(capacity()+1)";
+            }
+            for (int i = 0; i < old_size && i < backup_array.length; i++) {
+                if (vect.elementAt(i) != backup_array[i]) {
+                    return "Vector: "+vect+" : "+i+"th element changed after setSize(capacity()+1)";
+                }
+            }
+            for (int i = old_size; i < old_cap + 1; i++) {
+                if (vect.elementAt(i) != null) {
+                    return "Vector: "+vect+" : "+i+"th element not null after setSize(capacity()+1)";
+                }
+            }
+
+            old_size = vect.size();
+            vect.setSize(vect.size()/2);
+            if (vect.size() != old_size/2) {
+                return "Vector: "+vect+" size incorrectly changed after setSize(size()/2)";
+            }
+            for (int i = 0; i < old_size/2 && i < backup_array.length; i++) {
+                if (vect.elementAt(i) != backup_array[i]) {
+                    return "Vector: "+vect+" : "+i+"th element changed after setSize(size()/2)";
+                }
+            }
+
+            vect = nextVector();
+        }
+        return null;
+    }
+
+}
+