From bf8c663e8521b0b6e4f6ed1dc221979d84505f81 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Tue, 21 Nov 2023 19:57:47 +0100 Subject: [PATCH 01/27] Fix 2 bugs: - A foreign null does not succeed a cast to primitive - Bytecode node get their source section from their root. --- .../espresso/nodes/AbstractInstrumentableBytecodeNode.java | 6 ++++++ .../truffle/espresso/nodes/interop/ToEspressoNode.java | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) 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..2668c2f63638 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 @@ -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/interop/ToEspressoNode.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/interop/ToEspressoNode.java index fbd491469983..6df14b219839 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 @@ -165,7 +165,10 @@ 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) throws UnsupportedTypeException { + if (targetType.isPrimitive()) { + throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.cat("Cannot cast ", value, " to ", targetType.getTypeAsString())); + } return StaticObject.createForeignNull(EspressoLanguage.get(this), value); } From 96a8933055b3ebbb643a2988c0ec0f4032bf448e Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Tue, 21 Nov 2023 19:58:22 +0100 Subject: [PATCH 02/27] Frame.getValue(slot) does not work with static slots. For verifying purposes, trust static slot usages. --- .../truffle/tck/instrumentation/VerifierInstrument.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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..5e544f67b070 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,9 @@ 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)) { // Trust static slot usages. + Assert.assertEquals("Top-most nodes tagged with RootTag should have clean frames.", defaultValue, frame.getValue(slot)); + } } } } From 74525cb9c8a63841fef4ba028fcaad61cbc5d349 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Mon, 4 Dec 2023 11:34:32 +0100 Subject: [PATCH 03/27] Check that the object slot of static frame slots is the default value on entering roots. --- .../truffle/tck/instrumentation/VerifierInstrument.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 5e544f67b070..c902da27d171 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,9 +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++) { - if (!frame.isStatic(slot)) { // Trust static slot usages. - Assert.assertEquals("Top-most nodes tagged with RootTag should have clean frames.", defaultValue, frame.getValue(slot)); - } + Object value = !frame.isStatic(slot) + ? frame.getValue(slot) + // Note: If assertions are disabled and this static slot is + // primitive, we may miss the slot not being clean. + : frame.getObjectStatic(slot); + Assert.assertEquals("Top-most nodes tagged with RootTag should have clean frames.", defaultValue, value); } } } From 07af328edf4bb69a20f25529b26628828617c22d Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 6 Dec 2023 12:41:43 +0100 Subject: [PATCH 04/27] Abide by truffle expectations that frames should be clean on entering root nodes. --- .../truffle/espresso/nodes/BytecodeNode.java | 6 ++++++ .../truffle/espresso/nodes/EspressoFrame.java | 15 ++++++++++++++- .../nodes/EspressoInstrumentableRootNode.java | 1 - .../truffle/espresso/nodes/EspressoRootNode.java | 2 -- .../espresso/nodes/IntrinsicSubstitutorNode.java | 5 ----- .../nodes/IntrinsifiedNativeMethodNode.java | 5 ----- .../espresso/nodes/MethodWithBytecodeNode.java | 5 ----- .../truffle/espresso/nodes/NativeMethodNode.java | 5 ----- 8 files changed, 20 insertions(+), 24 deletions(-) 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..84d488d27345 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 @@ -57,6 +57,8 @@ private EspressoFrame() { private static final int BCI_SLOT = 0; private static final int VALUES_START = 1; + private static final Object UNINIT_MARKER = new Object(); + public static FrameDescriptor createFrameDescriptor(int locals, int stack) { int slotCount = locals + stack; FrameDescriptor.Builder builder = FrameDescriptor.newBuilder(slotCount + VALUES_START); @@ -64,6 +66,7 @@ public static FrameDescriptor createFrameDescriptor(int locals, int stack) { assert bciSlot == BCI_SLOT; int valuesStart = builder.addSlots(slotCount, FrameSlotKind.Static); // locals + stack assert valuesStart == VALUES_START; + builder.defaultValue(UNINIT_MARKER); return builder.build(); } @@ -314,7 +317,17 @@ static void setBCI(Frame frame, int bci) { } static int getBCI(Frame frame) { - return frame.getIntStatic(BCI_SLOT); + CompilerAsserts.neverPartOfCompilation(); + try { + // Note: if assertions are disabled, but the slot is still uninitialized, this will not + // throw and return 0. + return frame.getIntStatic(BCI_SLOT); + } catch (AssertionError e) { + // We did not have time to initialize the BCI before needing the inspection. Return 0 + // for compatibility with the assertions disabled case. + assert frame.getObjectStatic(BCI_SLOT) == UNINIT_MARKER; + return 0; + } } 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..0c067677c938 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 @@ -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..baf25ad0b958 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 @@ -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(); From 82aa6c87e966f2299249e2e07121c57484d011b1 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 3 Jan 2024 10:52:20 +0100 Subject: [PATCH 05/27] Make static slots tag initialize to 0. --- .../truffle/api/frame/FrameAccessor.java | 10 -- .../truffle/api/frame/FrameDescriptor.java | 20 +-- .../com/oracle/truffle/api/impl/Accessor.java | 4 - .../truffle/api/impl/FrameWithoutBoxing.java | 114 ++++++++++++------ .../instrumentation/VerifierInstrument.java | 12 +- 5 files changed, 88 insertions(+), 72 deletions(-) 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..4857d7511671 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 @@ -107,14 +107,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 +132,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 +157,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 +427,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 +473,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 +505,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 +530,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..a58fcef141af 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; @@ -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,57 @@ 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. + * + * 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). + * + * 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. + */ + + 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.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 c902da27d171..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,12 +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++) { - Object value = !frame.isStatic(slot) - ? frame.getValue(slot) - // Note: If assertions are disabled and this static slot is - // primitive, we may miss the slot not being clean. - : frame.getObjectStatic(slot); - Assert.assertEquals("Top-most nodes tagged with RootTag should have clean frames.", defaultValue, value); + 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)); + } } } } From ecc4e3cc8406544bdf9d7cbe4788b2a2e26d7435 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 3 Jan 2024 10:56:22 +0100 Subject: [PATCH 06/27] Exploit the fact that uninitialized static frame slots return 0 on accesses to always have a defined BCI. --- .../truffle/espresso/nodes/EspressoFrame.java | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) 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 84d488d27345..da8c5419ad23 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 @@ -57,8 +57,6 @@ private EspressoFrame() { private static final int BCI_SLOT = 0; private static final int VALUES_START = 1; - private static final Object UNINIT_MARKER = new Object(); - public static FrameDescriptor createFrameDescriptor(int locals, int stack) { int slotCount = locals + stack; FrameDescriptor.Builder builder = FrameDescriptor.newBuilder(slotCount + VALUES_START); @@ -66,7 +64,6 @@ public static FrameDescriptor createFrameDescriptor(int locals, int stack) { assert bciSlot == BCI_SLOT; int valuesStart = builder.addSlots(slotCount, FrameSlotKind.Static); // locals + stack assert valuesStart == VALUES_START; - builder.defaultValue(UNINIT_MARKER); return builder.build(); } @@ -313,21 +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) { - CompilerAsserts.neverPartOfCompilation(); - try { - // Note: if assertions are disabled, but the slot is still uninitialized, this will not - // throw and return 0. - return frame.getIntStatic(BCI_SLOT); - } catch (AssertionError e) { - // We did not have time to initialize the BCI before needing the inspection. Return 0 - // for compatibility with the assertions disabled case. - assert frame.getObjectStatic(BCI_SLOT) == UNINIT_MARKER; - return 0; - } + return frame.getIntStatic(BCI_SLOT) - 1; } public static int startingStackOffset(int maxLocals) { From 53e02790947a41a7f2bbffcab01dd6dd4011d1c7 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Fri, 5 Jan 2024 12:31:46 +0100 Subject: [PATCH 07/27] Propagate new frame static slot initialization handling changes to the compiler side. --- .../compiler/truffle/KnownTruffleTypes.java | 1 - .../truffle/nodes/frame/NewFrameNode.java | 51 +++++-------------- .../nodes/frame/VirtualFrameGetNode.java | 12 ++++- .../nodes/frame/VirtualFrameIsNode.java | 7 ++- .../phases/FrameAccessVerificationPhase.java | 2 +- .../TruffleGraphBuilderPlugins.java | 11 ++++ .../truffle/api/test/IndexedFrameTest.java | 6 +++ .../truffle/api/impl/FrameWithoutBoxing.java | 12 +++-- 8 files changed, 54 insertions(+), 48 deletions(-) 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..d8d4dec90106 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 @@ -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/nodes/frame/NewFrameNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/NewFrameNode.java index feca3183719c..a02e7902ce9c 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 @@ -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/VirtualFrameGetNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/nodes/frame/VirtualFrameGetNode.java index 9440eefe1247..57b1aa37feb0 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 @@ -87,7 +87,17 @@ public void virtualize(VirtualizerTool tool) { if (frameSlotIndex < tagVirtual.entryCount() && frameSlotIndex < dataVirtual.entryCount()) { 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 goes can go through the regular + * route. + */ + 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..506684832761 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 @@ -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/phases/FrameAccessVerificationPhase.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/FrameAccessVerificationPhase.java index fb3189597a9f..862bffbe8d81 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 @@ -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; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java index 19b78a032380..8b4610bc208d 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java @@ -920,6 +920,17 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec return false; } }); + r.register(new RequiredInvocationPlugin("isStatic", Receiver.class, int.class) { + @Override + public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver frameNode, ValueNode frameSlotNode) { + int frameSlotIndex = maybeGetConstantNumberedFrameSlotIndex(frameNode, frameSlotNode); + if (frameSlotIndex >= 0) { + b.addPush(JavaKind.Boolean, new VirtualFrameIsNode(frameNode, frameSlotIndex, VirtualFrameIsNode.STATIC_TAG, VirtualFrameAccessType.Indexed)); + return true; + } + return false; + } + }); r.register(new RequiredInvocationPlugin("extend", int.class) { @Override public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode value) { 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..e228da67154c 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 @@ -147,6 +147,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/src/com/oracle/truffle/api/impl/FrameWithoutBoxing.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/FrameWithoutBoxing.java index a58fcef141af..cf5d35b12b35 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 @@ -441,7 +441,10 @@ private byte getIndexedTagChecked(int slot) { @Override public boolean isObject(int slot) { - return isNonStaticType(slot, OBJECT_TAG); + return isNonStaticType(slot, OBJECT_TAG) && + // Uninitialized static slots get OBJECT_TAG before the first set , so we + // explicitly check for non-staticness of the slot. + !isStatic(slot); } @Override @@ -772,14 +775,13 @@ public void clearStatic(int slot) { * * 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. + * 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). - * - * 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. */ private boolean checkStaticGet(int slot, byte tag) { From 628900ccb27d9e214f31dffda9ebd0927e8ea18c Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Fri, 5 Jan 2024 16:33:09 +0100 Subject: [PATCH 08/27] Fix typo, and remove unused constants. --- .../truffle/nodes/frame/VirtualFrameGetNode.java | 2 +- .../oracle/truffle/api/frame/FrameDescriptor.java | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) 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 57b1aa37feb0..730bc5117366 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 @@ -91,7 +91,7 @@ public void virtualize(VirtualizerTool tool) { (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 goes can go through the regular + * value for the access kind. Reading an object can go through the regular * route. */ ValueNode dataEntry = ConstantNode.defaultForKind(getStackKind(), graph()); 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 4857d7511671..240d0fcd6769 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,6 @@ * @since 0.8 or earlier */ public final class FrameDescriptor implements Cloneable { - /* - * Changing these constants implies changes in NewFrameNode.java as well: - */ - /** - * @since 22.2 - */ - static final int NO_STATIC_MODE = 1; - /** - * @since 22.2 - */ - static final int ALL_STATIC_MODE = 2; - /** - * @since 22.2 - */ - static final int MIXED_STATIC_MODE = NO_STATIC_MODE | ALL_STATIC_MODE; private final Object defaultValue; From 89326ffe508322c0feaa289eae7271b583221d51 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Fri, 5 Jan 2024 16:36:18 +0100 Subject: [PATCH 09/27] Make some FrameDescriptor fields as immutable during Truffle host compilations. --- .../truffle/host/InjectImmutableFrameFieldsPhase.java | 5 +++-- .../compiler/truffle/host/TruffleKnownHostTypes.java | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) 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..98955915aeca 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 @@ -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..6e07003c69c9 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 @@ -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"); From 0f98481d104dba9392baa1f5241a59a762d4b764 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Thu, 25 Jan 2024 14:10:13 +0100 Subject: [PATCH 10/27] Add possibility to easily remove assertions from subprocess tests. Use it to ensure consistent handling of uninitialized static slots. --- .../truffle/test/BytecodeOSRNodeTest.java | 145 +++++++++++++++++- .../truffle/api/test/SubprocessTestUtils.java | 40 ++++- 2 files changed, 172 insertions(+), 13 deletions(-) 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..f8ccd9473923 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 @@ -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 { @@ -107,6 +109,20 @@ private static void checkInCompiledCode() { boundaryCall(); } + private static boolean checkFrameAssertionsDisabled() { + FrameDescriptor.Builder builder = FrameDescriptor.newBuilder(); + int slot = builder.addSlot(FrameSlotKind.Static, null, null); + FrameDescriptor desc = builder.build(); + try { + FrameWithoutBoxing frame = new FrameWithoutBoxing(desc, new Object[0]); + frame.setIntStatic(slot, 1); + frame.getFloatStatic(slot); + } catch (AssertionError e) { + return false; + } + return true; + } + /* * Test that an infinite interpreter loop triggers OSR. */ @@ -407,6 +423,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.assertTrue(checkFrameAssertionsDisabled()); + 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 +490,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.assertTrue(checkFrameAssertionsDisabled()); + frameTransferWithStaticAccesses(); + }, 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 +1606,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.setBooleanStatic(booleanSlot, true); + // everything else is uninitialized + } + + @Override + public void checkRegularState(VirtualFrame frame) { + try { + assertEquals(true, frame.getBooleanStatic(booleanSlot)); + // these slots are uninitialized + ensureUninitReturnsDefault(frame, byteSlot); + 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.setBooleanStatic(booleanSlot, false); + // everything else is uninitialized + } + + @Override + public void checkOSRState(VirtualFrame frame) { + try { + assertEquals(false, frame.getBooleanStatic(booleanSlot)); + // these slots are uninitialized + ensureUninitReturnsDefault(frame, byteSlot); + 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/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. */ From c84d71c7783552b7d24bc8c827ec9053467d9f06 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Thu, 25 Jan 2024 14:10:44 +0100 Subject: [PATCH 11/27] Update FrameHostReadsTest using assertion disabled. --- .../truffle/test/FrameHostReadsTest.java | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) 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..ea8a0c4a14d4 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 @@ -24,18 +24,24 @@ */ 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; import com.oracle.truffle.api.Truffle; import com.oracle.truffle.api.frame.FrameDescriptor; +import com.oracle.truffle.api.frame.FrameSlotKind; 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 +65,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.", checkFrameAssertionsDisabled()); StructuredGraph graph = getFinalGraph("snippet0"); @@ -86,25 +104,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 also read one for FrameWithoutBoxing.indexedPrimitiveLocals. */ - Assert.assertEquals(2, arrayLengthReads); + Assert.assertEquals(1, arrayLengthReads); Assert.assertEquals(0, otherReads); } @@ -119,4 +133,18 @@ public Throwable fillInStackTrace() { }; } + private static boolean checkFrameAssertionsDisabled() { + FrameDescriptor.Builder builder = FrameDescriptor.newBuilder(); + int slot = builder.addSlot(FrameSlotKind.Static, null, null); + FrameDescriptor desc = builder.build(); + try { + FrameWithoutBoxing frame = new FrameWithoutBoxing(desc, new Object[0]); + frame.setIntStatic(slot, 1); + frame.getFloatStatic(slot); + } catch (AssertionError e) { + return false; + } + return true; + } + } From 060c9b36abbdbe8d5ae2e00f4ba4def73454a9ef Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Thu, 25 Jan 2024 14:12:38 +0100 Subject: [PATCH 12/27] Adds an assertion in frame access node to fail early when static slots are not correctly used. Also adds comments for clarity. --- .../truffle/nodes/frame/VirtualFrameAccessorNode.java | 8 ++++++++ .../compiler/truffle/nodes/frame/VirtualFrameGetNode.java | 6 ++++++ .../compiler/truffle/nodes/frame/VirtualFrameSetNode.java | 1 + 3 files changed, 15 insertions(+) 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..c19f4db4fc2d 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 @@ -98,6 +98,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() { + assert 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 730bc5117366..457ae497179d 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 @@ -85,6 +85,7 @@ 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 && accessKind.isPrimitive() && @@ -93,6 +94,11 @@ public void virtualize(VirtualizerTool tool) { * 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); 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..5f62957f48f3 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 @@ -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)); From 14574618b5ca6d0a3b781d014d04ed6bbc92135e Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Thu, 25 Jan 2024 14:13:19 +0100 Subject: [PATCH 13/27] Remove intrinsification of 'isStatic' --- .../substitutions/TruffleGraphBuilderPlugins.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java index 8b4610bc208d..19b78a032380 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java @@ -920,17 +920,6 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec return false; } }); - r.register(new RequiredInvocationPlugin("isStatic", Receiver.class, int.class) { - @Override - public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver frameNode, ValueNode frameSlotNode) { - int frameSlotIndex = maybeGetConstantNumberedFrameSlotIndex(frameNode, frameSlotNode); - if (frameSlotIndex >= 0) { - b.addPush(JavaKind.Boolean, new VirtualFrameIsNode(frameNode, frameSlotIndex, VirtualFrameIsNode.STATIC_TAG, VirtualFrameAccessType.Indexed)); - return true; - } - return false; - } - }); r.register(new RequiredInvocationPlugin("extend", int.class) { @Override public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode value) { From 2cc27f0ed96c6a06be3831d7fd6d2d6454243c1d Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Thu, 25 Jan 2024 14:15:09 +0100 Subject: [PATCH 14/27] More precise handling of static slot transfer during bytecode OSR. Static slots always transfer as static, even if uninitialized. --- .../truffle/runtime/BytecodeOSRMetadata.java | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) 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..b146383aba51 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,23 @@ 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)) { + osrEntry.indexedFrameTags[i] = frame.getTag(i); + } else { + // 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; + } } } }); @@ -508,6 +524,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. From 8f6a411857746c75565abcecc3c4eaa2aa5851c6 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 31 Jan 2024 09:58:17 +0100 Subject: [PATCH 15/27] ClearPrimitiveEffect in FrameAccessVerification uses correct access flag. --- .../compiler/truffle/nodes/frame/VirtualFrameAccessFlags.java | 4 +++- .../compiler/truffle/phases/FrameAccessVerificationPhase.java | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) 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..9ad6612351b4 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 @@ -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/phases/FrameAccessVerificationPhase.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/phases/FrameAccessVerificationPhase.java index 862bffbe8d81..389c47a1c98c 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 @@ -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))); } } } From 69467ac5959439d24e48e935514cfc51a02861d1 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 31 Jan 2024 10:25:51 +0100 Subject: [PATCH 16/27] Style fixes: use multi-line comment, remove useless whitespace, invert if condition. --- .../truffle/api/impl/FrameWithoutBoxing.java | 2 +- .../truffle/runtime/BytecodeOSRMetadata.java | 33 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) 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 cf5d35b12b35..29e78dd55b29 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 @@ -442,7 +442,7 @@ private byte getIndexedTagChecked(int slot) { @Override public boolean isObject(int slot) { return isNonStaticType(slot, OBJECT_TAG) && - // Uninitialized static slots get OBJECT_TAG before the first set , so we + // Uninitialized static slots get OBJECT_TAG before the first set, so we // explicitly check for non-staticness of the slot. !isStatic(slot); } 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 b146383aba51..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,22 +202,25 @@ 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++) { - if (!frame.isStatic(i)) { - osrEntry.indexedFrameTags[i] = frame.getTag(i); - } else { - // 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. + 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); } } } From 163c0a0a1ded595d5b75c9614579fdff3f0f6aee Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 31 Jan 2024 10:28:16 +0100 Subject: [PATCH 17/27] Adds precision to Frame documentation. --- .../src/com/oracle/truffle/api/frame/Frame.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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..3ece9fe1b02f 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 @@ -436,7 +436,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 +464,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 +492,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 +520,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 +548,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 +576,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 +604,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) { From 69dec2d0209b3e4f675fa18e8b8882c0d9e9a30d Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Mon, 5 Feb 2024 09:35:14 +0100 Subject: [PATCH 18/27] Adds profiles to ToEspressoNode. --- .../nodes/interop/ToEspressoNode.java | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) 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 6df14b219839..92a6833e45d0 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 @@ -37,6 +37,7 @@ import com.oracle.truffle.api.interop.UnsupportedTypeException; import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.NodeInfo; +import com.oracle.truffle.api.profiles.BranchProfile; import com.oracle.truffle.api.profiles.InlinedBranchProfile; import com.oracle.truffle.espresso.EspressoLanguage; import com.oracle.truffle.espresso.impl.ArrayKlass; @@ -152,11 +153,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 BranchProfile 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(); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.cat("Cannot cast ", value, " to ", targetType.getTypeAsString())); } @@ -165,8 +168,10 @@ 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) throws UnsupportedTypeException { + @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, + @Cached BranchProfile error) throws UnsupportedTypeException { if (targetType.isPrimitive()) { + error.enter(); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.cat("Cannot cast ", value, " to ", targetType.getTypeAsString())); } return StaticObject.createForeignNull(EspressoLanguage.get(this), value); @@ -179,7 +184,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 BranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); WrappedProxyKlass proxyKlass = lookupProxyKlassNode.execute(metaObject, getMetaName(metaObject, interop), targetType); @@ -187,8 +193,10 @@ public Object doMappedInterface(Object value, Klass targetType, targetType.safeInitialize(); return proxyKlass.createProxyInstance(value, getLanguage(), interop); } + error.enter(); throw new ClassCastException(); } catch (ClassCastException e) { + error.enter(); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getTypeAsString())); } } @@ -198,7 +206,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 BranchProfile 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); @@ -207,6 +216,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(); throw UnsupportedTypeException.create(new Object[]{value}, targetType.getTypeAsString()); } @@ -217,7 +227,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 BranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); String metaName = getMetaName(metaObject, interop); @@ -227,8 +238,10 @@ public Object doTypeConverter(Object value, Klass targetType, if (converter != null) { return converter.convert(StaticObject.createForeign(getLanguage(), targetType, value, interop)); } + error.enter(); throw new ClassCastException(); } catch (ClassCastException e) { + error.enter(); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getNameAsString(), e.getMessage())); } } @@ -241,7 +254,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 BranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); String metaName = getMetaName(metaObject, interop); @@ -251,8 +265,10 @@ public Object doInternalTypeConverter(Object value, Klass targetType, if (converter != null) { return converter.convertInternal(interop, value, getMeta(), converterToEspresso); } + error.enter(); throw new ClassCastException(); } catch (ClassCastException e) { + error.enter(); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getNameAsString(), e.getMessage())); } } From 42794662bd6aa7faf248cf058069eb7503527ee2 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Mon, 5 Feb 2024 09:35:49 +0100 Subject: [PATCH 19/27] Add changelog entry for uninitialized static slot behavior. --- truffle/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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 From 4ed5c6b649143da1e8f3b3128c3d40791ac85bc0 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 7 Feb 2024 08:37:18 +0100 Subject: [PATCH 20/27] restore and deprecate static-mode related fields in FrameDescriptor --- .../truffle/api/frame/FrameDescriptor.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 240d0fcd6769..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,6 +60,30 @@ * @since 0.8 or earlier */ public final class FrameDescriptor implements Cloneable { + /** + * @since 22.2 + * @deprecated since 24.1 + */ + @Deprecated static final int NO_STATIC_MODE = 1; + /** + * @since 22.2 + * @deprecated since 24.1 + */ + @Deprecated static final int ALL_STATIC_MODE = 2; + /** + * @since 22.2 + * @deprecated since 24.1 + */ + @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 + */ + @Deprecated final int staticMode = MIXED_STATIC_MODE; private final Object defaultValue; From fa3b87f283f47e1781135c186ae397a9117a7459 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 7 Feb 2024 08:45:21 +0100 Subject: [PATCH 21/27] Use InlinedBranchProfile instead of regular BranchProfile --- .../nodes/interop/ToEspressoNode.java | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) 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 92a6833e45d0..e3f3622f54d6 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 @@ -37,7 +37,6 @@ import com.oracle.truffle.api.interop.UnsupportedTypeException; import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.NodeInfo; -import com.oracle.truffle.api.profiles.BranchProfile; import com.oracle.truffle.api.profiles.InlinedBranchProfile; import com.oracle.truffle.espresso.EspressoLanguage; import com.oracle.truffle.espresso.impl.ArrayKlass; @@ -154,12 +153,12 @@ public static boolean isStaticObject(Object value) { @Specialization public Object doStaticObject(StaticObject value, Klass targetType, @Cached InstanceOf.Dynamic instanceOf, - @Cached BranchProfile error) throws UnsupportedTypeException { + @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(); + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.cat("Cannot cast ", value, " to ", targetType.getTypeAsString())); } @@ -169,9 +168,9 @@ public Object doStaticObject(StaticObject value, Klass targetType, }) public Object doForeignNull(Object value, @SuppressWarnings("unused") Klass targetType, @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, - @Cached BranchProfile error) throws UnsupportedTypeException { + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { if (targetType.isPrimitive()) { - error.enter(); + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.cat("Cannot cast ", value, " to ", targetType.getTypeAsString())); } return StaticObject.createForeignNull(EspressoLanguage.get(this), value); @@ -185,7 +184,7 @@ 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, - @Cached BranchProfile error) throws UnsupportedTypeException { + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); WrappedProxyKlass proxyKlass = lookupProxyKlassNode.execute(metaObject, getMetaName(metaObject, interop), targetType); @@ -193,10 +192,10 @@ public Object doMappedInterface(Object value, Klass targetType, targetType.safeInitialize(); return proxyKlass.createProxyInstance(value, getLanguage(), interop); } - error.enter(); + error.enter(this); throw new ClassCastException(); } catch (ClassCastException e) { - error.enter(); + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getTypeAsString())); } } @@ -207,7 +206,7 @@ public Object doMappedInterface(Object value, Klass targetType, }) public Object doArray(Object value, ArrayKlass targetType, @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, - @Cached BranchProfile error) throws UnsupportedTypeException { + @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); @@ -216,7 +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(); + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, targetType.getTypeAsString()); } @@ -228,7 +227,7 @@ public Object doArray(Object value, ArrayKlass targetType, public Object doTypeConverter(Object value, Klass targetType, @Cached LookupTypeConverterNode lookupTypeConverter, @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, - @Cached BranchProfile error) throws UnsupportedTypeException { + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); String metaName = getMetaName(metaObject, interop); @@ -238,10 +237,10 @@ public Object doTypeConverter(Object value, Klass targetType, if (converter != null) { return converter.convert(StaticObject.createForeign(getLanguage(), targetType, value, interop)); } - error.enter(); + error.enter(this); throw new ClassCastException(); } catch (ClassCastException e) { - error.enter(); + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getNameAsString(), e.getMessage())); } } @@ -255,7 +254,7 @@ public Object doInternalTypeConverter(Object value, Klass targetType, @Cached ToReference.DynamicToReference converterToEspresso, @Cached LookupInternalTypeConverterNode lookupInternalTypeConverter, @SuppressWarnings("unused") @CachedLibrary(limit = "LIMIT") InteropLibrary interop, - @Cached BranchProfile error) throws UnsupportedTypeException { + @Cached InlinedBranchProfile error) throws UnsupportedTypeException { try { Object metaObject = getMetaObjectOrThrow(value, interop); String metaName = getMetaName(metaObject, interop); @@ -265,10 +264,10 @@ public Object doInternalTypeConverter(Object value, Klass targetType, if (converter != null) { return converter.convertInternal(interop, value, getMeta(), converterToEspresso); } - error.enter(); + error.enter(this); throw new ClassCastException(); } catch (ClassCastException e) { - error.enter(); + error.enter(this); throw UnsupportedTypeException.create(new Object[]{value}, EspressoError.format("Could not cast foreign object to %s: ", targetType.getNameAsString(), e.getMessage())); } } From dd3782aeef1e0457a191897beee791f5571b06c9 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Thu, 15 Feb 2024 16:32:06 +0100 Subject: [PATCH 22/27] Use reflection to check for frame assertions disabled. Fixes test called in BytecodeOSRTest. Typo fix in FrameHostReads test comment. --- .../truffle/test/BytecodeOSRNodeTest.java | 31 +++++++++++-------- .../truffle/test/FrameHostReadsTest.java | 29 +++++++++-------- 2 files changed, 34 insertions(+), 26 deletions(-) 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 f8ccd9473923..7785456be761 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 @@ -26,6 +26,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.lang.reflect.Field; import java.util.List; import java.util.concurrent.TimeUnit; @@ -109,18 +110,22 @@ private static void checkInCompiledCode() { boundaryCall(); } - private static boolean checkFrameAssertionsDisabled() { - FrameDescriptor.Builder builder = FrameDescriptor.newBuilder(); - int slot = builder.addSlot(FrameSlotKind.Static, null, null); - FrameDescriptor desc = builder.build(); + private static void checkFrameAssertionsDisabled() { + Field assertionsEnabledField = null; + for (Field f : FrameWithoutBoxing.class.getDeclaredFields()) { + if (f.getName().equals("ASSERTIONS_ENABLED")) { + assertionsEnabledField = f; + break; + } + } + Assert.assertNotNull(assertionsEnabledField); try { - FrameWithoutBoxing frame = new FrameWithoutBoxing(desc, new Object[0]); - frame.setIntStatic(slot, 1); - frame.getFloatStatic(slot); - } catch (AssertionError e) { - return false; + assertionsEnabledField.setAccessible(true); + boolean enabledStatus = (boolean) assertionsEnabledField.get(null); + Assert.assertFalse(enabledStatus); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); } - return true; } /* @@ -435,7 +440,7 @@ public void testFrameTransferWithStaticAccessesWithAssertionsDisabled() throws T // Execute in a subprocess to disable assertion checking for frames. SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class, () -> { - Assert.assertTrue(checkFrameAssertionsDisabled()); + checkFrameAssertionsDisabled(); frameTransferWithStaticAccesses(); }, true, List.of(FrameWithoutBoxing.class)); } @@ -509,8 +514,8 @@ public void testFrameTransferWithUninitializedStaticSlotsWithoutAssertions() thr // Execute in a subprocess to disable assertion checking for frames. SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class, () -> { - Assert.assertTrue(checkFrameAssertionsDisabled()); - frameTransferWithStaticAccesses(); + checkFrameAssertionsDisabled(); + frameTransferWithUninitializedStaticSlots(); }, true, List.of(FrameWithoutBoxing.class)); } 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 ea8a0c4a14d4..4ad0a0e875fe 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 @@ -25,6 +25,7 @@ package jdk.graal.compiler.truffle.test; import java.io.IOException; +import java.lang.reflect.Field; import java.util.List; import org.graalvm.word.LocationIdentity; @@ -33,7 +34,6 @@ import com.oracle.truffle.api.Truffle; import com.oracle.truffle.api.frame.FrameDescriptor; -import com.oracle.truffle.api.frame.FrameSlotKind; import com.oracle.truffle.api.impl.FrameWithoutBoxing; import com.oracle.truffle.api.test.SubprocessTestUtils; @@ -81,7 +81,7 @@ private void compileAndCheck() { 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.", checkFrameAssertionsDisabled()); + Assert.assertTrue("Frame assertions should be disabled.", areFrameAssertionsDisabled()); StructuredGraph graph = getFinalGraph("snippet0"); @@ -116,7 +116,7 @@ private void compileAndCheck() { Assert.assertEquals(5, arrayReads); /* - * Array.length reads. We also read one for FrameWithoutBoxing.indexedPrimitiveLocals. + * Array.length reads. We read one for FrameWithoutBoxing.indexedPrimitiveLocals. */ Assert.assertEquals(1, arrayLengthReads); Assert.assertEquals(0, otherReads); @@ -133,18 +133,21 @@ public Throwable fillInStackTrace() { }; } - private static boolean checkFrameAssertionsDisabled() { - FrameDescriptor.Builder builder = FrameDescriptor.newBuilder(); - int slot = builder.addSlot(FrameSlotKind.Static, null, null); - FrameDescriptor desc = builder.build(); + private static boolean areFrameAssertionsDisabled() { + Field assertionsEnabledField = null; + for (Field f : FrameWithoutBoxing.class.getDeclaredFields()) { + if (f.getName().equals("ASSERTIONS_ENABLED")) { + assertionsEnabledField = f; + break; + } + } + Assert.assertNotNull(assertionsEnabledField); try { - FrameWithoutBoxing frame = new FrameWithoutBoxing(desc, new Object[0]); - frame.setIntStatic(slot, 1); - frame.getFloatStatic(slot); - } catch (AssertionError e) { - return false; + assertionsEnabledField.setAccessible(true); + return !((boolean) assertionsEnabledField.get(null)); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); } - return true; } } From 4288fbe24b75d775e7dc65370e470b9e7991d8e8 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Thu, 15 Feb 2024 16:36:04 +0100 Subject: [PATCH 23/27] Use byte static slot to differentiate uninit/Regular/OSR state, rather than 2-state boolean. --- .../compiler/truffle/test/BytecodeOSRNodeTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 7785456be761..05695a84e152 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 @@ -1631,16 +1631,16 @@ private void ensureUninitReturnsDefault(VirtualFrame frame, int index) { @Override public void setRegularState(VirtualFrame frame) { - frame.setBooleanStatic(booleanSlot, true); + frame.setByteStatic(byteSlot, (byte) 1); // everything else is uninitialized } @Override public void checkRegularState(VirtualFrame frame) { try { - assertEquals(true, frame.getBooleanStatic(booleanSlot)); + assertEquals((byte) 1, frame.getByteStatic(byteSlot)); // these slots are uninitialized - ensureUninitReturnsDefault(frame, byteSlot); + ensureUninitReturnsDefault(frame, booleanSlot); ensureUninitReturnsDefault(frame, doubleSlot); ensureUninitReturnsDefault(frame, floatSlot); ensureUninitReturnsDefault(frame, intSlot); @@ -1654,16 +1654,16 @@ public void checkRegularState(VirtualFrame frame) { @Override public void setOSRState(VirtualFrame frame) { - frame.setBooleanStatic(booleanSlot, false); + frame.setByteStatic(byteSlot, (byte) 2); // everything else is uninitialized } @Override public void checkOSRState(VirtualFrame frame) { try { - assertEquals(false, frame.getBooleanStatic(booleanSlot)); + assertEquals((byte) 2, frame.getByteStatic(byteSlot)); // these slots are uninitialized - ensureUninitReturnsDefault(frame, byteSlot); + ensureUninitReturnsDefault(frame, booleanSlot); ensureUninitReturnsDefault(frame, doubleSlot); ensureUninitReturnsDefault(frame, floatSlot); ensureUninitReturnsDefault(frame, intSlot); From 513f6fe32d9ef03af0bb7c34da977c5d107226f4 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Thu, 29 Feb 2024 13:41:23 +0100 Subject: [PATCH 24/27] Remove isStatic check from isObject, and make it an assert for all `isType` methods. Update documentation to specify not to trust `isType` on static slots. --- .../com/oracle/truffle/api/frame/Frame.java | 30 ++++++++++++++----- .../truffle/api/impl/FrameWithoutBoxing.java | 9 ++---- 2 files changed, 26 insertions(+), 13 deletions(-) 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 3ece9fe1b02f..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 */ 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 29e78dd55b29..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 @@ -236,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; } @@ -268,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); @@ -441,10 +441,7 @@ private byte getIndexedTagChecked(int slot) { @Override public boolean isObject(int slot) { - return isNonStaticType(slot, OBJECT_TAG) && - // Uninitialized static slots get OBJECT_TAG before the first set, so we - // explicitly check for non-staticness of the slot. - !isStatic(slot); + return isNonStaticType(slot, OBJECT_TAG); } @Override From 001be02ae464a285971fb9547fef92e2ca351e90 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 6 Mar 2024 08:29:32 +0100 Subject: [PATCH 25/27] Fix frame slot type verification in tests. --- .../truffle/api/test/IndexedFrameTest.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) 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 e228da67154c..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 From 774f69ac6318404e37e0e1871254a3e8b3871291 Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 13 Mar 2024 10:43:35 +0100 Subject: [PATCH 26/27] Frame assertions checking is delegated to a helper method. Use GraalError.guarantee instead of assert in compiler. --- .../truffle/test/BytecodeOSRNodeTest.java | 25 +------- .../truffle/test/FrameAssertionsChecker.java | 64 +++++++++++++++++++ .../truffle/test/FrameHostReadsTest.java | 23 +------ .../nodes/frame/VirtualFrameAccessorNode.java | 5 +- 4 files changed, 72 insertions(+), 45 deletions(-) create mode 100644 compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameAssertionsChecker.java 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 05695a84e152..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,7 +26,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.lang.reflect.Field; import java.util.List; import java.util.concurrent.TimeUnit; @@ -110,24 +109,6 @@ private static void checkInCompiledCode() { boundaryCall(); } - private static void checkFrameAssertionsDisabled() { - 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); - boolean enabledStatus = (boolean) assertionsEnabledField.get(null); - Assert.assertFalse(enabledStatus); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } - } - /* * Test that an infinite interpreter loop triggers OSR. */ @@ -440,7 +421,7 @@ public void testFrameTransferWithStaticAccessesWithAssertionsDisabled() throws T // Execute in a subprocess to disable assertion checking for frames. SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class, () -> { - checkFrameAssertionsDisabled(); + Assert.assertFalse(FrameAssertionsChecker.areFrameAssertionsEnabled()); frameTransferWithStaticAccesses(); }, true, List.of(FrameWithoutBoxing.class)); } @@ -514,7 +495,7 @@ public void testFrameTransferWithUninitializedStaticSlotsWithoutAssertions() thr // Execute in a subprocess to disable assertion checking for frames. SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class, () -> { - checkFrameAssertionsDisabled(); + Assert.assertFalse(FrameAssertionsChecker.areFrameAssertionsEnabled()); frameTransferWithUninitializedStaticSlots(); }, true, List.of(FrameWithoutBoxing.class)); } 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 4ad0a0e875fe..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 @@ -25,7 +25,6 @@ package jdk.graal.compiler.truffle.test; import java.io.IOException; -import java.lang.reflect.Field; import java.util.List; import org.graalvm.word.LocationIdentity; @@ -81,7 +80,7 @@ private void compileAndCheck() { 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.", areFrameAssertionsDisabled()); + Assert.assertTrue("Frame assertions should be disabled.", !FrameAssertionsChecker.areFrameAssertionsEnabled()); StructuredGraph graph = getFinalGraph("snippet0"); @@ -132,22 +131,4 @@ public Throwable fillInStackTrace() { } }; } - - private static boolean areFrameAssertionsDisabled() { - 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/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 c19f4db4fc2d..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; @@ -103,7 +104,7 @@ public final int getAccessTag() { * accessed statically, and non-static slots accessed non-statically. */ protected final void ensureStaticSlotAccessConsistency() { - assert getFrame().isStatic(getFrameSlotIndex()) == accessFlags.isStatic() : "Inconsistent Static slot usage."; + GraalError.guarantee(getFrame().isStatic(getFrameSlotIndex()) == accessFlags.isStatic(), "Inconsistent Static slot usage."); } protected final void insertDeoptimization(VirtualizerTool tool) { From 9475aee6904be665b5bb59272d5f4b6494ff440b Mon Sep 17 00:00:00 2001 From: "thomas.garcia" Date: Wed, 13 Mar 2024 10:48:28 +0100 Subject: [PATCH 27/27] Update header years. --- .../src/jdk/graal/compiler/truffle/KnownTruffleTypes.java | 2 +- .../compiler/truffle/host/InjectImmutableFrameFieldsPhase.java | 2 +- .../jdk/graal/compiler/truffle/host/TruffleKnownHostTypes.java | 2 +- .../jdk/graal/compiler/truffle/nodes/frame/NewFrameNode.java | 2 +- .../compiler/truffle/nodes/frame/VirtualFrameAccessFlags.java | 2 +- .../graal/compiler/truffle/nodes/frame/VirtualFrameGetNode.java | 2 +- .../graal/compiler/truffle/nodes/frame/VirtualFrameIsNode.java | 2 +- .../graal/compiler/truffle/nodes/frame/VirtualFrameSetNode.java | 2 +- .../compiler/truffle/phases/FrameAccessVerificationPhase.java | 2 +- .../espresso/nodes/AbstractInstrumentableBytecodeNode.java | 2 +- .../src/com/oracle/truffle/espresso/nodes/EspressoFrame.java | 2 +- .../truffle/espresso/nodes/EspressoInstrumentableRootNode.java | 2 +- .../src/com/oracle/truffle/espresso/nodes/EspressoRootNode.java | 2 +- .../oracle/truffle/espresso/nodes/interop/ToEspressoNode.java | 2 +- 14 files changed, 14 insertions(+), 14 deletions(-) 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 d8d4dec90106..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 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 98955915aeca..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 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 6e07003c69c9..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 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 a02e7902ce9c..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 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 9ad6612351b4..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 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 457ae497179d..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 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 506684832761..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 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 5f62957f48f3..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 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 389c47a1c98c..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 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 2668c2f63638..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 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 da8c5419ad23..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 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 0c067677c938..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 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 baf25ad0b958..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 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 e3f3622f54d6..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