[GR-51805] Verify init memory semantics

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/InfopointReasonTest.java

@@ -27,6 +27,8 @@
 import static jdk.graal.compiler.core.common.GraalOptions.OptAssumptions;
 import static org.junit.Assert.assertNotNull;
 
+import org.junit.Test;
+
 import jdk.graal.compiler.code.CompilationResult;
 import jdk.graal.compiler.core.GraalCompiler;
 import jdk.graal.compiler.core.target.Backend;
@@ -41,12 +43,10 @@
 import jdk.graal.compiler.phases.tiers.HighTierContext;
 import jdk.graal.compiler.phases.tiers.Suites;
 import jdk.graal.compiler.phases.util.Providers;
-import jdk.vm.ci.meta.ProfilingInfo;
-import org.junit.Test;
-
 import jdk.vm.ci.code.site.Call;
 import jdk.vm.ci.code.site.Infopoint;
 import jdk.vm.ci.code.site.InfopointReason;
+import jdk.vm.ci.meta.ProfilingInfo;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 
 /**

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/InitMemoryVerificationTest.java

@@ -0,0 +1,136 @@
+/*
+ * 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+package jdk.graal.compiler.core.test;
+
+import java.util.Optional;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import jdk.graal.compiler.debug.DebugContext;
+import jdk.graal.compiler.debug.GraalError;
+import jdk.graal.compiler.nodes.GraphState;
+import jdk.graal.compiler.nodes.StructuredGraph;
+import jdk.graal.compiler.nodes.StructuredGraph.AllowAssumptions;
+import jdk.graal.compiler.nodes.extended.MembarNode;
+import jdk.graal.compiler.nodes.extended.PublishWritesNode;
+import jdk.graal.compiler.nodes.util.GraphUtil;
+import jdk.graal.compiler.phases.BasePhase;
+import jdk.graal.compiler.phases.VerifyPhase;
+import jdk.graal.compiler.phases.common.InitMemoryVerificationPhase;
+import jdk.graal.compiler.phases.tiers.MidTierContext;
+import jdk.graal.compiler.phases.tiers.Suites;
+
+/**
+ * Tests that {@link InitMemoryVerificationPhase} actually verifies the properties it aims to.
+ */
+public class InitMemoryVerificationTest extends GraalCompilerTest {
+
+    @SuppressWarnings("unused")
+    private static class TestClass {
+        private final int field;
+
+        TestClass(int field) {
+            this.field = field;
+        }
+    }
+
+    public static Object allocate(int val) {
+        return new TestClass(val + 2);
+    }
+
+    @SuppressWarnings("try")
+    private StructuredGraph getMidTierGraph(String snippet, Suites suites) {
+        StructuredGraph g = parseEager(snippet, AllowAssumptions.NO, getInitialOptions());
+        suites.getHighTier().apply(g, getDefaultHighTierContext());
+        suites.getMidTier().apply(g, getDefaultMidTierContext());
+        return g;
+    }
+
+    @Test
+    public void testVerificationPasses() {
+        StructuredGraph g = getMidTierGraph("allocate", createSuites(getInitialOptions()));
+        assertInGraph(g, PublishWritesNode.class, MembarNode.class);
+        new InitMemoryVerificationPhase().apply(g, getDefaultLowTierContext());
+    }
+
+    @Test
+    @SuppressWarnings("try")
+    public void testVerificationFailsWithoutPublish() {
+        Suites s = createSuites(getInitialOptions());
+        s.getMidTier().appendPhase(new BasePhase<>() {
+
+            @Override
+            public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
+                return ALWAYS_APPLICABLE;
+            }
+
+            @Override
+            protected void run(StructuredGraph graph, MidTierContext context) {
+                for (PublishWritesNode publish : graph.getNodes().filter(PublishWritesNode.class)) {
+                    publish.replaceAtUsages(publish.allocation());
+                    GraphUtil.removeFixedWithUnusedInputs(publish);
+                }
+            }
+        });
+
+        StructuredGraph g = getMidTierGraph("allocate", s);
+        assertNotInGraph(g, PublishWritesNode.class);
+        try (DebugContext.Scope scope = getDebugContext().disable()) {
+            new InitMemoryVerificationPhase().apply(g, getDefaultLowTierContext());
+            throw new GraalError("Should fail init memory verification");
+        } catch (VerifyPhase.VerificationError e) {
+            Assert.assertTrue(e.getMessage().contains("unpublished allocations"));
+        }
+    }
+
+    @Test
+    @SuppressWarnings("try")
+    public void testVerificationFailsWithoutMembar() {
+        Suites s = createSuites(getInitialOptions());
+        s.getMidTier().appendPhase(new BasePhase<>() {
+            @Override
+            public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
+                return ALWAYS_APPLICABLE;
+            }
+
+            @Override
+            protected void run(StructuredGraph graph, MidTierContext context) {
+                for (MembarNode memBar : graph.getNodes().filter(MembarNode.class)) {
+                    GraphUtil.removeFixedWithUnusedInputs(memBar);
+                }
+            }
+        });
+
+        StructuredGraph g = getMidTierGraph("allocate", s);
+        assertNotInGraph(g, GraphUtil.class);
+        try (DebugContext.Scope scope = getDebugContext().disable()) {
+            new InitMemoryVerificationPhase().apply(g, getDefaultLowTierContext());
+            throw new GraalError("Should fail init memory verification");
+        } catch (VerifyPhase.VerificationError e) {
+            Assert.assertTrue(e.getMessage().contains("writes to init memory not guarded by an init barrier"));
+        }
+    }
+}

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/phases/EconomyCompilerConfiguration.java

@@ -39,7 +39,6 @@
 import jdk.graal.compiler.phases.tiers.HighTierContext;
 import jdk.graal.compiler.phases.tiers.LowTierContext;
 import jdk.graal.compiler.phases.tiers.MidTierContext;
-
 import jdk.vm.ci.code.Architecture;
 
 /**
@@ -60,7 +59,7 @@ public PhaseSuite<MidTierContext> createMidTier(OptionValues options) {
 
     @Override
     public PhaseSuite<LowTierContext> createLowTier(OptionValues options, Architecture arch) {
-        return new EconomyLowTier();
+        return new EconomyLowTier(options);
     }
 
     @Override

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/phases/EconomyLowTier.java

@@ -25,11 +25,14 @@
 package jdk.graal.compiler.core.phases;
 
 import jdk.graal.compiler.debug.Assertions;
+import jdk.graal.compiler.graph.Graph;
+import jdk.graal.compiler.options.OptionValues;
 import jdk.graal.compiler.phases.PlaceholderPhase;
 import jdk.graal.compiler.phases.common.AddressLoweringPhase;
 import jdk.graal.compiler.phases.common.BarrierSetVerificationPhase;
 import jdk.graal.compiler.phases.common.CanonicalizerPhase;
 import jdk.graal.compiler.phases.common.ExpandLogicPhase;
+import jdk.graal.compiler.phases.common.InitMemoryVerificationPhase;
 import jdk.graal.compiler.phases.common.LowTierLoweringPhase;
 import jdk.graal.compiler.phases.common.RemoveOpaqueValuePhase;
 import jdk.graal.compiler.phases.common.TransplantGraphsPhase;
@@ -39,7 +42,10 @@
 public class EconomyLowTier extends BaseTier<LowTierContext> {
 
     @SuppressWarnings("this-escape")
-    public EconomyLowTier() {
+    public EconomyLowTier(OptionValues options) {
+        if (Graph.Options.VerifyGraalGraphs.getValue(options)) {
+            appendPhase(new InitMemoryVerificationPhase());
+        }
         CanonicalizerPhase canonicalizer = CanonicalizerPhase.create();
         appendPhase(new LowTierLoweringPhase(canonicalizer));
         appendPhase(new ExpandLogicPhase(canonicalizer));

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/phases/LowTier.java

@@ -27,6 +27,7 @@
 import static jdk.graal.compiler.phases.common.DeadCodeEliminationPhase.Optionality.Required;
 
 import jdk.graal.compiler.core.common.GraalOptions;
+import jdk.graal.compiler.graph.Graph;
 import jdk.graal.compiler.options.Option;
 import jdk.graal.compiler.options.OptionKey;
 import jdk.graal.compiler.options.OptionType;
@@ -38,6 +39,7 @@
 import jdk.graal.compiler.phases.common.ExpandLogicPhase;
 import jdk.graal.compiler.phases.common.FinalCanonicalizerPhase;
 import jdk.graal.compiler.phases.common.FixReadsPhase;
+import jdk.graal.compiler.phases.common.InitMemoryVerificationPhase;
 import jdk.graal.compiler.phases.common.LowTierLoweringPhase;
 import jdk.graal.compiler.phases.common.OptimizeExtendsPhase;
 import jdk.graal.compiler.phases.common.OptimizeOffsetAddressPhase;
@@ -72,6 +74,10 @@ public LowTier(OptionValues options) {
             appendPhase(new ProfileCompiledMethodsPhase());
         }
 
+        if (Graph.Options.VerifyGraalGraphs.getValue(options)) {
+            appendPhase(new InitMemoryVerificationPhase());
+        }
+
         appendPhase(new LowTierLoweringPhase(canonicalizerWithGVN));
 
         appendPhase(new ExpandLogicPhase(canonicalizerWithGVN));

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hightiercodegen/NodeLowerer.java

@@ -38,12 +38,10 @@
 import jdk.graal.compiler.nodes.LogicNegationNode;
 import jdk.graal.compiler.nodes.ParameterNode;
 import jdk.graal.compiler.nodes.PhiNode;
-import jdk.graal.compiler.nodes.PiNode;
 import jdk.graal.compiler.nodes.ReturnNode;
 import jdk.graal.compiler.nodes.ShortCircuitOrNode;
 import jdk.graal.compiler.nodes.UnwindNode;
 import jdk.graal.compiler.nodes.ValueNode;
-import jdk.graal.compiler.nodes.ValueProxyNode;
 import jdk.graal.compiler.nodes.calc.AbsNode;
 import jdk.graal.compiler.nodes.calc.BinaryArithmeticNode;
 import jdk.graal.compiler.nodes.calc.CompareNode;
@@ -66,7 +64,6 @@
 import jdk.graal.compiler.nodes.debug.BlackholeNode;
 import jdk.graal.compiler.nodes.extended.BoxNode;
 import jdk.graal.compiler.nodes.extended.BytecodeExceptionNode;
-import jdk.graal.compiler.nodes.extended.FixedValueAnchorNode;
 import jdk.graal.compiler.nodes.extended.ForeignCall;
 import jdk.graal.compiler.nodes.extended.GetClassNode;
 import jdk.graal.compiler.nodes.extended.JavaReadNode;
@@ -99,6 +96,7 @@
 import jdk.graal.compiler.nodes.java.StoreFieldNode;
 import jdk.graal.compiler.nodes.java.StoreIndexedNode;
 import jdk.graal.compiler.nodes.memory.ReadNode;
+import jdk.graal.compiler.nodes.spi.ValueProxy;
 import jdk.graal.compiler.replacements.nodes.ArrayEqualsNode;
 import jdk.graal.compiler.replacements.nodes.BasicArrayCopyNode;
 import jdk.graal.compiler.replacements.nodes.BinaryMathIntrinsicNode;
@@ -194,8 +192,6 @@ protected void dispatch(Node node) {
             lower((IsNullNode) node);
         } else if (node instanceof ExceptionObjectNode) {
             lower((ExceptionObjectNode) node);
-        } else if (node instanceof ValueProxyNode) {
-            lowerValue(((ValueProxyNode) node).value());
         } else if (node instanceof ConstantNode) {
             lower((ConstantNode) node);
         } else if (node instanceof LoadIndexedNode) {
@@ -301,8 +297,6 @@ protected void dispatch(Node node) {
             lower((BinaryArithmeticNode<?>) node);
         } else if (node instanceof GetClassNode) {
             lower((GetClassNode) node);
-        } else if (node instanceof PiNode) {
-            lowerValue(((PiNode) node).getOriginalNode());
         } else if (node instanceof UnaryMathIntrinsicNode) {
             lower((UnaryMathIntrinsicNode) node);
         } else if (node instanceof BinaryMathIntrinsicNode) {
@@ -327,8 +321,8 @@ protected void dispatch(Node node) {
             lower((ClassIsAssignableFromNode) node);
         } else if (node instanceof DynamicNewInstanceNode n) {
             lower(n);
-        } else if (node instanceof FixedValueAnchorNode n) {
-            lowerValue(n.getOriginalNode());
+        } else if (node instanceof ValueProxy proxy) {
+            lowerValue(proxy.getOriginalNode());
         } else {
             if (!isIgnored(node)) {
                 handleUnknownNodeType(node);

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java

@@ -118,12 +118,13 @@
 import jdk.graal.compiler.nodes.calc.UnsignedRightShiftNode;
 import jdk.graal.compiler.nodes.calc.XorNode;
 import jdk.graal.compiler.nodes.extended.BranchProbabilityNode;
-import jdk.graal.compiler.nodes.extended.FixedValueAnchorNode;
 import jdk.graal.compiler.nodes.extended.ForeignCallNode;
 import jdk.graal.compiler.nodes.extended.JavaReadNode;
 import jdk.graal.compiler.nodes.extended.JavaWriteNode;
 import jdk.graal.compiler.nodes.extended.LoadHubNode;
+import jdk.graal.compiler.nodes.extended.MembarNode;
 import jdk.graal.compiler.nodes.extended.ObjectIsArrayNode;
+import jdk.graal.compiler.nodes.extended.PublishWritesNode;
 import jdk.graal.compiler.nodes.gc.BarrierSet;
 import jdk.graal.compiler.nodes.graphbuilderconf.ForeignCallPlugin;
 import jdk.graal.compiler.nodes.graphbuilderconf.GeneratedPluginFactory;
@@ -606,10 +607,11 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
                     b.add(new ArrayCopyCallNode(foreignCalls, wordTypes, value, srcBegin, newArray, ConstantNode.forInt(0), length, JavaKind.Char, LocationIdentity.init(), false, true, true,
                                     vmConfig.heapWordSize));
 
-                    // Writes to init must be protected by a FixedValueAnchorNode so that floating
+                    // Writes to init must be protected by a PublishWritesNode so that floating
                     // reads don't bypass the ArrayCopyCallNode above.
                     b.pop(JavaKind.Object);
-                    b.addPush(JavaKind.Object, new FixedValueAnchorNode(newArray));
+                    b.addPush(JavaKind.Object, new PublishWritesNode(newArray));
+                    b.add(MembarNode.forInitialization());
                 }
                 return true;
             }

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

@@ -33,8 +33,8 @@
 import jdk.graal.compiler.api.directives.GraalDirectives;
 import jdk.graal.compiler.api.replacements.Snippet;
 import jdk.graal.compiler.hotspot.meta.HotSpotProviders;
-import jdk.graal.compiler.nodes.extended.FixedValueAnchorNode;
 import jdk.graal.compiler.nodes.extended.MembarNode;
+import jdk.graal.compiler.nodes.extended.PublishWritesNode;
 import jdk.graal.compiler.nodes.java.DynamicNewArrayNode;
 import jdk.graal.compiler.nodes.java.NewArrayNode;
 import jdk.graal.compiler.options.OptionValues;
@@ -70,63 +70,63 @@ public static boolean[] booleanArrayClone(boolean[] src) {
         boolean[] result = (boolean[]) NewArrayNode.newUninitializedArray(Boolean.TYPE, src.length);
         ArrayCopyCallNode.disjointArraycopy(src, 0, result, 0, src.length, JavaKind.Boolean, LocationIdentity.init(), HotSpotReplacementsUtil.getHeapWordSize(INJECTED_VMCONFIG));
         MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
-        return (boolean[]) FixedValueAnchorNode.getObject(result);
+        return (boolean[]) PublishWritesNode.publishWrites(result);
     }
 
     @Snippet
     public static byte[] byteArrayClone(byte[] src) {
         byte[] result = (byte[]) NewArrayNode.newUninitializedArray(Byte.TYPE, src.length);
         ArrayCopyCallNode.disjointArraycopy(src, 0, result, 0, src.length, JavaKind.Byte, LocationIdentity.init(), HotSpotReplacementsUtil.getHeapWordSize(INJECTED_VMCONFIG));
         MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
-        return (byte[]) FixedValueAnchorNode.getObject(result);
+        return (byte[]) PublishWritesNode.publishWrites(result);
     }
 
     @Snippet
     public static short[] shortArrayClone(short[] src) {
         short[] result = (short[]) NewArrayNode.newUninitializedArray(Short.TYPE, src.length);
         ArrayCopyCallNode.disjointArraycopy(src, 0, result, 0, src.length, JavaKind.Short, LocationIdentity.init(), HotSpotReplacementsUtil.getHeapWordSize(INJECTED_VMCONFIG));
         MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
-        return (short[]) FixedValueAnchorNode.getObject(result);
+        return (short[]) PublishWritesNode.publishWrites(result);
     }
 
     @Snippet
     public static char[] charArrayClone(char[] src) {
         char[] result = (char[]) NewArrayNode.newUninitializedArray(Character.TYPE, src.length);
         ArrayCopyCallNode.disjointArraycopy(src, 0, result, 0, src.length, JavaKind.Char, LocationIdentity.init(), HotSpotReplacementsUtil.getHeapWordSize(INJECTED_VMCONFIG));
         MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
-        return (char[]) FixedValueAnchorNode.getObject(result);
+        return (char[]) PublishWritesNode.publishWrites(result);
     }
 
     @Snippet
     public static int[] intArrayClone(int[] src) {
         int[] result = (int[]) NewArrayNode.newUninitializedArray(Integer.TYPE, src.length);
         ArrayCopyCallNode.disjointArraycopy(src, 0, result, 0, src.length, JavaKind.Int, LocationIdentity.init(), HotSpotReplacementsUtil.getHeapWordSize(INJECTED_VMCONFIG));
         MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
-        return (int[]) FixedValueAnchorNode.getObject(result);
+        return (int[]) PublishWritesNode.publishWrites(result);
     }
 
     @Snippet
     public static float[] floatArrayClone(float[] src) {
         float[] result = (float[]) NewArrayNode.newUninitializedArray(Float.TYPE, src.length);
         ArrayCopyCallNode.disjointArraycopy(src, 0, result, 0, src.length, JavaKind.Float, LocationIdentity.init(), HotSpotReplacementsUtil.getHeapWordSize(INJECTED_VMCONFIG));
         MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
-        return (float[]) FixedValueAnchorNode.getObject(result);
+        return (float[]) PublishWritesNode.publishWrites(result);
     }
 
     @Snippet
     public static long[] longArrayClone(long[] src) {
         long[] result = (long[]) NewArrayNode.newUninitializedArray(Long.TYPE, src.length);
         ArrayCopyCallNode.disjointArraycopy(src, 0, result, 0, src.length, JavaKind.Long, LocationIdentity.init(), HotSpotReplacementsUtil.getHeapWordSize(INJECTED_VMCONFIG));
         MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
-        return (long[]) FixedValueAnchorNode.getObject(result);
+        return (long[]) PublishWritesNode.publishWrites(result);
     }
 
     @Snippet
     public static double[] doubleArrayClone(double[] src) {
         double[] result = (double[]) NewArrayNode.newUninitializedArray(Double.TYPE, src.length);
         ArrayCopyCallNode.disjointArraycopy(src, 0, result, 0, src.length, JavaKind.Double, LocationIdentity.init(), HotSpotReplacementsUtil.getHeapWordSize(INJECTED_VMCONFIG));
         MembarNode.memoryBarrier(MembarNode.FenceKind.ALLOCATION_INIT, LocationIdentity.INIT_LOCATION);
-        return (double[]) FixedValueAnchorNode.getObject(result);
+        return (double[]) PublishWritesNode.publishWrites(result);
     }
 
     @Snippet