GR-73325: Preserve bytecode handler slot sentinels

Author: mur47x111

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/lir/LIR.java

@@ -292,16 +292,10 @@ public static LabelRef getExceptionEdge(LIRInstruction op) {
     public static final int MAX_EXCEPTION_EDGE_OP_DISTANCE_FROM_END = 3;
 
     /**
-     * Multi-result operations may be followed by moves that materialize additional defined values
-     * before the block end. Those moves are still part of the operation's normal successor value
-     * setup, so the verifier allows them to extend the usual exception-edge distance.
+     * Custom calling conventions may materialize additional return values with moves after the
+     * call. Those moves are still part of the operation's normal successor value setup, so the
+     * verifier allows them to extend the usual exception-edge distance.
      */
-    private static boolean hasMultipleDefs(LIRInstruction op) {
-        int[] defCount = {0};
-        op.visitEachOutput((instruction, value, mode, flags) -> defCount[0]++);
-        return defCount[0] > 1;
-    }
-
     private static boolean hasOnlyTrailingMoves(ArrayList<LIRInstruction> ops, int exceptionEdgeIndex, int lastIndex) {
         for (int i = exceptionEdgeIndex + 1; i < lastIndex; i++) {
             if (!ops.get(i).isMoveOp()) {
@@ -327,8 +321,7 @@ public static boolean verifyBlock(LIR lir, BasicBlock<?> block) {
                 assert opWithExceptionEdge == null : "multiple ops with an exception edge not allowed";
                 opWithExceptionEdge = op;
                 int distanceFromEnd = lastIndex - index;
-                assert distanceFromEnd <= MAX_EXCEPTION_EDGE_OP_DISTANCE_FROM_END ||
-                                (hasMultipleDefs(op) && hasOnlyTrailingMoves(ops, index, lastIndex)) : distanceFromEnd;
+                assert distanceFromEnd <= MAX_EXCEPTION_EDGE_OP_DISTANCE_FROM_END || hasOnlyTrailingMoves(ops, index, lastIndex) : distanceFromEnd;
             }
             index++;
         }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/code/PendingExceptionStateSupport.java

@@ -31,6 +31,7 @@
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 
+import com.oracle.svm.core.NeverInline;
 import com.oracle.svm.shared.BuildPhaseProvider.ReadyForCompilation;
 import com.oracle.svm.core.heap.UnknownPrimitiveField;
 import com.oracle.svm.core.thread.ThreadListener;
@@ -90,10 +91,10 @@ public void setMaxSlots(int newMaxSlots) {
     }
 
     /**
-     * Enables diagnostic poisoning of pending-state slots. Consumed object slots are always cleared
-     * to {@code null} for GC. When disabled, consumed primitive slots are left unchanged. When
-     * enabled, initially empty slots and consumed primitive slots contain recognizable sentinel
-     * values to expose stale or duplicate reads.
+     * Enables diagnostic poisoning of pending-state slots. When disabled, consumed object slots are
+     * cleared to {@code null} for GC and consumed primitive slots are left unchanged. When enabled,
+     * initially empty and consumed slots contain recognizable sentinel values to expose stale or
+     * duplicate reads.
      */
     @Platforms(Platform.HOSTED_ONLY.class)
     public void setUseSlotSentinel(boolean useSlotSentinel) {
@@ -134,6 +135,11 @@ static void initializeSlotSentinels(PendingExceptionStateHolder holder) {
         Arrays.fill(holder.primitiveSlots, PRIMITIVE_SLOT_SENTINEL);
     }
 
+    @NeverInline("Keep object sentinel materialization out of generated bytecode-handler unwind graphs")
+    public static void poisonObjectSlot(Object[] objectSlots, int slotIndex) {
+        objectSlots[slotIndex] = OBJECT_SLOT_SENTINEL;
+    }
+
     private static final class ObjectSlotSentinel {
     }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/BytecodeHandlerFeature.java

@@ -49,6 +49,7 @@
 import com.oracle.svm.shared.singletons.traits.BuiltinTraits.DisallowLayered;
 import com.oracle.svm.shared.singletons.traits.BuiltinTraits.NoLayeredCallbacks;
 import com.oracle.svm.shared.singletons.traits.SingletonTraits;
+import com.oracle.svm.shared.util.ReflectionUtil;
 import com.oracle.svm.shared.util.SubstrateUtil;
 import com.oracle.svm.util.OriginalClassProvider;
 
@@ -126,6 +127,10 @@ public void afterRegistration(AfterRegistrationAccess access) {
     @Override
     public void beforeAnalysis(BeforeAnalysisAccess access) {
         BeforeAnalysisAccessImpl accessImpl = (BeforeAnalysisAccessImpl) access;
+        if (Options.BytecodeHandlerSlotSentinel.getValue()) {
+            accessImpl.registerAsRoot(ReflectionUtil.lookupMethod(PendingExceptionStateSupport.class, "poisonObjectSlot", Object[].class, int.class), false,
+                            "Object-slot sentinel support, registered in " + BytecodeHandlerFeature.class);
+        }
         BytecodeInterpreterAnnotations.registerCompilerDirectives(accessImpl.getMetaAccess(), OriginalClassProvider::getOriginalType);
     }
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SubstrateBytecodeHandlerUnwindPath.java

@@ -25,6 +25,7 @@
 package com.oracle.svm.hosted;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.util.List;
 import java.util.function.Function;
 
@@ -34,22 +35,27 @@
 import com.oracle.svm.core.graal.code.PendingExceptionStateSupport;
 import com.oracle.svm.core.graal.nodes.ReadReservedRegisterFloatingNode;
 import com.oracle.svm.core.graal.thread.LoadVMThreadLocalNode;
+import com.oracle.svm.core.nodes.SubstrateMethodCallTargetNode;
 import com.oracle.svm.core.threadlocal.VMThreadLocalInfo;
 import com.oracle.svm.shared.util.ReflectionUtil;
 
 import jdk.graal.compiler.core.common.memory.BarrierType;
 import jdk.graal.compiler.core.common.memory.MemoryOrderMode;
 import jdk.graal.compiler.core.common.type.ObjectStamp;
+import jdk.graal.compiler.core.common.type.StampFactory;
+import jdk.graal.compiler.core.common.type.StampPair;
 import jdk.graal.compiler.debug.GraalError;
 import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.nodeinfo.InputType;
 import jdk.graal.compiler.nodes.AbstractMergeNode;
+import jdk.graal.compiler.nodes.CallTargetNode.InvokeKind;
 import jdk.graal.compiler.nodes.ConstantNode;
 import jdk.graal.compiler.nodes.EndNode;
 import jdk.graal.compiler.nodes.FixedNode;
 import jdk.graal.compiler.nodes.FixedWithNextNode;
 import jdk.graal.compiler.nodes.FrameState;
 import jdk.graal.compiler.nodes.InvokeWithExceptionNode;
+import jdk.graal.compiler.nodes.InvokeNode;
 import jdk.graal.compiler.nodes.NamedLocationIdentity;
 import jdk.graal.compiler.nodes.NodeView;
 import jdk.graal.compiler.nodes.StructuredGraph;
@@ -130,6 +136,7 @@ final class SubstrateBytecodeHandlerUnwindPath {
 
     private static final Field OBJECT_SLOTS_FIELD = ReflectionUtil.lookupField(PendingExceptionStateHolder.class, "objectSlots");
     private static final Field PRIMITIVE_SLOTS_FIELD = ReflectionUtil.lookupField(PendingExceptionStateHolder.class, "primitiveSlots");
+    private static final Method POISON_OBJECT_SLOT_METHOD = ReflectionUtil.lookupMethod(PendingExceptionStateSupport.class, "poisonObjectSlot", Object[].class, int.class);
 
     private record PendingStateRead(ValueNode value, FixedWithNextNode last) {
     }
@@ -547,14 +554,11 @@ private static PendingStateRead readObjectPendingStateSlot(MetaAccessProvider me
                         true));
         graph.addAfterFixed(insertAfter, read);
         /*
-         * Object pending-state slots are thread-local roots. Clear consumed references so they do not
-         * keep object graphs live until a later exception overwrites this slot.
+         * Object pending-state slots are thread-local roots. Replace consumed references with the
+         * debug sentinel or null so they do not keep object graphs live until a later exception
+         * overwrites this slot.
          */
-        JavaWriteNode clear = graph.add(createClearObjectSlotWrite(graph, slotAddress));
-        graph.addAfterFixed(read, clear);
-        if (stateAfter != null) {
-            clear.setStateAfter(stateAfter);
-        }
+        FixedWithNextNode clear = appendClearObjectSlotWrite(metaAccess, graph, read, objectSlots, slotIndex, slotAddress, stateAfter);
         return new PendingStateRead(read, clear);
     }
 
@@ -608,19 +612,40 @@ private static void clearObjectPendingStateSlot(MetaAccessProvider metaAccess, F
         long objectArrayBaseOffset = metaAccess.getArrayBaseOffset(JavaKind.Object);
         int objectArrayIndexScale = metaAccess.getArrayIndexScale(JavaKind.Object);
         AddressNode slotAddress = elementAddress(graph, objectSlots, objectArrayBaseOffset + (long) slotIndex * objectArrayIndexScale);
-        JavaWriteNode clear = graph.add(createClearObjectSlotWrite(graph, slotAddress));
-        graph.addAfterFixed(last, clear);
+        appendClearObjectSlotWrite(metaAccess, graph, last, objectSlots, slotIndex, slotAddress, null);
     }
 
-    private static JavaWriteNode createClearObjectSlotWrite(StructuredGraph graph, AddressNode slotAddress) {
-        return new JavaWriteNode(JavaKind.Object,
+    private static FixedWithNextNode appendClearObjectSlotWrite(MetaAccessProvider metaAccess, StructuredGraph graph, FixedWithNextNode insertAfter,
+                    ValueNode objectSlots, int slotIndex, AddressNode slotAddress, FrameState stateAfter) {
+        if (useSlotDebugSentinel()) {
+            ResolvedJavaMethod poisonMethod = metaAccess.lookupJavaMethod(POISON_OBJECT_SLOT_METHOD);
+            SubstrateMethodCallTargetNode callTarget = graph.add(new SubstrateMethodCallTargetNode(InvokeKind.Static, poisonMethod,
+                            new ValueNode[]{objectSlots, ConstantNode.forInt(slotIndex, graph)}, StampPair.createSingle(StampFactory.forVoid())));
+            FrameState callState = stateAfter;
+            if (callState == null) {
+                FrameState lastFrameState = GraphUtil.findLastFrameState(insertAfter);
+                callState = lastFrameState == null ? null : lastFrameState.duplicateWithVirtualState();
+            }
+            GraalError.guarantee(callState != null, "Missing frame state for object-slot sentinel write");
+            InvokeNode poison = graph.add(new InvokeNode(callTarget, callState.bci));
+            graph.addAfterFixed(insertAfter, poison);
+            poison.setStateAfter(callState);
+            poison.setStateDuring(callState.duplicateWithVirtualState());
+            return poison;
+        }
+        JavaWriteNode clear = graph.add(new JavaWriteNode(JavaKind.Object,
                         slotAddress,
                         NamedLocationIdentity.getArrayLocation(JavaKind.Object),
                         ConstantNode.defaultForKind(JavaKind.Object, graph),
                         BarrierType.ARRAY,
                         true,
                         true,
-                        MemoryOrderMode.PLAIN);
+                        MemoryOrderMode.PLAIN));
+        graph.addAfterFixed(insertAfter, clear);
+        if (stateAfter != null) {
+            clear.setStateAfter(stateAfter);
+        }
+        return clear;
     }
 
     private static FixedWithNextNode appendPrimitiveSlotDebugWrite(StructuredGraph graph, FixedWithNextNode insertAfter,