Automatic merge of master into galahad

common.json

@@ -4,7 +4,7 @@
     "Jsonnet files should not include this file directly but use ci/common.jsonnet instead."
   ],
 
-  "mx_version": "7.14.3",
+  "mx_version": "7.16.1",
 
   "COMMENT.jdks": "When adding or removing JDKs keep in sync with JDKs in ci/common.jsonnet",
   "jdks": {

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/BytecodeOSRNodeTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,14 +26,9 @@
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.util.List;
 import java.util.concurrent.TimeUnit;
 
-import com.oracle.truffle.api.test.SubprocessTestUtils;
-import com.oracle.truffle.runtime.BytecodeOSRMetadata;
-import com.oracle.truffle.runtime.OptimizedTruffleRuntime;
-import com.oracle.truffle.runtime.OptimizedCallTarget;
-
-import jdk.graal.compiler.test.GraalTest;
 import org.graalvm.polyglot.Context;
 import org.junit.Assert;
 import org.junit.Before;
@@ -54,10 +49,17 @@
 import com.oracle.truffle.api.frame.FrameSlotKind;
 import com.oracle.truffle.api.frame.FrameSlotTypeException;
 import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.impl.FrameWithoutBoxing;
 import com.oracle.truffle.api.nodes.BytecodeOSRNode;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.nodes.RootNode;
+import com.oracle.truffle.api.test.SubprocessTestUtils;
+import com.oracle.truffle.runtime.BytecodeOSRMetadata;
+import com.oracle.truffle.runtime.OptimizedCallTarget;
+import com.oracle.truffle.runtime.OptimizedTruffleRuntime;
+
+import jdk.graal.compiler.test.GraalTest;
 
 public class BytecodeOSRNodeTest extends TestWithSynchronousCompiling {
 
@@ -407,6 +409,24 @@ public void testFrameTransfer() {
      */
     @Test
     public void testFrameTransferWithStaticAccesses() {
+        frameTransferWithStaticAccesses();
+    }
+
+    /*
+     * Test that the frame transfer helper works as expected, even with static accesses, both on OSR
+     * enter and exit, and even when assertions are disabled.
+     */
+    @Test
+    public void testFrameTransferWithStaticAccessesWithAssertionsDisabled() throws Throwable {
+        // Execute in a subprocess to disable assertion checking for frames.
+        SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class,
+                        () -> {
+                            Assert.assertFalse(FrameAssertionsChecker.areFrameAssertionsEnabled());
+                            frameTransferWithStaticAccesses();
+                        }, true, List.of(FrameWithoutBoxing.class));
+    }
+
+    public static void frameTransferWithStaticAccesses() {
         var frameBuilder = FrameDescriptor.newBuilder();
         RootNode rootNode = new Program(new FrameTransferringWithStaticAccessNode(frameBuilder), frameBuilder.build());
         OptimizedCallTarget target = (OptimizedCallTarget) rootNode.getCallTarget();
@@ -456,6 +476,40 @@ public void testFrameTransferWithUninitializedSlots() {
         Assert.assertEquals(42, target.call());
     }
 
+    /*
+     * Same as above, but with static slots.
+     *
+     * Note that uninitialized static slots differs from regular slots in that it may be read as any
+     * kind, and always return the default ('defaultValue' for Object, '0' for primitives).
+     */
+    @Test
+    public void testFrameTransferWithUninitializedStaticSlots() {
+        frameTransferWithUninitializedStaticSlots();
+    }
+
+    /*
+     * Same as above, but with assertion disabled.
+     */
+    @Test
+    public void testFrameTransferWithUninitializedStaticSlotsWithoutAssertions() throws Throwable {
+        // Execute in a subprocess to disable assertion checking for frames.
+        SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(BytecodeOSRNodeTest.class,
+                        () -> {
+                            Assert.assertFalse(FrameAssertionsChecker.areFrameAssertionsEnabled());
+                            frameTransferWithUninitializedStaticSlots();
+                        }, true, List.of(FrameWithoutBoxing.class));
+    }
+
+    public static void frameTransferWithUninitializedStaticSlots() {
+        // use a non-null default value to make sure it gets copied properly.
+        var frameBuilder = FrameDescriptor.newBuilder();
+        Object defaultValue = new Object();
+        frameBuilder.defaultValue(defaultValue);
+        RootNode rootNode = new Program(new FrameTransferringNodeWithUninitializedStaticSlots(frameBuilder, defaultValue), frameBuilder.build());
+        OptimizedCallTarget target = (OptimizedCallTarget) rootNode.getCallTarget();
+        Assert.assertEquals(42, target.call());
+    }
+
     /*
      * Test that we can safely recover from bailing out of OSR compilation while an OSR frame is
      * currently executing.
@@ -1538,6 +1592,71 @@ public void checkOSRState(VirtualFrame frame) {
         }
     }
 
+    public static class FrameTransferringNodeWithUninitializedStaticSlots extends FrameTransferringWithStaticAccessNode {
+        final Object defaultValue;
+
+        public FrameTransferringNodeWithUninitializedStaticSlots(FrameDescriptor.Builder frameDescriptor, Object defaultValue) {
+            super(frameDescriptor);
+            this.defaultValue = defaultValue;
+        }
+
+        private void ensureUninitReturnsDefault(VirtualFrame frame, int index) {
+            assertEquals(defaultValue, frame.getObjectStatic(index));
+            assertEquals((byte) 0, frame.getByteStatic(index));
+            assertEquals(false, frame.getBooleanStatic(index));
+            assertEquals(0, frame.getIntStatic(index));
+            assertEquals(0L, frame.getLongStatic(index));
+            assertEquals(0f, frame.getFloatStatic(index));
+            assertEquals(0d, frame.getDoubleStatic(index));
+        }
+
+        @Override
+        public void setRegularState(VirtualFrame frame) {
+            frame.setByteStatic(byteSlot, (byte) 1);
+            // everything else is uninitialized
+        }
+
+        @Override
+        public void checkRegularState(VirtualFrame frame) {
+            try {
+                assertEquals((byte) 1, frame.getByteStatic(byteSlot));
+                // these slots are uninitialized
+                ensureUninitReturnsDefault(frame, booleanSlot);
+                ensureUninitReturnsDefault(frame, doubleSlot);
+                ensureUninitReturnsDefault(frame, floatSlot);
+                ensureUninitReturnsDefault(frame, intSlot);
+                ensureUninitReturnsDefault(frame, longSlot);
+                ensureUninitReturnsDefault(frame, objectSlot);
+            } catch (FrameSlotTypeException ex) {
+                CompilerDirectives.transferToInterpreter();
+                throw new IllegalStateException("Error accessing index slot");
+            }
+        }
+
+        @Override
+        public void setOSRState(VirtualFrame frame) {
+            frame.setByteStatic(byteSlot, (byte) 2);
+            // everything else is uninitialized
+        }
+
+        @Override
+        public void checkOSRState(VirtualFrame frame) {
+            try {
+                assertEquals((byte) 2, frame.getByteStatic(byteSlot));
+                // these slots are uninitialized
+                ensureUninitReturnsDefault(frame, booleanSlot);
+                ensureUninitReturnsDefault(frame, doubleSlot);
+                ensureUninitReturnsDefault(frame, floatSlot);
+                ensureUninitReturnsDefault(frame, intSlot);
+                ensureUninitReturnsDefault(frame, longSlot);
+                ensureUninitReturnsDefault(frame, objectSlot);
+            } catch (FrameSlotTypeException ex) {
+                CompilerDirectives.transferToInterpreter();
+                throw new IllegalStateException("Error accessing index slot");
+            }
+        }
+    }
+
     public static class OSRDisablingTransferringNode extends FrameTransferringNode {
         @CompilationFinal int staticSlot;
 

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);
+        }
+    }
+}

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/FrameHostReadsTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,18 +24,23 @@
  */
 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.impl.FrameWithoutBoxing;
+import com.oracle.truffle.api.test.SubprocessTestUtils;
+
+import jdk.graal.compiler.api.directives.GraalDirectives;
+import jdk.graal.compiler.nodes.FieldLocationIdentity;
+import jdk.graal.compiler.nodes.NamedLocationIdentity;
+import jdk.graal.compiler.nodes.StructuredGraph;
+import jdk.graal.compiler.nodes.memory.ReadNode;
 
 /**
  * Tests that we can move frame array reads out of the loop even if there is a side-effect in the
@@ -59,11 +64,23 @@ public static int snippet0(FrameWithoutBoxing frame, int index) {
     }
 
     @Test
-    public void test0() {
+    public void test0() throws IOException, InterruptedException {
+        // Run in subprocess to disable assertions in FrameWithoutBoxing.
+        // Assertion checking requires additional checks in the frame descriptor for handling of
+        // static slots.
+        SubprocessTestUtils.executeInSubprocessWithAssertionsDisabled(FrameHostReadsTest.class, () -> {
+            compileAndCheck();
+        },
+                        true,
+                        List.of(FrameWithoutBoxing.class));
+    }
+
+    private void compileAndCheck() {
         getTruffleCompiler();
         initAssertionError();
         Assert.assertSame("New frame implementation detected. Make sure to update this test.", FrameWithoutBoxing.class,
                         Truffle.getRuntime().createVirtualFrame(new Object[0], FrameDescriptor.newBuilder().build()).getClass());
+        Assert.assertTrue("Frame assertions should be disabled.", !FrameAssertionsChecker.areFrameAssertionsEnabled());
 
         StructuredGraph graph = getFinalGraph("snippet0");
 
@@ -86,25 +103,21 @@ public void test0() {
         }
 
         /*
-         * Frame read for FrameWithoutBoxing.indexedTags and
-         * FrameWithoutBoxing.indexedPrimitiveLocals.
-         *
-         * Note we expect two reads here as assertions are enabled.
+         * Frame read for FrameWithoutBoxing.indexedPrimitiveLocals.
          */
-        Assert.assertEquals(2, fieldReads);
+        Assert.assertEquals(1, fieldReads);
 
         /*
          * Array reads inside of the loop. We expect the loop to get unrolled, so we expect 5 times
          * the number of reads. It is important that the loop does not contain reads to
          * FrameWithoutBoxing.indexedTags or FrameWithoutBoxing.indexedPrimitiveLocals.
          */
-        Assert.assertEquals(10, arrayReads);
+        Assert.assertEquals(5, arrayReads);
 
         /*
-         * Array.length reads. We also read two because we have assertions enabled. one for
-         * FrameWithoutBoxing.indexedTags and one for FrameWithoutBoxing.indexedPrimitiveLocals.
+         * Array.length reads. We read one for FrameWithoutBoxing.indexedPrimitiveLocals.
          */
-        Assert.assertEquals(2, arrayLengthReads);
+        Assert.assertEquals(1, arrayLengthReads);
         Assert.assertEquals(0, otherReads);
     }
 
@@ -118,5 +131,4 @@ public Throwable fillInStackTrace() {
             }
         };
     }
-
 }