From fb3b5f597e78a4288158088b211948ca5edcd61b Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Tue, 27 Feb 2024 12:15:12 +0100 Subject: [PATCH 1/3] Remove use of old method handle intrinsification. --- .../graal/compiler/nodes/GraphDecoder.java | 8 +-- .../graal/compiler/nodes/StructuredGraph.java | 42 ++++++++++- .../nodes/extended/UnsafeAccessNode.java | 3 +- .../nodes/extended/UnsafeMemoryLoadNode.java | 7 +- .../nodes/extended/UnsafeMemoryStoreNode.java | 7 +- .../AbstractUnsafeCompareAndSwapNode.java | 13 ++-- .../nodes/java/AtomicReadAndAddNode.java | 7 +- .../nodes/java/AtomicReadAndWriteNode.java | 7 +- .../nodes/spi/TrackedUnsafeAccess.java | 34 +++++++++ .../phases/common/inlining/InliningUtil.java | 4 +- .../StandardGraphBuilderPlugins.java | 20 +++--- .../svm/truffle/tck/PermissionsFeature.java | 72 ++++++++++++++----- vm/mx.vm/mx_vm_gate.py | 3 +- 13 files changed, 170 insertions(+), 57 deletions(-) create mode 100644 compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/TrackedUnsafeAccess.java diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/GraphDecoder.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/GraphDecoder.java index ae931344a3a2..b3244ea63723 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/GraphDecoder.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/GraphDecoder.java @@ -591,9 +591,7 @@ public final void decode(EncodedGraph encodedGraph, Iterable methods; + /** + * See {@link #markUnsafeAccess(Class)} for explanation. + */ private enum UnsafeAccessState { + /** + * A {@link TrackedUnsafeAccess} node has never been added to this graph. + */ NO_ACCESS, + + /** + * A {@link TrackedUnsafeAccess} node was added to this graph at a prior point. + */ HAS_ACCESS, + + /** + * In synthetic methods we disable unsafe access tracking. + */ DISABLED } @@ -1104,7 +1119,32 @@ public boolean hasUnsafeAccess() { return hasUnsafeAccess == UnsafeAccessState.HAS_ACCESS; } - public void markUnsafeAccess() { + /** + * HotSpot requires compilations with unsafe accesses to set a flag and uses that information to + * modify the behavior of its signal handling. In Graal we label nodes which require this flag + * with the marker interface {@link TrackedUnsafeAccess}. + * + * @param nodeClass The class from which a node is created that requires unsafe access to be + * set. + */ + public void markUnsafeAccess(Class nodeClass) { + assert TrackedUnsafeAccess.class.isAssignableFrom(nodeClass) : Assertions.errorMessage("%s does not implement MarkedUnsafeAccess", nodeClass); + markUnsafeAccess(); + } + + public void maybeMarkUnsafeAccess(EncodedGraph graph) { + if (graph.hasUnsafeAccess()) { + markUnsafeAccess(); + } + } + + public void maybeMarkUnsafeAccess(StructuredGraph graph) { + if (graph.hasUnsafeAccess()) { + markUnsafeAccess(); + } + } + + private void markUnsafeAccess() { if (hasUnsafeAccess == UnsafeAccessState.DISABLED) { return; } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeAccessNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeAccessNode.java index 1e8e7a7008ff..65eea2a526dc 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeAccessNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeAccessNode.java @@ -44,6 +44,7 @@ import jdk.graal.compiler.nodes.memory.OrderedMemoryAccess; import jdk.graal.compiler.nodes.spi.Canonicalizable; import jdk.graal.compiler.nodes.spi.CanonicalizerTool; +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import jdk.graal.compiler.nodes.type.StampTool; import jdk.vm.ci.meta.ConstantReflectionProvider; import jdk.vm.ci.meta.JavaConstant; @@ -52,7 +53,7 @@ import jdk.vm.ci.meta.ResolvedJavaType; @NodeInfo(cycles = CYCLES_2, size = SIZE_1) -public abstract class UnsafeAccessNode extends FixedWithNextNode implements Canonicalizable, OrderedMemoryAccess, MemoryAccess { +public abstract class UnsafeAccessNode extends FixedWithNextNode implements Canonicalizable, OrderedMemoryAccess, MemoryAccess, TrackedUnsafeAccess { public static final NodeClass TYPE = NodeClass.create(UnsafeAccessNode.class); @Input ValueNode object; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeMemoryLoadNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeMemoryLoadNode.java index 706f1e9c29da..49206974a961 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeMemoryLoadNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeMemoryLoadNode.java @@ -27,6 +27,8 @@ import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_2; import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_1; +import org.graalvm.word.LocationIdentity; + import jdk.graal.compiler.core.common.type.StampFactory; import jdk.graal.compiler.graph.NodeClass; import jdk.graal.compiler.nodeinfo.NodeInfo; @@ -34,15 +36,14 @@ import jdk.graal.compiler.nodes.ValueNode; import jdk.graal.compiler.nodes.memory.MemoryAccess; import jdk.graal.compiler.nodes.spi.Lowerable; -import org.graalvm.word.LocationIdentity; - +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import jdk.vm.ci.meta.JavaKind; /** * Load of a value at a location specified as an absolute address. */ @NodeInfo(cycles = CYCLES_2, size = SIZE_1) -public class UnsafeMemoryLoadNode extends FixedWithNextNode implements Lowerable, MemoryAccess { +public class UnsafeMemoryLoadNode extends FixedWithNextNode implements Lowerable, MemoryAccess, TrackedUnsafeAccess { public static final NodeClass TYPE = NodeClass.create(UnsafeMemoryLoadNode.class); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeMemoryStoreNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeMemoryStoreNode.java index 69c1e09215cf..740629df8c69 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeMemoryStoreNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/UnsafeMemoryStoreNode.java @@ -27,6 +27,8 @@ import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_2; import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_1; +import org.graalvm.word.LocationIdentity; + import jdk.graal.compiler.core.common.type.StampFactory; import jdk.graal.compiler.graph.NodeClass; import jdk.graal.compiler.nodeinfo.NodeInfo; @@ -34,15 +36,14 @@ import jdk.graal.compiler.nodes.ValueNode; import jdk.graal.compiler.nodes.memory.SingleMemoryKill; import jdk.graal.compiler.nodes.spi.Lowerable; -import org.graalvm.word.LocationIdentity; - +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import jdk.vm.ci.meta.JavaKind; /** * Store of a value at a location specified as an absolute address. */ @NodeInfo(cycles = CYCLES_2, size = SIZE_1) -public class UnsafeMemoryStoreNode extends AbstractStateSplit implements Lowerable, SingleMemoryKill { +public class UnsafeMemoryStoreNode extends AbstractStateSplit implements Lowerable, SingleMemoryKill, TrackedUnsafeAccess { public static final NodeClass TYPE = NodeClass.create(UnsafeMemoryStoreNode.class); @Input protected ValueNode value; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AbstractUnsafeCompareAndSwapNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AbstractUnsafeCompareAndSwapNode.java index feb1431e69e9..9a5b7364f95f 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AbstractUnsafeCompareAndSwapNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AbstractUnsafeCompareAndSwapNode.java @@ -31,15 +31,14 @@ import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_8; import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_8; +import org.graalvm.word.LocationIdentity; + import jdk.graal.compiler.core.common.memory.MemoryOrderMode; import jdk.graal.compiler.core.common.type.FloatStamp; import jdk.graal.compiler.core.common.type.Stamp; import jdk.graal.compiler.debug.Assertions; import jdk.graal.compiler.graph.NodeClass; import jdk.graal.compiler.nodeinfo.NodeInfo; -import jdk.graal.compiler.nodes.virtual.VirtualArrayNode; -import jdk.graal.compiler.nodes.virtual.VirtualInstanceNode; -import jdk.graal.compiler.nodes.virtual.VirtualObjectNode; import jdk.graal.compiler.nodes.LogicConstantNode; import jdk.graal.compiler.nodes.LogicNode; import jdk.graal.compiler.nodes.NodeView; @@ -52,15 +51,17 @@ import jdk.graal.compiler.nodes.memory.OrderedMemoryAccess; import jdk.graal.compiler.nodes.memory.SingleMemoryKill; import jdk.graal.compiler.nodes.spi.Lowerable; +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import jdk.graal.compiler.nodes.spi.Virtualizable; import jdk.graal.compiler.nodes.spi.VirtualizerTool; -import org.graalvm.word.LocationIdentity; - +import jdk.graal.compiler.nodes.virtual.VirtualArrayNode; +import jdk.graal.compiler.nodes.virtual.VirtualInstanceNode; +import jdk.graal.compiler.nodes.virtual.VirtualObjectNode; import jdk.vm.ci.meta.JavaKind; import jdk.vm.ci.meta.ResolvedJavaField; @NodeInfo(allowedUsageTypes = {Value, Memory}, cycles = CYCLES_8, size = SIZE_8) -public abstract class AbstractUnsafeCompareAndSwapNode extends AbstractMemoryCheckpoint implements OrderedMemoryAccess, Lowerable, SingleMemoryKill, Virtualizable { +public abstract class AbstractUnsafeCompareAndSwapNode extends AbstractMemoryCheckpoint implements OrderedMemoryAccess, Lowerable, SingleMemoryKill, Virtualizable, TrackedUnsafeAccess { public static final NodeClass TYPE = NodeClass.create(AbstractUnsafeCompareAndSwapNode.class); @Input ValueNode object; @Input ValueNode offset; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AtomicReadAndAddNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AtomicReadAndAddNode.java index 1254e7f71c99..ac2891a31481 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AtomicReadAndAddNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AtomicReadAndAddNode.java @@ -28,6 +28,8 @@ import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_8; import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_2; +import org.graalvm.word.LocationIdentity; + import jdk.graal.compiler.core.common.type.StampFactory; import jdk.graal.compiler.graph.NodeClass; import jdk.graal.compiler.nodeinfo.NodeInfo; @@ -35,8 +37,7 @@ import jdk.graal.compiler.nodes.memory.AbstractMemoryCheckpoint; import jdk.graal.compiler.nodes.memory.SingleMemoryKill; import jdk.graal.compiler.nodes.spi.Lowerable; -import org.graalvm.word.LocationIdentity; - +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import jdk.vm.ci.meta.JavaKind; /** @@ -44,7 +45,7 @@ * {@code sun.misc.Unsafe.getAndAddInt(Object, long, int)}. */ @NodeInfo(allowedUsageTypes = Memory, cycles = CYCLES_8, size = SIZE_2) -public final class AtomicReadAndAddNode extends AbstractMemoryCheckpoint implements Lowerable, SingleMemoryKill { +public final class AtomicReadAndAddNode extends AbstractMemoryCheckpoint implements Lowerable, SingleMemoryKill, TrackedUnsafeAccess { public static final NodeClass TYPE = NodeClass.create(AtomicReadAndAddNode.class); @Input ValueNode object; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AtomicReadAndWriteNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AtomicReadAndWriteNode.java index a7ffee5ac85a..370c4f442954 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AtomicReadAndWriteNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/AtomicReadAndWriteNode.java @@ -27,6 +27,8 @@ import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_8; import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_2; +import org.graalvm.word.LocationIdentity; + import jdk.graal.compiler.core.common.type.StampFactory; import jdk.graal.compiler.graph.NodeClass; import jdk.graal.compiler.nodeinfo.NodeInfo; @@ -34,8 +36,7 @@ import jdk.graal.compiler.nodes.memory.AbstractMemoryCheckpoint; import jdk.graal.compiler.nodes.memory.SingleMemoryKill; import jdk.graal.compiler.nodes.spi.Lowerable; -import org.graalvm.word.LocationIdentity; - +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import jdk.vm.ci.meta.JavaKind; /** @@ -43,7 +44,7 @@ * {@code sun.misc.Unsafe.getAndSetInt(Object, long, int)}. */ @NodeInfo(cycles = CYCLES_8, size = SIZE_2) -public final class AtomicReadAndWriteNode extends AbstractMemoryCheckpoint implements Lowerable, SingleMemoryKill { +public final class AtomicReadAndWriteNode extends AbstractMemoryCheckpoint implements Lowerable, SingleMemoryKill, TrackedUnsafeAccess { public static final NodeClass TYPE = NodeClass.create(AtomicReadAndWriteNode.class); @Input ValueNode object; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/TrackedUnsafeAccess.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/TrackedUnsafeAccess.java new file mode 100644 index 000000000000..8b812ffcc77b --- /dev/null +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/TrackedUnsafeAccess.java @@ -0,0 +1,34 @@ +/* + * 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.nodes.spi; + +import jdk.graal.compiler.nodes.StructuredGraph; + +/** + * Marker interface to denote a node requires {@link StructuredGraph#markUnsafeAccess(Class)} to be + * set. + */ +public interface TrackedUnsafeAccess { +} diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/inlining/InliningUtil.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/inlining/InliningUtil.java index ab45ce736feb..ac40d0344d0a 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/inlining/InliningUtil.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/inlining/InliningUtil.java @@ -667,9 +667,7 @@ private static ValueNode finishInlining(Invoke invoke, StructuredGraph graph, Fi // Copy inlined methods from inlinee to caller graph.updateMethods(inlineGraph); - if (inlineGraph.hasUnsafeAccess()) { - graph.markUnsafeAccess(); - } + graph.maybeMarkUnsafeAccess(inlineGraph); assert inlineGraph.getSpeculationLog() == null || inlineGraph.getSpeculationLog() == graph.getSpeculationLog() : "Only the root graph should have a speculation log"; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java index 5fe16c068cc4..de820f45cb04 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java @@ -683,7 +683,7 @@ private static void registerUnsafeGetAndOpPlugins(Registration r, boolean isSunM public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver unsafe, ValueNode object, ValueNode offset, ValueNode value) { // Emits a null-check for the otherwise unused receiver unsafe.get(true); - createUnsafeAccess(object, b, (obj, loc) -> new AtomicReadAndWriteNode(obj, offset, value, kind, loc)); + createUnsafeAccess(object, b, (obj, loc) -> new AtomicReadAndWriteNode(obj, offset, value, kind, loc), AtomicReadAndWriteNode.class); return true; } }); @@ -694,7 +694,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver unsafe, ValueNode object, ValueNode offset, ValueNode delta) { // Emits a null-check for the otherwise unused receiver unsafe.get(true); - createUnsafeAccess(object, b, (obj, loc) -> new AtomicReadAndAddNode(obj, offset, delta, kind, loc)); + createUnsafeAccess(object, b, (obj, loc) -> new AtomicReadAndAddNode(obj, offset, delta, kind, loc), AtomicReadAndAddNode.class); return true; } }); @@ -1445,9 +1445,9 @@ private void setAccessNodeResult(FixedWithNextNode node, GraphBuilderContext b) } } - protected final void createUnsafeAccess(ValueNode value, GraphBuilderContext b, UnsafeNodeConstructor nodeConstructor) { + protected final void createUnsafeAccess(ValueNode value, GraphBuilderContext b, UnsafeNodeConstructor nodeConstructor, Class constructorClass) { StructuredGraph graph = b.getGraph(); - graph.markUnsafeAccess(); + graph.markUnsafeAccess(constructorClass); /* For unsafe access object pointers can only be stored in the heap */ if (unsafeAccessKind == JavaKind.Object) { ValueNode object = value; @@ -1527,7 +1527,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec // Emits a null-check for the otherwise unused receiver unsafe.get(true); b.addPush(returnKind, new UnsafeMemoryLoadNode(address, unsafeAccessKind, OFF_HEAP_LOCATION)); - b.getGraph().markUnsafeAccess(); + b.getGraph().markUnsafeAccess(UnsafeMemoryLoadNode.class); return true; } @@ -1543,7 +1543,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec unsafe.get(true); // Note that non-ordered raw accesses can be turned into floatable field accesses. UnsafeNodeConstructor unsafeNodeConstructor = (obj, loc) -> new RawLoadNode(obj, offset, unsafeAccessKind, loc, memoryOrder); - createUnsafeAccess(object, b, unsafeNodeConstructor); + createUnsafeAccess(object, b, unsafeNodeConstructor, RawLoadNode.class); return true; } } @@ -1567,7 +1567,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec unsafe.get(true); ValueNode maskedValue = b.maskSubWordValue(value, unsafeAccessKind); b.add(new UnsafeMemoryStoreNode(address, maskedValue, unsafeAccessKind, OFF_HEAP_LOCATION)); - b.getGraph().markUnsafeAccess(); + b.getGraph().markUnsafeAccess(UnsafeMemoryStoreNode.class); return true; } @@ -1582,7 +1582,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec // Emits a null-check for the otherwise unused receiver unsafe.get(true); ValueNode maskedValue = b.maskSubWordValue(value, unsafeAccessKind); - createUnsafeAccess(object, b, (obj, loc) -> new RawStoreNode(obj, offset, maskedValue, unsafeAccessKind, loc, true, memoryOrder)); + createUnsafeAccess(object, b, (obj, loc) -> new RawStoreNode(obj, offset, maskedValue, unsafeAccessKind, loc, true, memoryOrder), RawStoreNode.class); return true; } } @@ -1603,9 +1603,9 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec // Emits a null-check for the otherwise unused receiver unsafe.get(true); if (isLogic) { - createUnsafeAccess(object, b, (obj, loc) -> new UnsafeCompareAndSwapNode(obj, offset, expected, newValue, unsafeAccessKind, loc, memoryOrder)); + createUnsafeAccess(object, b, (obj, loc) -> new UnsafeCompareAndSwapNode(obj, offset, expected, newValue, unsafeAccessKind, loc, memoryOrder), UnsafeCompareAndSwapNode.class); } else { - createUnsafeAccess(object, b, (obj, loc) -> new UnsafeCompareAndExchangeNode(obj, offset, expected, newValue, unsafeAccessKind, loc, memoryOrder)); + createUnsafeAccess(object, b, (obj, loc) -> new UnsafeCompareAndExchangeNode(obj, offset, expected, newValue, unsafeAccessKind, loc, memoryOrder), UnsafeCompareAndExchangeNode.class); } return true; } diff --git a/substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java b/substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java index f3a98f3d8a32..38ec91fff8ba 100644 --- a/substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java +++ b/substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java @@ -50,20 +50,12 @@ import java.util.function.Predicate; import java.util.stream.Collectors; -import jdk.graal.compiler.debug.DebugContext; -import jdk.graal.compiler.graph.NodeInputList; -import jdk.graal.compiler.nodes.Invoke; -import jdk.graal.compiler.nodes.PiNode; -import jdk.graal.compiler.nodes.StructuredGraph; -import jdk.graal.compiler.nodes.ValueNode; -import jdk.graal.compiler.nodes.java.NewInstanceNode; -import jdk.graal.compiler.options.Option; -import jdk.graal.compiler.options.OptionType; import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.nativeimage.hosted.Feature; import org.graalvm.polyglot.io.FileSystem; import com.oracle.graal.pointsto.BigBang; +import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider; import com.oracle.graal.pointsto.meta.AnalysisMethod; import com.oracle.graal.pointsto.meta.AnalysisType; import com.oracle.graal.pointsto.meta.InvokeInfo; @@ -82,6 +74,18 @@ import com.oracle.truffle.api.TruffleLanguage; import com.oracle.truffle.runtime.OptimizedCallTarget; +import jdk.graal.compiler.debug.DebugContext; +import jdk.graal.compiler.graph.Node; +import jdk.graal.compiler.graph.NodeInputList; +import jdk.graal.compiler.graph.NodeSourcePosition; +import jdk.graal.compiler.nodes.Invoke; +import jdk.graal.compiler.nodes.PiNode; +import jdk.graal.compiler.nodes.StructuredGraph; +import jdk.graal.compiler.nodes.ValueNode; +import jdk.graal.compiler.nodes.java.NewInstanceNode; +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; +import jdk.graal.compiler.options.Option; +import jdk.graal.compiler.options.OptionType; import jdk.vm.ci.common.JVMCIError; import jdk.vm.ci.meta.ModifiersProvider; import jdk.vm.ci.meta.ResolvedJavaMethod; @@ -333,6 +337,43 @@ private Map> callGraph(BigBang bb, Set> visited, + SVMHost hostVM) { + /* + * In this situation it is unnecessary to check for unsafe accesses. + */ + if (inlinedUnsafeCall == null || isSystemOrSafeClass(mNode)) { + return; + } + + StructuredGraph mGraph = hostVM.getAnalysisGraph(mNode.getMethod()); + for (Node node : mGraph.getNodes().filter(n -> n instanceof TrackedUnsafeAccess)) { + /* + * Check the origin of all tracked unsafe accesses. + * + * We must determine whether the access originates from a safe class. It is possible for + * these accesses to be inlined into other methods during the method handle + * intrinsification process. + */ + NodeSourcePosition current = node.getNodeSourcePosition(); + boolean foundSystemClass = false; + while (current != null) { + var declaringClass = OriginalClassProvider.getJavaClass(current.getMethod().getDeclaringClass()); + if (!declaringClass.equals(sunMiscUnsafe) && isSystemClass(declaringClass)) { + foundSystemClass = true; + break; + } + current = current.getCaller(); + } + if (!foundSystemClass) { + visited.computeIfAbsent(inlinedUnsafeCall, (e) -> new HashSet<>()).add(mNode); + return; + } + } + } + private boolean callGraphImpl( BaseMethodNode mNode, Set targets, @@ -343,13 +384,7 @@ private boolean callGraphImpl( AnalysisMethod m = mNode.getMethod(); String mName = getMethodName(m); path.addFirst(mNode); - StructuredGraph mGraph = hostVM.getAnalysisGraph(m); - if (mGraph.hasUnsafeAccess() && !isSystemOrSafeClass(mNode)) { - debugContext.log(DebugContext.VERY_DETAILED_LEVEL, "Method: %s has unsafe access.", mName); - if (inlinedUnsafeCall != null) { - visited.computeIfAbsent(inlinedUnsafeCall, (e) -> new HashSet<>()).add(mNode); - } - } + findUnsafeAccesses(mNode, visited, hostVM); try { boolean callPathContainsTarget = false; debugContext.log(DebugContext.VERY_DETAILED_LEVEL, "Entered method: %s.", mName); @@ -514,7 +549,10 @@ private static boolean isSystemOrSafeClass(BaseMethodNode methodNode) { * @param methodNode the {@link BaseMethodNode} to check */ private static boolean isSystemClass(BaseMethodNode methodNode) { - Class clz = methodNode.getOwner().getJavaClass(); + return isSystemClass(methodNode.getOwner().getJavaClass()); + } + + private static boolean isSystemClass(Class clz) { if (clz == null) { return false; } diff --git a/vm/mx.vm/mx_vm_gate.py b/vm/mx.vm/mx_vm_gate.py index b2ba407f6dbb..116d8cb5462b 100644 --- a/vm/mx.vm/mx_vm_gate.py +++ b/vm/mx.vm/mx_vm_gate.py @@ -682,7 +682,8 @@ def _collect_excludes(suite, suite_import, excludes): ] + mx_sdk_vm_impl.svm_experimental_options([ '-H:ClassInitialization=:build_time', '-H:+EnforceMaxRuntimeCompileMethods', - '-H:+UseOldMethodHandleIntrinsics', + '--add-exports=jdk.graal.compiler/jdk.graal.compiler.graph.iterators=ALL-UNNAMED', + '--add-exports=jdk.graal.compiler/jdk.graal.compiler.nodes.spi=ALL-UNNAMED', '-cp', cp, '-H:-FoldSecurityManagerGetter', From 56e3ead09029d90dc1f3637cdd9c21343134a385 Mon Sep 17 00:00:00 2001 From: Doug Simon Date: Tue, 5 Mar 2024 16:19:13 +0100 Subject: [PATCH 2/3] tighten up code related to TrackedUnsafeAccess --- .../jdk/graal/compiler/nodes/StructuredGraph.java | 15 +++++++-------- .../compiler/nodes/spi/TrackedUnsafeAccess.java | 4 ++-- .../replacements/StandardGraphBuilderPlugins.java | 3 ++- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/StructuredGraph.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/StructuredGraph.java index 6cacdf748b9c..ebd644d066a6 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/StructuredGraph.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/StructuredGraph.java @@ -60,9 +60,9 @@ import jdk.graal.compiler.nodes.cfg.HIRBlock; import jdk.graal.compiler.nodes.java.ExceptionObjectNode; import jdk.graal.compiler.nodes.java.MethodCallTargetNode; -import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import jdk.graal.compiler.nodes.spi.ProfileProvider; import jdk.graal.compiler.nodes.spi.ResolvedJavaMethodProfileProvider; +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import jdk.graal.compiler.nodes.spi.VirtualizableAllocation; import jdk.graal.compiler.nodes.util.GraphUtil; import jdk.graal.compiler.options.OptionValues; @@ -1120,15 +1120,14 @@ public boolean hasUnsafeAccess() { } /** - * HotSpot requires compilations with unsafe accesses to set a flag and uses that information to - * modify the behavior of its signal handling. In Graal we label nodes which require this flag - * with the marker interface {@link TrackedUnsafeAccess}. + * Records that this graph encodes a memory access via the {@code Unsafe} class. + * + * HotSpot requires this information to modify the behavior of its signal handling for compiled + * code that contains an unsafe memory access. * - * @param nodeClass The class from which a node is created that requires unsafe access to be - * set. + * @param nodeClass the type of the node encoding the unsafe access */ - public void markUnsafeAccess(Class nodeClass) { - assert TrackedUnsafeAccess.class.isAssignableFrom(nodeClass) : Assertions.errorMessage("%s does not implement MarkedUnsafeAccess", nodeClass); + public void markUnsafeAccess(Class nodeClass) { markUnsafeAccess(); } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/TrackedUnsafeAccess.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/TrackedUnsafeAccess.java index 8b812ffcc77b..580872c361f2 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/TrackedUnsafeAccess.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/TrackedUnsafeAccess.java @@ -27,8 +27,8 @@ import jdk.graal.compiler.nodes.StructuredGraph; /** - * Marker interface to denote a node requires {@link StructuredGraph#markUnsafeAccess(Class)} to be - * set. + * Marker interface for a node that requires {@link StructuredGraph#markUnsafeAccess(Class)} to be + * called prior to being added to a graph. */ public interface TrackedUnsafeAccess { } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java index de820f45cb04..37a55f4d3efe 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java @@ -49,6 +49,7 @@ import java.util.Objects; import java.util.function.BiFunction; +import jdk.graal.compiler.nodes.spi.TrackedUnsafeAccess; import org.graalvm.word.LocationIdentity; import jdk.graal.compiler.api.directives.GraalDirectives; @@ -1445,7 +1446,7 @@ private void setAccessNodeResult(FixedWithNextNode node, GraphBuilderContext b) } } - protected final void createUnsafeAccess(ValueNode value, GraphBuilderContext b, UnsafeNodeConstructor nodeConstructor, Class constructorClass) { + protected final void createUnsafeAccess(ValueNode value, GraphBuilderContext b, UnsafeNodeConstructor nodeConstructor, Class constructorClass) { StructuredGraph graph = b.getGraph(); graph.markUnsafeAccess(constructorClass); /* For unsafe access object pointers can only be stored in the heap */ From 1d0b0551488b9bc78ca93f0d23f5119c82119193 Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Mon, 11 Mar 2024 21:40:36 +0100 Subject: [PATCH 3/3] adjust TCK smoke test unsafe access --- .../tck/tests/language/TCKSmokeTestLanguage.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/truffle/src/com.oracle.truffle.tck.tests.language/src/com/oracle/truffle/tck/tests/language/TCKSmokeTestLanguage.java b/truffle/src/com.oracle.truffle.tck.tests.language/src/com/oracle/truffle/tck/tests/language/TCKSmokeTestLanguage.java index 2010bf4d6eb1..b97eb50c4306 100644 --- a/truffle/src/com.oracle.truffle.tck.tests.language/src/com/oracle/truffle/tck/tests/language/TCKSmokeTestLanguage.java +++ b/truffle/src/com.oracle.truffle.tck.tests.language/src/com/oracle/truffle/tck/tests/language/TCKSmokeTestLanguage.java @@ -141,14 +141,11 @@ void doInterrupt() { private static final class UnsafeCallNode extends BaseNode { private static final Unsafe UNSAFE; - private static final long VALUE_OFFSET; static { try { Field f = Unsafe.class.getDeclaredField("theUnsafe"); f.setAccessible(true); UNSAFE = (Unsafe) f.get(null); - f = Integer.class.getDeclaredField("value"); - VALUE_OFFSET = getFieldOffset(f); } catch (ReflectiveOperationException e) { throw new RuntimeException(e); } @@ -165,16 +162,25 @@ void execute(VirtualFrame frame) { doBehindBoundaryUnsafeAccess(); } + @TruffleBoundary + static long getFieldOffset(Class clazz, String value) { + try { + return getFieldOffset(clazz.getDeclaredField(value)); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e); + } + } + static void doUnsafeAccess() { int i = 42; - int result = UNSAFE.getInt(i, VALUE_OFFSET); + int result = UNSAFE.getInt(i, getFieldOffset(Integer.class, "value")); assert i == result; } @TruffleBoundary static void doBehindBoundaryUnsafeAccess() { int i = 23; - int result = UNSAFE.getInt(i, VALUE_OFFSET); + int result = UNSAFE.getInt(i, getFieldOffset(Integer.class, "value")); assert i == result; } }