Ensure FrameStates don't violate lock order forced by lightweight locking.

Author: mur47x111

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/MonitorPEATest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 2025, 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
@@ -33,13 +33,7 @@
 import org.junit.Test;
 
 import jdk.graal.compiler.api.directives.GraalDirectives;
-import jdk.graal.compiler.graph.Node;
-import jdk.graal.compiler.graph.NodeFlood;
 import jdk.graal.compiler.hotspot.replacements.HotSpotReplacementsUtil;
-import jdk.graal.compiler.nodes.AbstractEndNode;
-import jdk.graal.compiler.nodes.StructuredGraph;
-import jdk.graal.compiler.nodes.java.MonitorEnterNode;
-import jdk.graal.compiler.nodes.java.MonitorExitNode;
 
 /**
  * Tests that PEA preserves the monitorenter order. This is essential for lightweight locking.
@@ -164,37 +158,7 @@ public void testSnippet4() {
         test("snippet4", new Object(), true, true);
     }
 
-    @Override
-    protected void checkMidTierGraph(StructuredGraph graph) {
-        // The following lock depth check only works with simple control flow.
-        NodeFlood flood = graph.createNodeFlood();
-        flood.add(graph.start());
-
-        int lockDepth = -1;
-
-        for (Node current : flood) {
-            if (current instanceof AbstractEndNode) {
-                AbstractEndNode end = (AbstractEndNode) current;
-                flood.add(end.merge());
-            } else {
-                if (current instanceof MonitorEnterNode enter) {
-                    int depth = enter.getMonitorId().getLockDepth();
-                    assertTrue(lockDepth < depth);
-                    lockDepth = depth;
-                } else if (current instanceof MonitorExitNode exit) {
-                    int depth = exit.getMonitorId().getLockDepth();
-                    assertTrue(lockDepth >= depth);
-                    lockDepth = depth;
-                }
-
-                for (Node successor : current.successors()) {
-                    flood.add(successor);
-                }
-            }
-        }
-    }
-
-    static class B {
+    public static class B {
         Float f = 1.0f;
     }
 
@@ -296,6 +260,88 @@ public void testSnippet9() {
         test("snippet9");
     }
 
+    @BytecodeParserNeverInline
+    public static void callee0() {
+        synchronized (B.class) {
+        }
+    }
+
+    @BytecodeParserNeverInline
+    public static void callee1() {
+        staticObj = new B();
+        callee0();
+    }
+
+    @BytecodeParserNeverInline
+    public static void callee() {
+        callee1();
+    }
+
+    public static void snippet10() {
+        B b = new B();
+        synchronized (b) {
+            callee();
+        }
+    }
+
+    @Test
+    public void testSnippet10() {
+        test("snippet10");
+    }
+
+    static RuntimeException cachedException = new RuntimeException();
+
+    public static void snippet11(boolean flag) {
+        B b = new B();
+        synchronized (b) {
+            if (flag) {
+                staticObj = b;
+                throw cachedException;
+            }
+        }
+    }
+
+    @Test
+    public void testSnippet11() {
+        test("snippet11", true);
+    }
+
+    public static void snippet12() {
+        B b = new B();
+        synchronized (b) {
+            callee();
+            staticObj = b;
+        }
+    }
+
+    @Test
+    public void testSnippet12() {
+        test("snippet12");
+    }
+
+    public static void snippet13(boolean flag, boolean escape) {
+        B b = new B();
+
+        synchronized (b) {
+            synchronized (b) {
+                if (GraalDirectives.injectBranchProbability(0.9, flag)) {
+                    GraalDirectives.controlFlowAnchor();
+                    if (GraalDirectives.injectBranchProbability(0.1, escape)) {
+                        staticObj = b;
+                    }
+                } else {
+                    staticObj1 = b;
+                    throw cachedException;
+                }
+            }
+        }
+    }
+
+    @Test
+    public void testSnippet13() {
+        test("snippet13", true, true);
+    }
+
     public static void snippet15(boolean deoptimize) {
         synchronized (new B()) {
             synchronized (new StringBuilder()) {

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/HotSpotDebugInfoBuilder.java

@@ -45,7 +45,6 @@
 import jdk.graal.compiler.nodes.extended.ForeignCall;
 import jdk.graal.compiler.nodes.spi.NodeValueMap;
 import jdk.graal.compiler.nodes.spi.NodeWithState;
-
 import jdk.vm.ci.code.BytecodeFrame;
 import jdk.vm.ci.code.StackLockValue;
 import jdk.vm.ci.code.VirtualObject;
@@ -102,7 +101,7 @@ protected void verifyFrameState(NodeWithState node, FrameState topState) {
 
         FrameState current = topState;
         while (current != null) {
-            assert current.getMethod() instanceof HotSpotResolvedJavaMethod : current;
+            GraalError.guarantee(current.getMethod() instanceof HotSpotResolvedJavaMethod, "unsupported method type %s", current.getMethod());
             current = current.outerFrameState();
         }
 
@@ -112,7 +111,7 @@ protected void verifyFrameState(NodeWithState node, FrameState topState) {
             if (DefaultHotSpotLoweringProvider.RuntimeCalls.runtimeCalls.containsValue(descriptor.getSignature())) {
                 // This case is special because we can't use rethrowException. This path must be
                 // marked as reexecutable so it doesn't fail some internal asserts even though the
-                // actual deopt is done with Unpack_exception which overrides the reexectuable flag
+                // actual deopt is done with Unpack_exception which overrides the reexecutable flag
                 GraalError.guarantee(topState.getStackState() == FrameState.StackState.BeforePop && topState.bci >= 0 && topState.stackSize() == 0, "invalid state %s for bytecode exception %s",
                                 topState, descriptor.getSignature());
                 GraalError.guarantee(!topState.getStackState().duringCall, "must be not duringCall to set reexecute bit");

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/replacements/MonitorSnippets.java

@@ -850,6 +850,8 @@ public static class Templates extends AbstractTemplates {
 
         public final Counters counters;
 
+        private final boolean requiresStrictLockOrder;
+
         @SuppressWarnings("this-escape")
         public Templates(OptionValues options, SnippetCounter.Group.Factory factory, HotSpotProviders providers, GraalHotSpotVMConfig config) {
             super(options, providers);
@@ -899,6 +901,7 @@ public Templates(OptionValues options, SnippetCounter.Group.Factory factory, Hot
             this.checkCounter = snippet(providers, MonitorSnippets.class, "checkCounter");
 
             this.counters = new Counters(factory);
+            this.requiresStrictLockOrder = providers.getPlatformConfigurationProvider().requiresStrictLockOrder();
         }
 
         public void lower(CheckFastPathMonitorEnterNode checkFastPathMonitorEnterNode, HotSpotRegistersProvider registers, GraalHotSpotVMConfig config, LoweringTool tool) {
@@ -916,11 +919,31 @@ public void lower(CheckFastPathMonitorEnterNode checkFastPathMonitorEnterNode, H
             template(tool, checkFastPathMonitorEnterNode, args).instantiate(tool.getMetaAccess(), checkFastPathMonitorEnterNode, DEFAULT_REPLACER, args);
         }
 
+        private boolean verifyLockOrder(MonitorEnterNode monitorenterNode) {
+            if (requiresStrictLockOrder) {
+                FrameState state = monitorenterNode.stateAfter();
+                boolean subsequentLocksMustBeEliminated = false;
+                for (int lockIdx = 0; lockIdx < state.locksSize(); lockIdx++) {
+                    if (subsequentLocksMustBeEliminated) {
+                        if (!state.monitorIdAt(lockIdx).isEliminated()) {
+                            return false;
+                        }
+                    }
+                    if (state.monitorIdAt(lockIdx) == monitorenterNode.getMonitorId()) {
+                        subsequentLocksMustBeEliminated = true;
+                    }
+                }
+            }
+            return true;
+        }
+
         public void lower(MonitorEnterNode monitorenterNode, HotSpotRegistersProvider registers, LoweringTool tool) {
             StructuredGraph graph = monitorenterNode.graph();
             checkBalancedMonitors(graph, tool);
 
-            assert ((ObjectStamp) monitorenterNode.object().stamp(NodeView.DEFAULT)).nonNull();
+            GraalError.guarantee(((ObjectStamp) monitorenterNode.object().stamp(NodeView.DEFAULT)).nonNull(), "should have a non-null stamp: %s", monitorenterNode);
+            GraalError.guarantee(!monitorenterNode.getMonitorId().isEliminated(), "current monitor is eliminated: %s", monitorenterNode);
+            GraalError.guarantee(verifyLockOrder(monitorenterNode), "locks are disordered: %s", monitorenterNode);
 
             Arguments args = new Arguments(monitorenter, graph.getGuardsStage(), tool.getLoweringStage());
             args.add("object", monitorenterNode.object());

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/MonitorEnterNode.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2025, 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

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/VirtualizerTool.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2025, 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

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/LockEliminationPhase.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2025, 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

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/DefaultJavaLoweringProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2025, 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
@@ -122,6 +122,7 @@
 import jdk.graal.compiler.nodes.gc.BarrierSet;
 import jdk.graal.compiler.nodes.java.AbstractNewObjectNode;
 import jdk.graal.compiler.nodes.java.AccessIndexedNode;
+import jdk.graal.compiler.nodes.java.AccessMonitorNode;
 import jdk.graal.compiler.nodes.java.ArrayLengthNode;
 import jdk.graal.compiler.nodes.java.AtomicReadAndAddNode;
 import jdk.graal.compiler.nodes.java.AtomicReadAndWriteNode;
@@ -133,7 +134,6 @@
 import jdk.graal.compiler.nodes.java.LoweredAtomicReadAndAddNode;
 import jdk.graal.compiler.nodes.java.LoweredAtomicReadAndWriteNode;
 import jdk.graal.compiler.nodes.java.MonitorEnterNode;
-import jdk.graal.compiler.nodes.java.MonitorExitNode;
 import jdk.graal.compiler.nodes.java.MonitorIdNode;
 import jdk.graal.compiler.nodes.java.NewArrayNode;
 import jdk.graal.compiler.nodes.java.NewInstanceNode;
@@ -1169,8 +1169,8 @@ public void finishAllocatedObjects(LoweringTool tool, FixedWithNextNode insertAf
                 if (!newList.contains(lock)) {
                     // lock is nested and eliminated
                     for (Node usage : lock.usages().snapshot()) {
-                        if (usage.isAlive() && usage instanceof MonitorExitNode exit) {
-                            removeMonitorAccess(exit);
+                        if (usage.isAlive() && usage instanceof AccessMonitorNode access) {
+                            removeMonitorAccess(access);
                         }
                     }
                     lock.setEliminated();
@@ -1184,6 +1184,7 @@ public void finishAllocatedObjects(LoweringTool tool, FixedWithNextNode insertAf
         int lastDepth = -1;
         for (MonitorIdNode monitorId : locks) {
             GraalError.guarantee(lastDepth < monitorId.getLockDepth(), Assertions.errorMessage(lastDepth, monitorId, insertAfter, commit, allocations));
+            GraalError.guarantee(!monitorId.isEliminated(), Assertions.errorMessage(lastDepth, monitorId, insertAfter, commit, allocations));
             lastDepth = monitorId.getLockDepth();
             MonitorEnterNode enter = graph.add(new MonitorEnterNode(allocations[commit.getObjectIndex(monitorId)], monitorId));
             enter.setSynthetic();

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/virtual/phases/ea/PartialEscapeBlockState.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2025, 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

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/virtual/phases/ea/PartialEscapeClosure.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2025, 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

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/virtual/phases/ea/VirtualizerToolImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2025, 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