diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/BytecodeOSRNodeTest.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/BytecodeOSRNodeTest.java index 00395f8f38f8..129c6d48a4b4 100644 --- a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/BytecodeOSRNodeTest.java +++ b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/BytecodeOSRNodeTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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 @@ -26,14 +26,9 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.util.List; import java.util.concurrent.TimeUnit; -import com.oracle.truffle.api.test.SubprocessTestUtils; -import com.oracle.truffle.runtime.BytecodeOSRMetadata; -import com.oracle.truffle.runtime.OptimizedTruffleRuntime; -import com.oracle.truffle.runtime.OptimizedCallTarget; - -import jdk.graal.compiler.test.GraalTest; import org.graalvm.polyglot.Context; import org.junit.Assert; import org.junit.Before; @@ -54,10 +49,17 @@ import com.oracle.truffle.api.frame.FrameSlotKind; import com.oracle.truffle.api.frame.FrameSlotTypeException; import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.impl.FrameWithoutBoxing; import com.oracle.truffle.api.nodes.BytecodeOSRNode; import com.oracle.truffle.api.nodes.ExplodeLoop; import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.nodes.RootNode; +import com.oracle.truffle.api.test.SubprocessTestUtils; +import com.oracle.truffle.runtime.BytecodeOSRMetadata; +import com.oracle.truffle.runtime.OptimizedCallTarget; +import com.oracle.truffle.runtime.OptimizedTruffleRuntime; + +import jdk.graal.compiler.test.GraalTest; public class BytecodeOSRNodeTest extends TestWithSynchronousCompiling { @@ -407,6 +409,24 @@ public void testFrameTransfer() { */ @Test public void testFrameTransferWithStaticAccesses() { + frameTransferWithStaticAccesses(); + } + + /* + * Test that the frame transfer helper works as expected, even with static accesses, both on OSR + * enter and exit, and even when assertions are disabled. + */ + @Test + public void testFrameTransferWithStaticAccessesWithAssertionsDisabled() throws Throwable { + // Execute in a subprocess to disable assertion checking for frames. + SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class, + () -> { + Assert.assertFalse(FrameAssertionsChecker.areFrameAssertionsEnabled()); + frameTransferWithStaticAccesses(); + }, true, List.of(FrameWithoutBoxing.class)); + } + + public static void frameTransferWithStaticAccesses() { var frameBuilder = FrameDescriptor.newBuilder(); RootNode rootNode = new Program(new FrameTransferringWithStaticAccessNode(frameBuilder), frameBuilder.build()); OptimizedCallTarget target = (OptimizedCallTarget) rootNode.getCallTarget(); @@ -456,6 +476,40 @@ public void testFrameTransferWithUninitializedSlots() { Assert.assertEquals(42, target.call()); } + /* + * Same as above, but with static slots. + * + * Note that uninitialized static slots differs from regular slots in that it may be read as any + * kind, and always return the default ('defaultValue' for Object, '0' for primitives). + */ + @Test + public void testFrameTransferWithUninitializedStaticSlots() { + frameTransferWithUninitializedStaticSlots(); + } + + /* + * Same as above, but with assertion disabled. + */ + @Test + public void testFrameTransferWithUninitializedStaticSlotsWithoutAssertions() throws Throwable { + // Execute in a subprocess to disable assertion checking for frames. + SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class, + () -> { + Assert.assertFalse(FrameAssertionsChecker.areFrameAssertionsEnabled()); + frameTransferWithUninitializedStaticSlots(); + }, true, List.of(FrameWithoutBoxing.class)); + } + + public static void frameTransferWithUninitializedStaticSlots() { + // use a non-null default value to make sure it gets copied properly. + var frameBuilder = FrameDescriptor.newBuilder(); + Object defaultValue = new Object(); + frameBuilder.defaultValue(defaultValue); + RootNode rootNode = new Program(new FrameTransferringNodeWithUninitializedStaticSlots(frameBuilder, defaultValue), frameBuilder.build()); + OptimizedCallTarget target = (OptimizedCallTarget) rootNode.getCallTarget(); + Assert.assertEquals(42, target.call()); + } + /* * Test that we can safely recover from bailing out of OSR compilation while an OSR frame is * currently executing. @@ -1538,6 +1592,71 @@ public void checkOSRState(VirtualFrame frame) { } } + public static class FrameTransferringNodeWithUninitializedStaticSlots extends FrameTransferringWithStaticAccessNode { + final Object defaultValue; + + public FrameTransferringNodeWithUninitializedStaticSlots(FrameDescriptor.Builder frameDescriptor, Object defaultValue) { + super(frameDescriptor); + this.defaultValue = defaultValue; + } + + private void ensureUninitReturnsDefault(VirtualFrame frame, int index) { + assertEquals(defaultValue, frame.getObjectStatic(index)); + assertEquals((byte) 0, frame.getByteStatic(index)); + assertEquals(false, frame.getBooleanStatic(index)); + assertEquals(0, frame.getIntStatic(index)); + assertEquals(0L, frame.getLongStatic(index)); + assertEquals(0f, frame.getFloatStatic(index)); + assertEquals(0d, frame.getDoubleStatic(index)); + } + + @Override + public void setRegularState(VirtualFrame frame) { + frame.setByteStatic(byteSlot, (byte) 1); + // everything else is uninitialized + } + + @Override + public void checkRegularState(VirtualFrame frame) { + try { + assertEquals((byte) 1, frame.getByteStatic(byteSlot)); + // these slots are uninitialized + ensureUninitReturnsDefault(frame, booleanSlot); + ensureUninitReturnsDefault(frame, doubleSlot); + ensureUninitReturnsDefault(frame, floatSlot); + ensureUninitReturnsDefault(frame, intSlot); + ensureUninitReturnsDefault(frame, longSlot); + ensureUninitReturnsDefault(frame, objectSlot); + } catch (FrameSlotTypeException ex) { + CompilerDirectives.transferToInterpreter(); + throw new IllegalStateException("Error accessing index slot"); + } + } + + @Override + public void setOSRState(VirtualFrame frame) { + frame.setByteStatic(byteSlot, (byte) 2); + // everything else is uninitialized + } + + @Override + public void checkOSRState(VirtualFrame frame) { + try { + assertEquals((byte) 2, frame.getByteStatic(byteSlot)); + // these slots are uninitialized + ensureUninitReturnsDefault(frame, booleanSlot); + ensureUninitReturnsDefault(frame, doubleSlot); + ensureUninitReturnsDefault(frame, floatSlot); + ensureUninitReturnsDefault(frame, intSlot); + ensureUninitReturnsDefault(frame, longSlot); + ensureUninitReturnsDefault(frame, objectSlot); + } catch (FrameSlotTypeException ex) { + CompilerDirectives.transferToInterpreter(); + throw new IllegalStateException("Error accessing index slot"); + } + } + } + public static class OSRDisablingTransferringNode extends FrameTransferringNode { @CompilationFinal int staticSlot; diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameAssertionsChecker.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameAssertionsChecker.java new file mode 100644 index 000000000000..32c79276a485 --- /dev/null +++ b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameAssertionsChecker.java @@ -0,0 +1,64 @@ +/* + * 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.truffle.test; + +import java.lang.reflect.Field; + +import org.junit.Assert; + +import com.oracle.truffle.api.impl.FrameWithoutBoxing; + +public final class FrameAssertionsChecker { + private static final boolean ASSERTIONS_ENABLED; + + private FrameAssertionsChecker() { + } + + static { + ASSERTIONS_ENABLED = getFrameAssertionsStatus(); + } + + public static boolean areFrameAssertionsEnabled() { + return ASSERTIONS_ENABLED; + } + + private static boolean getFrameAssertionsStatus() { + Field assertionsEnabledField = null; + for (Field f : FrameWithoutBoxing.class.getDeclaredFields()) { + if (f.getName().equals("ASSERTIONS_ENABLED")) { + assertionsEnabledField = f; + break; + } + } + Assert.assertNotNull(assertionsEnabledField); + try { + assertionsEnabledField.setAccessible(true); + return ((boolean) assertionsEnabledField.get(null)); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } +} diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameHostReadsTest.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameHostReadsTest.java index 1a9a94a0bbc1..c7dfb073c3db 100644 --- a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameHostReadsTest.java +++ b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameHostReadsTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 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 @@ -24,11 +24,9 @@ */ package jdk.graal.compiler.truffle.test; -import jdk.graal.compiler.api.directives.GraalDirectives; -import jdk.graal.compiler.nodes.FieldLocationIdentity; -import jdk.graal.compiler.nodes.NamedLocationIdentity; -import jdk.graal.compiler.nodes.StructuredGraph; -import jdk.graal.compiler.nodes.memory.ReadNode; +import java.io.IOException; +import java.util.List; + import org.graalvm.word.LocationIdentity; import org.junit.Assert; import org.junit.Test; @@ -36,6 +34,13 @@ import com.oracle.truffle.api.Truffle; import com.oracle.truffle.api.frame.FrameDescriptor; import com.oracle.truffle.api.impl.FrameWithoutBoxing; +import com.oracle.truffle.api.test.SubprocessTestUtils; + +import jdk.graal.compiler.api.directives.GraalDirectives; +import jdk.graal.compiler.nodes.FieldLocationIdentity; +import jdk.graal.compiler.nodes.NamedLocationIdentity; +import jdk.graal.compiler.nodes.StructuredGraph; +import jdk.graal.compiler.nodes.memory.ReadNode; /** * Tests that we can move frame array reads out of the loop even if there is a side-effect in the @@ -59,11 +64,23 @@ public static int snippet0(FrameWithoutBoxing frame, int index) { } @Test - public void test0() { + public void test0() throws IOException, InterruptedException { + // Run in subprocess to disable assertions in FrameWithoutBoxing. + // Assertion checking requires additional checks in the frame descriptor for handling of + // static slots. + SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(FrameHostReadsTest.class, () -> { + compileAndCheck(); + }, + true, + List.of(FrameWithoutBoxing.class)); + } + + private void compileAndCheck() { getTruffleCompiler(); initAssertionError(); Assert.assertSame("New frame implementation detected. Make sure to update this test.", FrameWithoutBoxing.class, Truffle.getRuntime().createVirtualFrame(new Object[0], FrameDescriptor.newBuilder().build()).getClass()); + Assert.assertTrue("Frame assertions should be disabled.", !FrameAssertionsChecker.areFrameAssertionsEnabled()); StructuredGraph graph = getFinalGraph("snippet0"); @@ -86,25 +103,21 @@ public void test0() { } /* - * Frame read for FrameWithoutBoxing.indexedTags and - * FrameWithoutBoxing.indexedPrimitiveLocals. - * - * Note we expect two reads here as assertions are enabled. + * Frame read for FrameWithoutBoxing.indexedPrimitiveLocals. */ - Assert.assertEquals(2, fieldReads); + Assert.assertEquals(1, fieldReads); /* * Array reads inside of the loop. We expect the loop to get unrolled, so we expect 5 times * the number of reads. It is important that the loop does not contain reads to * FrameWithoutBoxing.indexedTags or FrameWithoutBoxing.indexedPrimitiveLocals. */ - Assert.assertEquals(10, arrayReads); + Assert.assertEquals(5, arrayReads); /* - * Array.length reads. We also read two because we have assertions enabled. one for - * FrameWithoutBoxing.indexedTags and one for FrameWithoutBoxing.indexedPrimitiveLocals. + * Array.length reads. We read one for FrameWithoutBoxing.indexedPrimitiveLocals. */ - Assert.assertEquals(2, arrayLengthReads); + Assert.assertEquals(1, arrayLengthReads); Assert.assertEquals(0, otherReads); } @@ -118,5 +131,4 @@ public Throwable fillInStackTrace() { } }; } - } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/KnownTruffleTypes.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/KnownTruffleTypes.java index 523ecce6928d..534d3eb25d52 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/KnownTruffleTypes.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/KnownTruffleTypes.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 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 @@ -104,7 +104,6 @@ public class KnownTruffleTypes extends AbstractKnownTruffleTypes { public final ResolvedJavaField FrameDescriptor_materializeCalled = findField(FrameDescriptor, "materializeCalled"); public final ResolvedJavaField FrameDescriptor_indexedSlotTags = findField(FrameDescriptor, "indexedSlotTags"); public final ResolvedJavaField FrameDescriptor_auxiliarySlotCount = findField(FrameDescriptor, "auxiliarySlotCount"); - public final ResolvedJavaField FrameDescriptor_staticMode = findField(FrameDescriptor, "staticMode"); public final ResolvedJavaType FrameSlotKind = lookupTypeCached("com.oracle.truffle.api.frame.FrameSlotKind"); public final ResolvedJavaField FrameSlotKind_Object = findField(FrameSlotKind, "Object"); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/InjectImmutableFrameFieldsPhase.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/InjectImmutableFrameFieldsPhase.java index c73828646e92..d8a3ce373041 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/InjectImmutableFrameFieldsPhase.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/InjectImmutableFrameFieldsPhase.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 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 @@ -34,7 +34,6 @@ import jdk.graal.compiler.phases.PhaseSuite; import jdk.graal.compiler.phases.common.HighTierLoweringPhase; import jdk.graal.compiler.phases.tiers.HighTierContext; - import jdk.vm.ci.meta.ResolvedJavaField; import jdk.vm.ci.meta.ResolvedJavaType; @@ -58,10 +57,12 @@ protected void run(StructuredGraph graph, HighTierContext context) { return; } ResolvedJavaType frameType = env.types().FrameWithoutBoxing; + ResolvedJavaType frameDescriptorType = env.types().FrameDescriptor; for (Node node : graph.getNodes()) { if (node instanceof LoadFieldNode) { LoadFieldNode fieldNode = (LoadFieldNode) node; - if (isForcedImmutable(env, fieldNode.field(), frameType) && !fieldNode.getLocationIdentity().isImmutable()) { + if ((isForcedImmutable(env, fieldNode.field(), frameType) || isForcedImmutable(env, fieldNode.field(), frameDescriptorType)) && + !fieldNode.getLocationIdentity().isImmutable()) { graph.replaceFixedWithFixed(fieldNode, graph.add(LoadFieldNode.createOverrideImmutable(fieldNode))); } } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/TruffleKnownHostTypes.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/TruffleKnownHostTypes.java index d7f22476ab24..f8afe2457c28 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/TruffleKnownHostTypes.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/host/TruffleKnownHostTypes.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 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 @@ -24,11 +24,10 @@ */ package jdk.graal.compiler.truffle.host; -import jdk.graal.compiler.truffle.AbstractKnownTruffleTypes; -import jdk.graal.compiler.truffle.KnownTruffleTypes; - import com.oracle.truffle.compiler.TruffleCompilerRuntime; +import jdk.graal.compiler.truffle.AbstractKnownTruffleTypes; +import jdk.graal.compiler.truffle.KnownTruffleTypes; import jdk.vm.ci.meta.MetaAccessProvider; import jdk.vm.ci.meta.ResolvedJavaMethod; import jdk.vm.ci.meta.ResolvedJavaType; @@ -41,6 +40,9 @@ public final class TruffleKnownHostTypes extends AbstractKnownTruffleTypes { // Checkstyle: stop field name check + // truffle.api.frame + public final ResolvedJavaType FrameDescriptor = lookupTypeCached("com.oracle.truffle.api.frame.FrameDescriptor"); + // truffle.api.impl public final ResolvedJavaType FrameWithoutBoxing = lookupType("com.oracle.truffle.api.impl.FrameWithoutBoxing"); public final ResolvedJavaType OptimizedCallTarget = lookupTypeCached("com.oracle.truffle.runtime.OptimizedCallTarget"); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/NewFrameNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/NewFrameNode.java index feca3183719c..b004624cec31 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/NewFrameNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/NewFrameNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 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 @@ -56,7 +56,6 @@ import jdk.graal.compiler.nodes.virtual.VirtualObjectNode; import jdk.graal.compiler.serviceprovider.SpeculationReasonGroup; import jdk.graal.compiler.truffle.KnownTruffleTypes; - import jdk.vm.ci.meta.ConstantReflectionProvider; import jdk.vm.ci.meta.JavaConstant; import jdk.vm.ci.meta.JavaKind; @@ -89,10 +88,6 @@ public final class NewFrameNode extends FixedWithNextNode implements IterableNod public static final byte FrameSlotKindIllegalTag = 7; // FrameSlotKind.Illegal.tag public static final byte FrameSlotKindStaticTag = 8; // FrameSlotKind.Static.tag - private static final byte FrameDescriptorNoStaticMode = 1; // FrameDescriptor.NO_STATIC_MODE - private static final byte FrameDescriptorAllStaticMode = 2; // FrameDescriptor.ALL_STATIC_MODE - private static final byte FrameDescriptorMixedStaticMode = FrameDescriptorNoStaticMode | FrameDescriptorAllStaticMode; // FrameDescriptor.MIXED_STATIC_MODE - public static final NodeClass TYPE = NodeClass.create(NewFrameNode.class); @Input ValueNode descriptor; @Input ValueNode arguments; @@ -112,7 +107,6 @@ public final class NewFrameNode extends FixedWithNextNode implements IterableNod private final byte[] indexedFrameSlotKinds; private final int indexedFrameSize; private final int auxiliarySize; - private final int staticMode; private final SpeculationReason intrinsifyAccessorsSpeculation; @@ -196,25 +190,14 @@ public NewFrameNode(GraphBuilderContext b, ValueNode frameDescriptorNode, ValueN JavaConstant indexedTagsArray = constantReflection.readFieldValue(types.FrameDescriptor_indexedSlotTags, frameDescriptor); this.indexedFrameSize = constantReflection.readArrayLength(indexedTagsArray); - JavaConstant staticModeValue = constantReflection.readFieldValue(types.FrameDescriptor_staticMode, frameDescriptor); - this.staticMode = staticModeValue.asInt(); - byte[] indexedFrameSlotKindsCandidate = new byte[indexedFrameSize]; - if (staticMode == FrameDescriptorNoStaticMode) { - Arrays.fill(indexedFrameSlotKindsCandidate, FrameSlotKindLongTag); - } else if (staticMode == FrameDescriptorAllStaticMode) { - Arrays.fill(indexedFrameSlotKindsCandidate, FrameSlotKindStaticTag); - } else { - if (staticMode == FrameDescriptorMixedStaticMode) { - final int indexedTagsArrayLength = constantReflection.readArrayLength(indexedTagsArray); - for (int i = 0; i < indexedTagsArrayLength; i++) { - final int slot = constantReflection.readArrayElement(indexedTagsArray, i).asInt(); - if (slot == FrameSlotKindStaticTag) { - indexedFrameSlotKindsCandidate[i] = FrameSlotKindStaticTag; - } else { - indexedFrameSlotKindsCandidate[i] = FrameSlotKindLongTag; - } - } + final int indexedTagsArrayLength = constantReflection.readArrayLength(indexedTagsArray); + for (int i = 0; i < indexedTagsArrayLength; i++) { + final int slot = constantReflection.readArrayElement(indexedTagsArray, i).asInt(); + if (slot == FrameSlotKindStaticTag) { + indexedFrameSlotKindsCandidate[i] = FrameSlotKindStaticTag; + } else { + indexedFrameSlotKindsCandidate[i] = FrameSlotKindLongTag; } } this.indexedFrameSlotKinds = indexedFrameSlotKindsCandidate; @@ -269,6 +252,10 @@ public byte[] getIndexedFrameSlotKinds() { return indexedFrameSlotKinds; } + public boolean isStatic(int slot) { + return indexedFrameSlotKinds[slot] == FrameSlotKindStaticTag; + } + public ValueNode getDescriptor() { return descriptor; } @@ -312,21 +299,7 @@ public void virtualize(VirtualizerTool tool) { ValueNode[] indexedTagArrayEntryState = new ValueNode[indexedFrameSize]; Arrays.fill(indexedObjectArrayEntryState, frameDefaultValue); - if (staticMode == FrameDescriptorNoStaticMode) { - Arrays.fill(indexedTagArrayEntryState, smallIntConstants.get(0)); - } else if (staticMode == FrameDescriptorAllStaticMode) { - Arrays.fill(indexedTagArrayEntryState, smallIntConstants.get(FrameSlotKindStaticTag)); - } else { - if (staticMode == FrameDescriptorMixedStaticMode) { - for (int slot = 0; slot < indexedFrameSize; slot++) { - if (indexedFrameSlotKinds[slot] == FrameSlotKindStaticTag) { - indexedTagArrayEntryState[slot] = smallIntConstants.get(FrameSlotKindStaticTag); - } else { - indexedTagArrayEntryState[slot] = smallIntConstants.get(0); - } - } - } - } + Arrays.fill(indexedTagArrayEntryState, smallIntConstants.get(0)); Arrays.fill(indexedPrimitiveArrayEntryState, defaultLong); tool.createVirtualObject((VirtualObjectNode) virtualFrameArrays.get(INDEXED_OBJECT_ARRAY), indexedObjectArrayEntryState, Collections. emptyList(), sourcePosition, false); tool.createVirtualObject((VirtualObjectNode) virtualFrameArrays.get(INDEXED_PRIMITIVE_ARRAY), indexedPrimitiveArrayEntryState, Collections. emptyList(), sourcePosition, diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameAccessFlags.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameAccessFlags.java index 2977fa63160a..70502b9aa5c8 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameAccessFlags.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameAccessFlags.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 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 @@ -42,7 +42,8 @@ public enum VirtualFrameAccessFlags { STATIC_BOTH_UPDATE(Constants.STATIC_UPDATE), // Special flag. - NON_STATIC_NO_SET_TAG_UPDATE(Constants.NON_STATIC_UPDATE_NO_SET_TAG); + NON_STATIC_NO_SET_TAG_UPDATE(Constants.NON_STATIC_UPDATE_NO_SET_TAG), + STATIC_NO_SET_TAG_UPDATE(Constants.STATIC_UPDATE_NO_SET_TAG); private final byte flags; @@ -82,6 +83,7 @@ private static final class Constants { public static final byte NON_STATIC_UPDATE = PRIMITIVE_FLAG | OBJECT_FLAG | SET_TAG_FLAG | UPDATES_FRAME; public static final byte NON_STATIC_UPDATE_NO_SET_TAG = PRIMITIVE_FLAG | OBJECT_FLAG | UPDATES_FRAME; + public static final byte STATIC_UPDATE_NO_SET_TAG = STATIC_FLAG | PRIMITIVE_FLAG | OBJECT_FLAG | UPDATES_FRAME; public static final byte STATIC = STATIC_FLAG | PRIMITIVE_FLAG | OBJECT_FLAG | SET_TAG_FLAG; public static final byte STATIC_PRIMITIVE = STATIC_FLAG | PRIMITIVE_FLAG | SET_TAG_FLAG; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameAccessorNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameAccessorNode.java index 0996fc9f93ca..25aae6f70400 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameAccessorNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameAccessorNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 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 @@ -28,6 +28,7 @@ import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_0; import jdk.graal.compiler.core.common.type.Stamp; +import jdk.graal.compiler.debug.GraalError; import jdk.graal.compiler.graph.NodeClass; import jdk.graal.compiler.nodeinfo.NodeInfo; import jdk.graal.compiler.nodes.ConstantNode; @@ -98,6 +99,14 @@ public final int getAccessTag() { return accessTag; } + /** + * Ensures that static-ness of slots is consistent. That means that static slots should only be + * accessed statically, and non-static slots accessed non-statically. + */ + protected final void ensureStaticSlotAccessConsistency() { + GraalError.guarantee(getFrame().isStatic(getFrameSlotIndex()) == accessFlags.isStatic(), "Inconsistent Static slot usage."); + } + protected final void insertDeoptimization(VirtualizerTool tool) { /* * Escape analysis does not allow insertion of a DeoptimizeNode. We work around this diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameGetNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameGetNode.java index 9440eefe1247..ab9c672df754 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameGetNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameGetNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 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 @@ -85,9 +85,25 @@ public void virtualize(VirtualizerTool tool) { VirtualObjectNode dataVirtual = (VirtualObjectNode) dataAlias; if (frameSlotIndex < tagVirtual.entryCount() && frameSlotIndex < dataVirtual.entryCount()) { + ensureStaticSlotAccessConsistency(); ValueNode actualTag = tool.getEntry(tagVirtual, frameSlotIndex); final boolean staticAccess = accessFlags.isStatic(); - if (!staticAccess && (!actualTag.isConstant() || actualTag.asJavaConstant().asInt() != accessTag)) { + if (staticAccess && accessKind.isPrimitive() && + (actualTag.isConstant() && actualTag.asJavaConstant().asInt() == 0)) { + /* + * Reading a primitive from an uninitialized static slot: return the default + * value for the access kind. Reading an object can go through the regular + * route. + * + * If it were allowed to go through the normal route, the value stored in the + * virtual array will always be an i64, and it would have to be converted to the + * access kind before returns. This shortcut ensures both correctness, and + * removes the need for conversion. + */ + ValueNode dataEntry = ConstantNode.defaultForKind(getStackKind(), graph()); + tool.replaceWith(dataEntry); + return; + } else if (!staticAccess && (!actualTag.isConstant() || actualTag.asJavaConstant().asInt() != accessTag)) { /* * We cannot constant fold the tag-check immediately, so we need to create a * guard comparing the actualTag with the accessTag. diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameIsNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameIsNode.java index ee568a526c38..a2e973ddaf5d 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameIsNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameIsNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 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 @@ -38,12 +38,12 @@ import jdk.graal.compiler.nodes.spi.Virtualizable; import jdk.graal.compiler.nodes.spi.VirtualizerTool; import jdk.graal.compiler.nodes.virtual.VirtualObjectNode; - import jdk.vm.ci.meta.JavaKind; @NodeInfo(cycles = CYCLES_0, size = SIZE_0) public final class VirtualFrameIsNode extends VirtualFrameAccessorNode implements Virtualizable { public static final NodeClass TYPE = NodeClass.create(VirtualFrameIsNode.class); + public static final int STATIC_TAG = NewFrameNode.FrameSlotKindStaticTag; public VirtualFrameIsNode(Receiver frame, int frameSlotIndex, int accessTag, VirtualFrameAccessType type) { super(TYPE, StampFactory.forKind(JavaKind.Boolean), frame, frameSlotIndex, accessTag, type, VirtualFrameAccessFlags.BENIGN); @@ -57,6 +57,11 @@ public void virtualize(VirtualizerTool tool) { VirtualObjectNode tagVirtual = (VirtualObjectNode) tagAlias; if (frameSlotIndex < tagVirtual.entryCount()) { + if (accessTag == STATIC_TAG) { + // Queries the slot kind in the frame descriptor. + tool.replaceWith(getConstant(frame.isStatic(frameSlotIndex) ? 1 : 0)); + return; + } ValueNode actualTag = tool.getEntry(tagVirtual, frameSlotIndex); if (actualTag.isConstant()) { tool.replaceWith(getConstant(actualTag.asJavaConstant().asInt() == accessTag ? 1 : 0)); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameSetNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameSetNode.java index 591acb25c4af..cf46d87c4b16 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameSetNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameSetNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 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 @@ -89,6 +89,7 @@ public void virtualize(VirtualizerTool tool) { VirtualObjectNode dataVirtual = (VirtualObjectNode) dataAlias; if (frameSlotIndex < tagVirtual.entryCount() && frameSlotIndex < dataVirtual.entryCount()) { + ensureStaticSlotAccessConsistency(); if (accessFlags.setsTag()) { if (accessFlags.isStatic()) { tool.setVirtualEntry(tagVirtual, frameSlotIndex, getConstantWithStaticModifier(accessTag)); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/FrameAccessVerificationPhase.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/FrameAccessVerificationPhase.java index fb3189597a9f..3ea01044f1eb 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/FrameAccessVerificationPhase.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/FrameAccessVerificationPhase.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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 @@ -117,7 +117,7 @@ public void swap(byte[] entries, int src, int dst) { private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; private static final byte NONE = (byte) 0xff; - private static final int TYPE_MASK = 0xf; + private static final int TYPE_MASK = 0x0f; private static final int MODE_MASK = 0x30; private static final int MODE_CLEARED = 0x00; @@ -218,7 +218,8 @@ void apply() { if (insertBefore.isAlive()) { StructuredGraph graph = insertBefore.graph(); ConstantNode defaultForKind = ConstantNode.defaultForKind(NewFrameNode.asJavaKind(accessTag), graph); - graph.addBeforeFixed(insertBefore, graph.add(new VirtualFrameSetNode(frame, index, accessTag, defaultForKind, type, VirtualFrameAccessFlags.NON_STATIC_NO_SET_TAG_UPDATE))); + graph.addBeforeFixed(insertBefore, graph.add(new VirtualFrameSetNode(frame, index, accessTag, defaultForKind, type, + frame.isStatic(index) ? VirtualFrameAccessFlags.STATIC_NO_SET_TAG_UPDATE : VirtualFrameAccessFlags.NON_STATIC_NO_SET_TAG_UPDATE))); } } } diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/AbstractInstrumentableBytecodeNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/AbstractInstrumentableBytecodeNode.java index 58df51f5811a..1b64b47e6bc9 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/AbstractInstrumentableBytecodeNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/AbstractInstrumentableBytecodeNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 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 @@ -36,6 +36,7 @@ import com.oracle.truffle.api.interop.NodeLibrary; import com.oracle.truffle.api.library.ExportLibrary; import com.oracle.truffle.api.library.ExportMessage; +import com.oracle.truffle.api.source.SourceSection; import com.oracle.truffle.espresso.EspressoScope; import com.oracle.truffle.espresso.classfile.attributes.Local; import com.oracle.truffle.espresso.descriptors.ByteSequence; @@ -65,6 +66,11 @@ public WrapperNode createWrapper(ProbeNode probeNode) { return new AbstractInstrumentableBytecodeNodeWrapper(this, probeNode); } + @Override + public SourceSection getSourceSection() { + return getRootNode().getSourceSection(); + } + @ExportMessage @SuppressWarnings("static-method") public final boolean hasScope(@SuppressWarnings("unused") Frame frame) { diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/BytecodeNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/BytecodeNode.java index 2bb6d53ea930..2f2a3ac7268c 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/BytecodeNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/BytecodeNode.java @@ -548,7 +548,13 @@ public void checkNoForeignObjectAssumption(StaticObject object) { @Override void initializeFrame(VirtualFrame frame) { + // Before arguments are set up, the frame is in a weird spot in which inspecting the frame + // is pretty meaningless. Return the 'Unknown bci' value to signify that. + setBCI(frame, -1); + // Push frame arguments into locals. initArguments(frame); + // Initialize the BCI slot. + setBCI(frame, 0); } // region OSR support diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoFrame.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoFrame.java index ca77673f0c52..254a445f7e2a 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoFrame.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoFrame.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 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 @@ -310,11 +310,11 @@ public static double getLocalDouble(Frame frame, int localSlot) { // endregion Local accessors static void setBCI(Frame frame, int bci) { - frame.setIntStatic(BCI_SLOT, bci); + frame.setIntStatic(BCI_SLOT, bci + 1); } static int getBCI(Frame frame) { - return frame.getIntStatic(BCI_SLOT); + return frame.getIntStatic(BCI_SLOT) - 1; } public static int startingStackOffset(int maxLocals) { diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoInstrumentableRootNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoInstrumentableRootNode.java index 496814704b88..7bb8e35392e6 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoInstrumentableRootNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoInstrumentableRootNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 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 @@ -34,7 +34,6 @@ */ @GenerateWrapper public abstract class EspressoInstrumentableRootNode extends EspressoInstrumentableNode { - abstract void beforeInstumentation(VirtualFrame frame); abstract Object execute(VirtualFrame frame); diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoRootNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoRootNode.java index 4eb478b60e36..5013cf4ef893 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoRootNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/EspressoRootNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 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 @@ -329,7 +329,6 @@ public Object execute(VirtualFrame frame) { Method method = getMethod(); assert method.isSynchronized(); assert method.getDeclaringKlass().isInitializedOrInitializing() : method.getDeclaringKlass(); - methodNode.beforeInstumentation(frame); StaticObject monitor = method.isStatic() ? /* class */ method.getDeclaringKlass().mirror() : /* receiver */ (StaticObject) frame.getArguments()[0]; @@ -373,7 +372,6 @@ assert getMethod().getDeclaringKlass().isInitializedOrInitializing() || getConte if (usesMonitors()) { initMonitorStack(frame, new MonitorStack()); } - methodNode.beforeInstumentation(frame); return methodNode.execute(frame); } } diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/IntrinsicSubstitutorNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/IntrinsicSubstitutorNode.java index 6a29cb5685eb..cfa259150775 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/IntrinsicSubstitutorNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/IntrinsicSubstitutorNode.java @@ -61,11 +61,6 @@ private IntrinsicSubstitutorNode(IntrinsicSubstitutorNode toSplit) { this.nbSplits = toSplit.nbSplits; } - @Override - void beforeInstumentation(VirtualFrame frame) { - // no op - } - @Override Object execute(VirtualFrame frame) { return substitution.invoke(frame.getArguments()); diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/IntrinsifiedNativeMethodNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/IntrinsifiedNativeMethodNode.java index cffe2f61b772..6e4e30fdf41d 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/IntrinsifiedNativeMethodNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/IntrinsifiedNativeMethodNode.java @@ -40,11 +40,6 @@ final class IntrinsifiedNativeMethodNode extends EspressoInstrumentableRootNodeI this.env = env; } - @Override - void beforeInstumentation(VirtualFrame frame) { - // no op - } - @Override Object execute(VirtualFrame frame) { Object[] args = frame.getArguments(); diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/MethodWithBytecodeNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/MethodWithBytecodeNode.java index a91f9d85592c..4eb28ed1b158 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/MethodWithBytecodeNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/MethodWithBytecodeNode.java @@ -57,11 +57,6 @@ public int getBci(Frame frame) { return bytecodeNode.getBci(frame); } - @Override - void beforeInstumentation(VirtualFrame frame) { - EspressoFrame.setBCI(frame, -1); - } - @Override Object execute(VirtualFrame frame) { bytecodeNode.initializeFrame(frame); diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/NativeMethodNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/NativeMethodNode.java index cf19859f33ab..f5c535d250e3 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/NativeMethodNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/NativeMethodNode.java @@ -96,11 +96,6 @@ private Object[] preprocessArgs(JniEnv env, Object[] args) { return nativeArgs; } - @Override - void beforeInstumentation(VirtualFrame frame) { - // no op - } - @Override public Object execute(VirtualFrame frame) { final JniEnv env = getContext().getJNI(); diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/interop/ToEspressoNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/interop/ToEspressoNode.java index fbd491469983..0f3aebf0cc72 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/interop/ToEspressoNode.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/interop/ToEspressoNode.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 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 @@ -152,11 +152,13 @@ public static boolean isStaticObject(Object value) { @Specialization public Object doStaticObject(StaticObject value, Klass targetType, - @Cached InstanceOf.Dynamic instanceOf) throws UnsupportedTypeException { + @Cached InstanceOf.Dynamic instanceOf, + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { assert !value.isForeignObject(); if (StaticObject.isNull(value) || instanceOf.execute(value.getKlass(), targetType)) { return value; // pass through, NULL coercion not needed. } + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.cat("Cannot cast ", value, " to ", targetType.getTypeAsString())); } @@ -165,7 +167,12 @@ public Object doStaticObject(StaticObject value, Klass targetType, "!isStaticObject(value)" }) public Object doForeignNull(Object value, @SuppressWarnings("unused") Klass targetType, - @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop) { + @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { + if (targetType.isPrimitive()) { + error.enter(this); + throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.cat("Cannot cast ", value, " to ", targetType.getTypeAsString())); + } return StaticObject.createForeignNull(EspressoLanguage.get(this), value); } @@ -176,7 +183,8 @@ public Object doForeignNull(Object value, @SuppressWarnings("unused") Klass targ }) public Object doMappedInterface(Object value, Klass targetType, @Cached LookupProxyKlassNode lookupProxyKlassNode, - @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop) throws UnsupportedTypeException { + @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); WrappedProxyKlass proxyKlass = lookupProxyKlassNode.execute(metaObject, getMetaName(metaObject, interop), targetType); @@ -184,8 +192,10 @@ public Object doMappedInterface(Object value, Klass targetType, targetType.safeInitialize(); return proxyKlass.createProxyInstance(value, getLanguage(), interop); } + error.enter(this); throw new ClassCastException(); } catch (ClassCastException e) { + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getTypeAsString())); } } @@ -195,7 +205,8 @@ public Object doMappedInterface(Object value, Klass targetType, "!isStaticObject(value)" }) public Object doArray(Object value, ArrayKlass targetType, - @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop) throws UnsupportedTypeException { + @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { if (targetType == getMeta()._byte_array) { if (interop.hasBufferElements(value) && !isHostString(value)) { return StaticObject.createForeign(EspressoLanguage.get(this), getMeta()._byte_array, value, interop); @@ -204,6 +215,7 @@ public Object doArray(Object value, ArrayKlass targetType, if (interop.hasArrayElements(value) && !isHostString(value)) { return StaticObject.createForeign(EspressoLanguage.get(this), targetType, value, interop); } + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, targetType.getTypeAsString()); } @@ -214,7 +226,8 @@ public Object doArray(Object value, ArrayKlass targetType, }) public Object doTypeConverter(Object value, Klass targetType, @Cached LookupTypeConverterNode lookupTypeConverter, - @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop) throws UnsupportedTypeException { + @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); String metaName = getMetaName(metaObject, interop); @@ -224,8 +237,10 @@ public Object doTypeConverter(Object value, Klass targetType, if (converter != null) { return converter.convert(StaticObject.createForeign(getLanguage(), targetType, value, interop)); } + error.enter(this); throw new ClassCastException(); } catch (ClassCastException e) { + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getNameAsString(), e.getMessage())); } } @@ -238,7 +253,8 @@ public Object doTypeConverter(Object value, Klass targetType, public Object doInternalTypeConverter(Object value, Klass targetType, @Cached ToReference.DynamicToReference converterToEspresso, @Cached LookupInternalTypeConverterNode lookupInternalTypeConverter, - @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop) throws UnsupportedTypeException { + @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); String metaName = getMetaName(metaObject, interop); @@ -248,8 +264,10 @@ public Object doInternalTypeConverter(Object value, Klass targetType, if (converter != null) { return converter.convertInternal(interop, value, getMeta(), converterToEspresso); } + error.enter(this); throw new ClassCastException(); } catch (ClassCastException e) { + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getNameAsString(), e.getMessage())); } } diff --git a/truffle/CHANGELOG.md b/truffle/CHANGELOG.md index 2f457502a9cc..3456468d1e8a 100644 --- a/truffle/CHANGELOG.md +++ b/truffle/CHANGELOG.md @@ -8,6 +8,7 @@ This changelog summarizes major changes between Truffle versions relevant to lan * GR-51385 Added an instrumentation filter to include available source sections only: `SourceSectionFilter.Builder#sourceSectionAvailableOnly(boolean)` * GR-51385 Added a debugger filter to suspend in available source sections only: `SuspensionFilter.Builder#sourceSectionAvailableOnly(boolean)`. * GR-52443 Removed many deprecated `DynamicObject` APIs, deprecated since 22.2 or earlier (`Shape` methods: `addProperty`, `defineProperty`, `removeProperty`, `replaceProperty`, `newInstance`, `createFactory`, `getObjectType`, `changeType`, `getLayout`, `getMutex`, `getParent`, `allocator`, and related interfaces; `Property.set*`, `*Location` methods: `getInternal`, `setInternal`, `set*`, `getType`, and `ObjectLocation`). +* GR-51136 Uninitialized static slots of a `Frame` can now be read, and returns the default value for the access kind. ## Version 24.0.0 diff --git a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/IndexedFrameTest.java b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/IndexedFrameTest.java index dc5e5be7e04e..e3326481e405 100644 --- a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/IndexedFrameTest.java +++ b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/IndexedFrameTest.java @@ -79,14 +79,17 @@ public Object execute(VirtualFrame frame) { } private static void assertIsType(Frame frame, int slot, FrameSlotKind k) { - assertEquals(k == FrameSlotKind.Boolean, frame.isBoolean(slot)); - assertEquals(k == FrameSlotKind.Byte, frame.isByte(slot)); - assertEquals(k == FrameSlotKind.Int, frame.isInt(slot)); - assertEquals(k == FrameSlotKind.Long, frame.isLong(slot)); - assertEquals(k == FrameSlotKind.Double, frame.isDouble(slot)); - assertEquals(k == FrameSlotKind.Float, frame.isFloat(slot)); - assertEquals(k == FrameSlotKind.Object, frame.isObject(slot)); - assertEquals(k == FrameSlotKind.Static, frame.isStatic(slot)); + if (k == FrameSlotKind.Static) { + assertTrue(frame.isStatic(slot)); + } else { + assertEquals(k == FrameSlotKind.Boolean, frame.isBoolean(slot)); + assertEquals(k == FrameSlotKind.Byte, frame.isByte(slot)); + assertEquals(k == FrameSlotKind.Int, frame.isInt(slot)); + assertEquals(k == FrameSlotKind.Long, frame.isLong(slot)); + assertEquals(k == FrameSlotKind.Double, frame.isDouble(slot)); + assertEquals(k == FrameSlotKind.Float, frame.isFloat(slot)); + assertEquals(k == FrameSlotKind.Object, frame.isObject(slot)); + } } @Test @@ -147,6 +150,12 @@ public void testInitialStaticSlotValue() { for (int slot = 0; slot < descriptor.getNumberOfSlots(); slot++) { assertIsType(frame, slot, FrameSlotKind.Static); assertSame(DEFAULT, frame.getObjectStatic(slot)); + assertEquals((byte) 0, frame.getByteStatic(slot)); + assertEquals(false, frame.getBooleanStatic(slot)); + assertEquals(0, frame.getIntStatic(slot)); + assertEquals(0.0f, frame.getFloatStatic(slot), 0); + assertEquals(0L, frame.getLongStatic(slot)); + assertEquals(0.0d, frame.getDoubleStatic(slot), 0); } }); } diff --git a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/SubprocessTestUtils.java b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/SubprocessTestUtils.java index aced17577240..26cc492c92cc 100644 --- a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/SubprocessTestUtils.java +++ b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/SubprocessTestUtils.java @@ -67,13 +67,6 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; -import com.sun.tools.attach.VirtualMachine; -import com.sun.tools.attach.VirtualMachineDescriptor; -import org.graalvm.nativeimage.ImageInfo; -import org.junit.Assert; -import org.junit.Assume; -import org.junit.Test; - import javax.management.MBeanServerConnection; import javax.management.ObjectName; import javax.management.openmbean.CompositeData; @@ -81,6 +74,14 @@ import javax.management.remote.JMXConnectorFactory; import javax.management.remote.JMXServiceURL; +import org.graalvm.nativeimage.ImageInfo; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.Test; + +import com.sun.tools.attach.VirtualMachine; +import com.sun.tools.attach.VirtualMachineDescriptor; + /** * Support for executing Truffle tests in a sub-process with filtered compilation failure options. * This support is useful for tests that explicitly set @@ -169,6 +170,31 @@ public static Subprocess executeInSubprocess(Class testClass, Runnable action return process.get(); } + /** + * Executes action in a sub-process with filtered compilation failure options. Also disables + * assertions for the classes in {@code daClasses} + * + * @param testClass the test enclosing class. + * @param action the test to execute. + * @param daClasses the classes whose assertions should be disabled. + * @param additionalVmOptions additional vm option added to java arguments. Prepend + * {@link #TO_REMOVE_PREFIX} to remove item from existing vm options. + * @return {@link Subprocess} if it's called by a test that is not executing in a sub-process. + * Returns {@code null} for a caller run in a sub-process. + * @see SubprocessTestUtils + */ + public static Subprocess executeInSubprocessWithAssertionsDisabled(Class testClass, Runnable action, boolean failOnNonZeroExitCode, List> daClasses, String... additionalVmOptions) + throws IOException, InterruptedException { + String[] vmOptionsWithAssertionsDisabled = getAssertionsDisabledOptions(daClasses); + AtomicReference process = new AtomicReference<>(); + newBuilder(testClass, action).failOnNonZeroExit(failOnNonZeroExitCode).prefixVmOption(additionalVmOptions).postfixVmOption(vmOptionsWithAssertionsDisabled).onExit((p) -> process.set(p)).run(); + return process.get(); + } + + private static String[] getAssertionsDisabledOptions(List> daClasses) { + return daClasses.stream().map((c) -> "-da:" + c.getName()).toArray(String[]::new); + } + /** * Returns {@code true} if it's called by a test that is already executing in a sub-process. */ diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/Frame.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/Frame.java index 1dbc66e9f4dd..4d00b8131709 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/Frame.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/Frame.java @@ -300,6 +300,8 @@ default byte getTag(int slot) { /** * Check whether the given indexed slot is of type object. + *

+ * This method should not be used with static slots. * * @param slot the slot of the local variable * @since 22.0 @@ -311,7 +313,9 @@ default boolean isObject(int slot) { /** * Check whether the given indexed slot is of type byte. - * + *

+ * This method should not be used with static slots. + * * @param slot the slot of the local variable * @since 22.0 */ @@ -322,7 +326,9 @@ default boolean isByte(int slot) { /** * Check whether the given indexed slot is of type boolean. - * + *

+ * This method should not be used with static slots. + * * @param slot the slot of the local variable * @since 22.0 */ @@ -333,7 +339,9 @@ default boolean isBoolean(int slot) { /** * Check whether the given indexed slot is of type int. - * + *

+ * This method should not be used with static slots. + * * @param slot the slot of the local variable * @since 22.0 */ @@ -344,7 +352,9 @@ default boolean isInt(int slot) { /** * Check whether the given indexed slot is of type long. - * + *

+ * This method should not be used with static slots. + * * @param slot the slot of the local variable * @since 22.0 */ @@ -355,7 +365,9 @@ default boolean isLong(int slot) { /** * Check whether the given indexed slot is of type float. - * + *

+ * This method should not be used with static slots. + * * @param slot the slot of the local variable * @since 22.0 */ @@ -366,7 +378,9 @@ default boolean isFloat(int slot) { /** * Check whether the given indexed slot is of type double. - * + *

+ * This method should not be used with static slots. + * * @param slot the slot of the local variable * @since 22.0 */ @@ -377,7 +391,9 @@ default boolean isDouble(int slot) { /** * Checks whether the given indexed slot is static. - * + *

+ * This method should not be used with static slots. + * * @param slot the slot of the local variable * @since 22.2 */ @@ -436,7 +452,7 @@ default void setAuxiliarySlot(int slot, Object value) { * {@link Object}. * * @param slot the slot of the local variable - * @return the current value of the local variable + * @return the current value of the local variable, or {@code null} if slot is uninitialized. * @since 22.2 */ default Object getObjectStatic(int slot) { @@ -464,7 +480,7 @@ default void setObjectStatic(int slot, Object value) { * implementations have to guarantee that the variable in the given slot is of type byte. * * @param slot the slot of the local variable - * @return the current value of the local variable + * @return the current value of the local variable, or {@code 0} if slot is uninitialized. * @since 22.2 */ default byte getByteStatic(int slot) { @@ -492,7 +508,7 @@ default void setByteStatic(int slot, byte value) { * implementations have to guarantee that the variable in the given slot is of type boolean. * * @param slot the slot of the local variable - * @return the current value of the local variable + * @return the current value of the local variable, or {@code false} if slot is uninitialized. * @since 22.2 */ default boolean getBooleanStatic(int slot) { @@ -520,7 +536,7 @@ default void setBooleanStatic(int slot, boolean value) { * implementations have to guarantee that the variable in the given slot can is of type int. * * @param slot the slot of the local variable - * @return the current value of the local variable + * @return the current value of the local variable, or {@code 0} if slot is uninitialized. * @since 22.2 */ default int getIntStatic(int slot) { @@ -548,7 +564,7 @@ default void setIntStatic(int slot, int value) { * implementations have to guarantee that the variable in the given slot is of type long. * * @param slot the slot of the local variable - * @return the current value of the local variable + * @return the current value of the local variable, or {@code 0} if slot is uninitialized. * @since 22.2 */ default long getLongStatic(int slot) { @@ -576,7 +592,7 @@ default void setLongStatic(int slot, long value) { * implementations have to guarantee that the variable in the given slot is of type float. * * @param slot the slot of the local variable - * @return the current value of the local variable + * @return the current value of the local variable, or {@code 0} if slot is uninitialized. * @since 22.2 */ default float getFloatStatic(int slot) { @@ -604,7 +620,7 @@ default void setFloatStatic(int slot, float value) { * implementations have to guarantee that the variable in the given slot is of type double. * * @param slot the slot of the local variable - * @return the current value of the local variable + * @return the current value of the local variable, or {@code 0} if slot is uninitialized. * @since 22.2 */ default double getDoubleStatic(int slot) { diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/FrameAccessor.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/FrameAccessor.java index 826e8854fa3a..7dba0769ee44 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/FrameAccessor.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/FrameAccessor.java @@ -56,15 +56,5 @@ public void markMaterializeCalled(FrameDescriptor descriptor) { public boolean getMaterializeCalled(FrameDescriptor descriptor) { return descriptor.materializeCalled; } - - @Override - public boolean usesAllStaticMode(FrameDescriptor descriptor) { - return descriptor.staticMode == FrameDescriptor.ALL_STATIC_MODE; - } - - @Override - public boolean usesMixedStaticMode(FrameDescriptor descriptor) { - return descriptor.staticMode == FrameDescriptor.MIXED_STATIC_MODE; - } } } diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/FrameDescriptor.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/FrameDescriptor.java index c32608da426a..1cad0185e730 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/FrameDescriptor.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/frame/FrameDescriptor.java @@ -60,21 +60,30 @@ * @since 0.8 or earlier */ public final class FrameDescriptor implements Cloneable { - /* - * Changing these constants implies changes in NewFrameNode.java as well: + /** + * @since 22.2 + * @deprecated since 24.1 */ + @Deprecated static final int NO_STATIC_MODE = 1; /** * @since 22.2 + * @deprecated since 24.1 */ - static final int NO_STATIC_MODE = 1; + @Deprecated static final int ALL_STATIC_MODE = 2; /** * @since 22.2 + * @deprecated since 24.1 */ - static final int ALL_STATIC_MODE = 2; + @Deprecated static final int MIXED_STATIC_MODE = NO_STATIC_MODE | ALL_STATIC_MODE; + /** + * Flag that defines the assignment strategy of initial {@link FrameSlotKind}s to slots in a + * frame. + * * @since 22.2 + * @deprecated since 24.1 */ - static final int MIXED_STATIC_MODE = NO_STATIC_MODE | ALL_STATIC_MODE; + @Deprecated final int staticMode = MIXED_STATIC_MODE; private final Object defaultValue; @@ -107,14 +116,6 @@ public final class FrameDescriptor implements Cloneable { */ boolean materializeCalled; - /** - * Flag that defines the assignment strategy of initial {@link FrameSlotKind}s to slots in a - * frame. - * - * @since 22.2 - */ - final int staticMode; - private static final String NEVER_PART_OF_COMPILATION_MESSAGE = "interpreter-only. includes hashmap operations."; private static final byte[] EMPTY_BYTE_ARRAY = {}; @@ -140,18 +141,16 @@ public FrameDescriptor(Object defaultValue) { this.indexedSlotNames = null; this.indexedSlotInfos = null; this.descriptorInfo = null; - this.staticMode = NO_STATIC_MODE; this.defaultValue = defaultValue; } - private FrameDescriptor(Object defaultValue, byte[] indexedSlotTags, Object[] indexedSlotNames, Object[] indexedSlotInfos, Object info, int staticMode) { + private FrameDescriptor(Object defaultValue, byte[] indexedSlotTags, Object[] indexedSlotNames, Object[] indexedSlotInfos, Object info) { CompilerAsserts.neverPartOfCompilation("do not create a FrameDescriptor from compiled code"); this.indexedSlotTags = indexedSlotTags; this.indexedSlotNames = indexedSlotNames; this.indexedSlotInfos = indexedSlotInfos; this.descriptorInfo = info; - this.staticMode = staticMode; this.defaultValue = defaultValue; } @@ -167,7 +166,7 @@ public FrameDescriptor copy() { CompilerAsserts.neverPartOfCompilation(NEVER_PART_OF_COMPILATION_MESSAGE); synchronized (this) { FrameDescriptor clonedFrameDescriptor = new FrameDescriptor(this.defaultValue, indexedSlotTags == null ? null : indexedSlotTags.clone(), - indexedSlotNames == null ? null : indexedSlotNames.clone(), indexedSlotInfos == null ? null : indexedSlotInfos.clone(), descriptorInfo, staticMode); + indexedSlotNames == null ? null : indexedSlotNames.clone(), indexedSlotInfos == null ? null : indexedSlotInfos.clone(), descriptorInfo); clonedFrameDescriptor.auxiliarySlotCount = auxiliarySlotCount; clonedFrameDescriptor.activeAuxiliarySlotCount = activeAuxiliarySlotCount; if (auxiliarySlotMap != null) { @@ -437,7 +436,6 @@ public static final class Builder { private Object[] infos; private int size; private Object descriptorInfo; - private int staticMode; private Builder(int capacity) { this.tags = new byte[capacity]; @@ -484,7 +482,6 @@ public int addSlots(int count, FrameSlotKind kind) { Arrays.fill(tags, size, size + count, kind.tag); int newIndex = size; size += count; - staticMode |= (kind == FrameSlotKind.Static ? ALL_STATIC_MODE : NO_STATIC_MODE); return newIndex; } @@ -517,7 +514,6 @@ public int addSlot(FrameSlotKind kind, Object name, Object info) { tags[size] = kind.tag; int newIndex = size; size++; - staticMode |= (kind == FrameSlotKind.Static ? ALL_STATIC_MODE : NO_STATIC_MODE); return newIndex; } @@ -543,8 +539,7 @@ public Builder info(Object info) { * @since 22.0 */ public FrameDescriptor build() { - return new FrameDescriptor(defaultValue, Arrays.copyOf(tags, size), names == null ? null : Arrays.copyOf(names, size), infos == null ? null : Arrays.copyOf(infos, size), descriptorInfo, - staticMode != 0 ? staticMode : NO_STATIC_MODE); + return new FrameDescriptor(defaultValue, Arrays.copyOf(tags, size), names == null ? null : Arrays.copyOf(names, size), infos == null ? null : Arrays.copyOf(infos, size), descriptorInfo); } } } diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java index a445e6b6b5b8..9c721507c6e7 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java @@ -1032,10 +1032,6 @@ protected FrameSupport() { public abstract void markMaterializeCalled(FrameDescriptor descriptor); public abstract boolean getMaterializeCalled(FrameDescriptor descriptor); - - public abstract boolean usesAllStaticMode(FrameDescriptor descriptor); - - public abstract boolean usesMixedStaticMode(FrameDescriptor descriptor); } public abstract static class ExceptionSupport extends Support { diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/FrameWithoutBoxing.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/FrameWithoutBoxing.java index 1fd58aef4bbe..80aa911e6e83 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/FrameWithoutBoxing.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/FrameWithoutBoxing.java @@ -162,7 +162,6 @@ public FrameWithoutBoxing(FrameDescriptor descriptor, Object[] arguments) { final int indexedSize = descriptor.getNumberOfSlots(); final int auxiliarySize = descriptor.getNumberOfAuxiliarySlots(); Object defaultValue = descriptor.getDefaultValue(); - final Accessor.FrameSupport frameSupport = DefaultRuntimeAccessor.FRAME; final Object[] indexedLocalsArray; final long[] indexedPrimitiveLocalsArray; final byte[] indexedTagsArray; @@ -177,16 +176,9 @@ public FrameWithoutBoxing(FrameDescriptor descriptor, Object[] arguments) { Arrays.fill(indexedLocalsArray, defaultValue); } indexedPrimitiveLocalsArray = new long[indexedSize]; + // Do not initialize tags, even for static slots. In practice, this means that it is + // possible to statically access uninitialized slots. indexedTagsArray = new byte[indexedSize]; - if (frameSupport.usesAllStaticMode(descriptor)) { - Arrays.fill(indexedTagsArray, STATIC_TAG); - } else if (frameSupport.usesMixedStaticMode(descriptor)) { - for (int slot = 0; slot < indexedTagsArray.length; slot++) { - if (descriptor.getSlotKind(slot) == FrameSlotKind.Static) { - indexedTagsArray[slot] = STATIC_TAG; - } - } - } } if (auxiliarySize == 0) { auxiliarySlotsArray = EMPTY_OBJECT_ARRAY; @@ -244,7 +236,7 @@ public byte getTag(int slotIndex) { } private boolean isNonStaticType(int slotIndex, byte tag) { - assert tag < STATIC_TAG : tag; + assert !isStatic(slotIndex) : "Using isType on static slots is to be avoided."; return getIndexedTags()[slotIndex] == tag; } @@ -276,7 +268,7 @@ private static void unsafePutObject(Object receiver, long offset, Object value, @Override public Object getValue(int slot) { byte tag = getTag(slot); - assert (indexedTags[slot] & STATIC_TAG) == 0 : UNEXPECTED_NON_STATIC_READ; + assert !isStatic(slot) : UNEXPECTED_NON_STATIC_READ; switch (tag) { case BOOLEAN_TAG: return getBoolean(slot); @@ -484,7 +476,8 @@ public boolean isDouble(int slot) { @Override public boolean isStatic(int slot) { - return getTag(slot) == STATIC_TAG; + // Frame descriptor holds the definitive answer. + return getFrameDescriptor().getSlotKind(slot) == FrameSlotKind.Static; } @Override @@ -512,14 +505,14 @@ public Object getAuxiliarySlot(int slot) { @Override public Object getObjectStatic(int slot) { - assert indexedTags[slot] == STATIC_OBJECT_TAG : "Unexpected read of static object value"; + assert checkStaticGet(slot, STATIC_OBJECT_TAG) : "Unexpected read of static object value"; return getIndexedLocals()[slot]; } @Override public void setObjectStatic(int slot, Object value) { - assert (indexedTags[slot] & STATIC_TAG) != 0 : UNEXPECTED_STATIC_WRITE; + assert checkStatic(slot) : UNEXPECTED_STATIC_WRITE; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_OBJECT_TAG; @@ -530,14 +523,14 @@ public void setObjectStatic(int slot, Object value) { @Override public byte getByteStatic(int slot) { - assert indexedTags[slot] == STATIC_BYTE_TAG : "Unexpected read of static byte value"; + assert checkStaticGet(slot, STATIC_BYTE_TAG) : "Unexpected read of static byte value"; return (byte) narrow(getIndexedPrimitiveLocals()[slot]); } @Override public void setByteStatic(int slot, byte value) { - assert (indexedTags[slot] & STATIC_TAG) != 0 : UNEXPECTED_STATIC_WRITE; + assert checkStatic(slot) : UNEXPECTED_STATIC_WRITE; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_BYTE_TAG; @@ -548,14 +541,14 @@ public void setByteStatic(int slot, byte value) { @Override public boolean getBooleanStatic(int slot) { - assert indexedTags[slot] == STATIC_BOOLEAN_TAG : "Unexpected read of static boolean value"; + assert checkStaticGet(slot, STATIC_BOOLEAN_TAG) : "Unexpected read of static boolean value"; return narrow(getIndexedPrimitiveLocals()[slot]) != 0; } @Override public void setBooleanStatic(int slot, boolean value) { - assert (indexedTags[slot] & STATIC_TAG) != 0 : UNEXPECTED_STATIC_WRITE; + assert checkStatic(slot) : UNEXPECTED_STATIC_WRITE; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_BOOLEAN_TAG; @@ -566,14 +559,14 @@ public void setBooleanStatic(int slot, boolean value) { @Override public int getIntStatic(int slot) { - assert indexedTags[slot] == STATIC_INT_TAG : "Unexpected read of static int value"; + assert checkStaticGet(slot, STATIC_INT_TAG) : "Unexpected read of static int value"; return narrow(getIndexedPrimitiveLocals()[slot]); } @Override public void setIntStatic(int slot, int value) { - assert (indexedTags[slot] & STATIC_TAG) != 0 : UNEXPECTED_STATIC_WRITE; + assert checkStatic(slot) : UNEXPECTED_STATIC_WRITE; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_INT_TAG; @@ -584,14 +577,14 @@ public void setIntStatic(int slot, int value) { @Override public long getLongStatic(int slot) { - assert indexedTags[slot] == STATIC_LONG_TAG : "Unexpected read of static long value"; + assert checkStaticGet(slot, STATIC_LONG_TAG) : "Unexpected read of static long value"; return getIndexedPrimitiveLocals()[slot]; } @Override public void setLongStatic(int slot, long value) { - assert (indexedTags[slot] & STATIC_TAG) != 0 : UNEXPECTED_STATIC_WRITE; + assert checkStatic(slot) : UNEXPECTED_STATIC_WRITE; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_LONG_TAG; @@ -602,14 +595,14 @@ public void setLongStatic(int slot, long value) { @Override public float getFloatStatic(int slot) { - assert indexedTags[slot] == STATIC_FLOAT_TAG : "Unexpected read of static float value"; + assert checkStaticGet(slot, STATIC_FLOAT_TAG) : "Unexpected read of static float value"; return Float.intBitsToFloat(narrow(getIndexedPrimitiveLocals()[slot])); } @Override public void setFloatStatic(int slot, float value) { - assert (indexedTags[slot] & STATIC_TAG) != 0 : UNEXPECTED_STATIC_WRITE; + assert checkStatic(slot) : UNEXPECTED_STATIC_WRITE; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_FLOAT_TAG; @@ -620,14 +613,14 @@ public void setFloatStatic(int slot, float value) { @Override public double getDoubleStatic(int slot) { - assert indexedTags[slot] == STATIC_DOUBLE_TAG : "Unexpected read of static double value"; + assert checkStaticGet(slot, STATIC_DOUBLE_TAG) : "Unexpected read of static double value"; return Double.longBitsToDouble(getIndexedPrimitiveLocals()[slot]); } @Override public void setDoubleStatic(int slot, double value) { - assert (indexedTags[slot] & STATIC_TAG) != 0 : UNEXPECTED_STATIC_WRITE; + assert checkStatic(slot) : UNEXPECTED_STATIC_WRITE; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_DOUBLE_TAG; @@ -638,7 +631,7 @@ public void setDoubleStatic(int slot, double value) { @Override public void copyPrimitiveStatic(int srcSlot, int destSlot) { - assert indexedTags[srcSlot] > STATIC_TAG && (indexedTags[destSlot] & STATIC_TAG) != 0 : "Unexpected copy of static primitive value "; + assert checkStaticPrimitive(srcSlot) && checkStatic(destSlot) : "Unexpected copy of static primitive value "; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[destSlot] = indexedTags[srcSlot]; @@ -650,7 +643,7 @@ public void copyPrimitiveStatic(int srcSlot, int destSlot) { @Override public void copyObjectStatic(int srcSlot, int destSlot) { - assert (indexedTags[srcSlot] == STATIC_OBJECT_TAG || indexedTags[srcSlot] == STATIC_ILLEGAL_TAG) && (indexedTags[destSlot] & STATIC_TAG) != 0 : "Unexpected copy of static object value"; + assert checkStaticObject(srcSlot) && checkStatic(destSlot) : "Unexpected copy of static object value"; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[destSlot] = indexedTags[srcSlot]; @@ -662,7 +655,7 @@ public void copyObjectStatic(int srcSlot, int destSlot) { @Override public void copyStatic(int srcSlot, int destSlot) { - assert indexedTags[srcSlot] >= STATIC_TAG && indexedTags[destSlot] >= STATIC_TAG : "Unexpected copy of static value"; + assert checkStatic(srcSlot) && checkStatic(destSlot) : "Unexpected copy of static value"; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[destSlot] = indexedTags[srcSlot]; @@ -676,7 +669,7 @@ public void copyStatic(int srcSlot, int destSlot) { @Override public void swapPrimitiveStatic(int first, int second) { - assert indexedTags[first] > STATIC_TAG && indexedTags[second] > STATIC_TAG : "Unexpected swap of static primitive value"; + assert checkStaticPrimitive(first) && checkStaticPrimitive(second) : "Unexpected swap of static primitive value"; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { final byte swapTag = indexedTags[first]; @@ -694,8 +687,8 @@ public void swapPrimitiveStatic(int first, int second) { @Override public void swapObjectStatic(int first, int second) { - assert (indexedTags[first] == STATIC_OBJECT_TAG || indexedTags[first] == STATIC_ILLEGAL_TAG) && - (indexedTags[second] == STATIC_OBJECT_TAG || indexedTags[second] == STATIC_ILLEGAL_TAG) : "Unexpected swap of static object value"; + assert checkStaticObject(first) && + checkStaticObject(second) : "Unexpected swap of static object value"; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { final byte swapTag = indexedTags[first]; @@ -713,7 +706,7 @@ public void swapObjectStatic(int first, int second) { @Override public void swapStatic(int first, int second) { - assert indexedTags[first] >= STATIC_TAG && indexedTags[second] >= STATIC_TAG : "Unexpected swap of static value"; + assert checkStatic(first) && checkStatic(second) : "Unexpected swap of static value"; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { final byte swapTag = indexedTags[first]; @@ -736,7 +729,7 @@ public void swapStatic(int first, int second) { @Override public void clearPrimitiveStatic(int slot) { - assert indexedTags[slot] > STATIC_TAG : "Unexpected clear of static primitive value"; + assert checkStaticPrimitive(slot) : "Unexpected clear of static primitive value"; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_ILLEGAL_TAG; @@ -750,7 +743,7 @@ public void clearPrimitiveStatic(int slot) { @Override public void clearObjectStatic(int slot) { - assert indexedTags[slot] == STATIC_OBJECT_TAG || indexedTags[slot] == STATIC_ILLEGAL_TAG : "Unexpected clear of static object value"; + assert checkStaticObject(slot) : "Unexpected clear of static object value"; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_ILLEGAL_TAG; @@ -761,7 +754,7 @@ public void clearObjectStatic(int slot) { @Override public void clearStatic(int slot) { - assert indexedTags[slot] >= STATIC_TAG : "Unexpected clear of static value"; + assert checkStatic(slot) : "Unexpected clear of static value"; // We use this check instead of the assert keyword to update the tags in PE'd code. if (ASSERTIONS_ENABLED) { indexedTags[slot] = STATIC_ILLEGAL_TAG; @@ -774,6 +767,56 @@ public void clearStatic(int slot) { getIndexedLocals()[slot] = null; } + /* + * Implementation details for static slots tag handling: + * + * Static slots tags are not initialized to the STATIC_TAG value, but are initially left to 0. + * The first write to a static slot will (if checks are enabled) set the tag to its + * corresponding static tag. Note that this means the tag value in the frame itself is not + * reliable for determining if a slot is static, but instead the frame descriptor should be + * queried. + * + * Much like regular slots can be read when not yet written to, static slots can be read when + * not yet initialized (tag == 0), and will return the default value associated with the read + * (frameDescriptor.defaultValue() for reading an object, 0 for reading a primitive). + */ + + private boolean checkStaticGet(int slot, byte tag) { + byte frameTag = indexedTags[slot]; + if (frameTag == 0) { + // Uninitialized tag, allow static reading iff frame descriptor declares it static. + return isStatic(slot); + } + return frameTag == tag; + } + + private boolean checkStatic(int slot) { + byte frameTag = indexedTags[slot]; + if (frameTag == 0) { + // Uninitialized tag, allow static writing iff frame descriptor declares it static. + return isStatic(slot); + } + return frameTag >= STATIC_TAG; + } + + private boolean checkStaticPrimitive(int slot) { + byte frameTag = indexedTags[slot]; + if (frameTag == 0) { + // Uninitialized tag, allow static reading iff frame descriptor declares it static. + return isStatic(slot); + } + return frameTag > STATIC_TAG; + } + + private boolean checkStaticObject(int slot) { + byte frameTag = indexedTags[slot]; + if (frameTag == 0) { + // Uninitialized tag, allow static reading iff frame descriptor declares it static. + return isStatic(slot); + } + return frameTag == STATIC_OBJECT_TAG || frameTag == STATIC_ILLEGAL_TAG; + } + /** * Marker method to be called before performing a frame transfer. */ diff --git a/truffle/src/com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/BytecodeOSRMetadata.java b/truffle/src/com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/BytecodeOSRMetadata.java index 317d5cd1633a..9c54a67844b9 100644 --- a/truffle/src/com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/BytecodeOSRMetadata.java +++ b/truffle/src/com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/BytecodeOSRMetadata.java @@ -202,7 +202,26 @@ private void updateFrameSlots(FrameWithoutBoxing frame, OsrEntryDescription osrE // smaller than the number of slots. Copy it into an array with the correct size. osrEntry.indexedFrameTags = new byte[state.frameDescriptor.getNumberOfSlots()]; for (int i = 0; i < osrEntry.indexedFrameTags.length; i++) { - osrEntry.indexedFrameTags[i] = frame.getTag(i); + if (frame.isStatic(i)) { + /* + * Static check goes through the frame descriptor, which is more precise + * than the tag in case of uninitialized slot. The tag is forced to be + * STATIC, effectively disallowing the OSR frame to have uninitialized + * static slots. This solves a couple effects: + * + * - Uninitialized slots would get tag == 0, which, in the frame transfer + * loop would resolve as a non-static OBJECT access. Forcing them to be + * STATIC here solves that issue. + * + * - Uninitialized slots have a Long(0) in their slot, which causes problems + * when their virtualization returns them as-is. Static slots used in OSR + * forces the conversion to the requested kind anyway: we can take advantage + * of that behavior here to return the correct 0 value for all slot kinds. + */ + osrEntry.indexedFrameTags[i] = FrameWithoutBoxing.STATIC_TAG; + } else { + osrEntry.indexedFrameTags[i] = frame.getTag(i); + } } } }); @@ -508,6 +527,13 @@ private static void transferLoop( byte actualTag = source.getTag(i); byte expectedTag = expectedTags == null ? actualTag : expectedTags[i]; + if (expectedTag == FrameWithoutBoxing.STATIC_TAG) { + // Here we can skip incompatible tag check in case of static slot since the frame + // descriptor has not changed (checked as pre-condition in 'validateDescriptor'). + // This allows uninitialized static slots (tag == 0) to pass through (and transfer + // with 'tag == STATIC') + actualTag = FrameWithoutBoxing.STATIC_TAG; + } boolean incompatibleTags = expectedTag != actualTag; if (incompatibleTags) { // The tag for this slot may have changed; if so, deoptimize and update it. diff --git a/truffle/src/com.oracle.truffle.tck.instrumentation/src/com/oracle/truffle/tck/instrumentation/VerifierInstrument.java b/truffle/src/com.oracle.truffle.tck.instrumentation/src/com/oracle/truffle/tck/instrumentation/VerifierInstrument.java index 94c916ed29e8..41a7ff7a32fd 100644 --- a/truffle/src/com.oracle.truffle.tck.instrumentation/src/com/oracle/truffle/tck/instrumentation/VerifierInstrument.java +++ b/truffle/src/com.oracle.truffle.tck.instrumentation/src/com/oracle/truffle/tck/instrumentation/VerifierInstrument.java @@ -268,7 +268,12 @@ private static void checkFrameIsEmpty(EventContext context, MaterializedFrame fr node.getRootNode().getFrameDescriptor() == frame.getFrameDescriptor()) { Object defaultValue = frame.getFrameDescriptor().getDefaultValue(); for (int slot = 0; slot < frame.getFrameDescriptor().getNumberOfSlots(); slot++) { - Assert.assertEquals("Top-most nodes tagged with RootTag should have clean frames.", defaultValue, frame.getValue(slot)); + if (frame.isStatic(slot)) { + Assert.assertEquals("Top-most nodes tagged with RootTag should have clean frames.", defaultValue, frame.getObjectStatic(slot)); + Assert.assertEquals("Top-most nodes tagged with RootTag should have clean frames.", 0L, frame.getLongStatic(slot)); + } else { + Assert.assertEquals("Top-most nodes tagged with RootTag should have clean frames.", defaultValue, frame.getValue(slot)); + } } } }