8368274: [lworld] Unsafe.compareAndExchangeFlatValue() wrongly assumes padding bytes are the same for identical value objects

src/java.base/share/classes/jdk/internal/misc/Unsafe.java

@@ -25,7 +25,6 @@
 
 package jdk.internal.misc;
 
-import jdk.internal.value.ValueClass;
 import jdk.internal.vm.annotation.AOTRuntimeSetup;
 import jdk.internal.vm.annotation.AOTSafeClassInitializer;
 import jdk.internal.vm.annotation.ForceInline;
@@ -1748,15 +1747,8 @@ public final <V> boolean compareAndSetFlatValue(Object o, long offset,
                                                 Class<?> valueType,
                                                 V expected,
                                                 V x) {
-        while (true) {
-            Object witness = getFlatValueVolatile(o, offset, layout, valueType);
-            if (witness != expected) {
-                return false;
-            }
-            if (compareAndSetFlatValueAsBytes(o, offset, layout, valueType, witness, x)) {
-                return true;
-            }
-        }
+        Object[] array = newSpecialArray(valueType, 2, layout);
+        return compareAndSetFlatValueAsBytes(array, o, offset, layout, valueType, expected, x);
     }
 
     @IntrinsicCandidate
@@ -1790,15 +1782,8 @@ public final <V> Object compareAndExchangeFlatValue(Object o, long offset,
                                                     Class<?> valueType,
                                                     V expected,
                                                     V x) {
-        while (true) {
-            Object witness = getFlatValueVolatile(o, offset, layout, valueType);
-            if (witness != expected) {
-                return witness;
-            }
-            if (compareAndSetFlatValueAsBytes(o, offset, layout, valueType, witness, x)) {
-                return witness;
-            }
-        }
+        Object[] array = newSpecialArray(valueType, 2, layout);
+        return compareAndSetFlatValueAsBytes(array, o, offset, layout, valueType, expected, x) ? x : array[0];
     }
 
     @IntrinsicCandidate
@@ -2914,38 +2899,76 @@ public final void putDoubleOpaque(Object o, long offset, double x) {
     }
 
     @ForceInline
-    private boolean compareAndSetFlatValueAsBytes(Object o, long offset, int layout, Class<?> valueType, Object expected, Object x) {
+    private boolean compareAndSetFlatValueAsBytes(Object[] array, Object o, long offset, int layout, Class<?> valueType, Object expected, Object x) {
         // We turn the payload of an atomic value into a numeric value (of suitable type)
         // by storing the value into an array element (of matching layout) and by reading
         // back the array element as an integral value. After which we can implement the CAS
         // as a plain numeric CAS. Note: this only works if the payload contains no oops
         // (see VarHandles::isAtomicFlat).
-        Object[] expectedArray = newSpecialArray(valueType, 1, layout);
-        Object xArray = newSpecialArray(valueType, 1, layout);
-        long base = arrayInstanceBaseOffset(expectedArray);
-        int scale = arrayInstanceIndexScale(expectedArray);
-        putFlatValue(expectedArray, base, layout, valueType, expected);
-        putFlatValue(xArray, base, layout, valueType, x);
+
+        // array 0: witness (to translate to object), 1: x (to translate to raw)
+        // caller pass the array so it can capture the witness if needed
+        // we must witness the raw value instead of the value object, otherwise
+        // garbage value can cause failure
+        long base = arrayInstanceBaseOffset(array);
+        int scale = arrayInstanceIndexScale(array);
+        putFlatValue(array, base + scale, layout, valueType, x); // translate x to raw bytes
         switch (scale) {
             case 1: {
-                byte expectedByte = getByte(expectedArray, base);
-                byte xByte = getByte(xArray, base);
-                return compareAndSetByte(o, offset, expectedByte, xByte);
+                do {
+                    byte witnessByte = getByteVolatile(o, offset);
+                    putByte(array, base, witnessByte); // translate witness to value object
+                    Object witness = getFlatValue(array, base, layout, valueType);
+                    if (witness != expected) {
+                        return false;
+                    }
+                    byte xByte = getByte(array, base + scale);
+                    if (compareAndSetByte(o, offset, witnessByte, xByte)) {
+                        return true;
+                    }
+                } while (true);
             }
             case 2: {
-                short expectedShort = getShort(expectedArray, base);
-                short xShort = getShort(xArray, base);
-                return compareAndSetShort(o, offset, expectedShort, xShort);
+                do {
+                    short witnessShort = getShortVolatile(o, offset);
+                    putShort(array, base, witnessShort); // translate witness to value object
+                    Object witness = getFlatValue(array, base, layout, valueType);
+                    if (witness != expected) {
+                        return false;
+                    }
+                    short xShort = getShort(array, base + scale);
+                    if (compareAndSetShort(o, offset, witnessShort, xShort)) {
+                        return true;
+                    }
+                } while (true);
             }
             case 4: {
-                int expectedInt = getInt(expectedArray, base);
-                int xInt = getInt(xArray, base);
-                return compareAndSetInt(o, offset, expectedInt, xInt);
+                do {
+                    int witnessInt = getIntVolatile(o, offset);
+                    putInt(array, base, witnessInt); // translate witness to value object
+                    Object witness = getFlatValue(array, base, layout, valueType);
+                    if (witness != expected) {
+                        return false;
+                    }
+                    int xInt = getInt(array, base + scale);
+                    if (compareAndSetInt(o, offset, witnessInt, xInt)) {
+                        return true;
+                    }
+                } while (true);
             }
             case 8: {
-                long expectedLong = getLong(expectedArray, base);
-                long xLong = getLong(xArray, base);
-                return compareAndSetLong(o, offset, expectedLong, xLong);
+                do {
+                    long witnessLong = getLongVolatile(o, offset);
+                    putLong(array, base, witnessLong); // translate witness to value object
+                    Object witness = getFlatValue(array, base, layout, valueType);
+                    if (witness != expected) {
+                        return false;
+                    }
+                    long xLong = getLong(array, base + scale);
+                    if (compareAndSetLong(o, offset, witnessLong, xLong)) {
+                        return true;
+                    }
+                } while (true);
             }
             default: {
                 throw new UnsupportedOperationException();

test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestCompareAndExchange.java

@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2025, 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.
+ *
+ * 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 compiler.valhalla.inlinetypes;
+
+import compiler.lib.ir_framework.*;
+import jdk.internal.misc.Unsafe;
+
+import java.lang.reflect.Field;
+
+/*
+ * @test
+ * @key randomness
+ * @summary Test intrinsic support for value classes.
+ * @library /test/lib /
+ * @requires (os.simpleArch == "x64" | os.simpleArch == "aarch64")
+ * @enablePreview
+ * @modules java.base/jdk.internal.misc
+ *          java.base/jdk.internal.value
+ *          java.base/jdk.internal.vm.annotation
+ * @build jdk.test.whitebox.WhiteBox
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
+ * @run main/othervm/timeout=300 -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ *      compiler.valhalla.inlinetypes.TestCompareAndExchange
+ */
+
+/*
+ * My command line: java -cp /home/chagedor/JTwork/classes/compiler/valhalla/inlinetypes/TestCompareAndExchange.d:/home/chagedor/valhalla3/open/test/hotspot/jtreg/compiler/valhalla/inlinetypes:/home/chagedor/JTwork/classes/compiler/valhalla/inlinetypes/TestCompareAndExchange.d/test/lib:/home/chagedor/valhalla3/open/test/lib:/home/chagedor/valhalla3/open/test/hotspot/jtreg:/home/chagedor/jtreg/lib/javatest.jar:/home/chagedor/jtreg/lib/jtreg.jar -Djava.library.path=. -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Dir.framework.server.port=35391 --enable-preview --add-exports java.base/jdk.internal.value=ALL-UNNAMED --add-exports java.base/jdk.internal.vm.annotation=ALL-UNNAMED --add-exports java.base/jdk.internal.misc=ALL-UNNAMED -XX:-BackgroundCompilation -DWarmup=10000 -XX:CompileCommand=dontinline,*Unsafe*::getFlatValue* -XX:CompileCommand=dontinline,*Unsafe*::putFlatValue* -DIgnoreCompilerControls=true -XX:-TieredCompilation -XX:CompileOnly=*Small*::*,*::test70 -XX:CompileCommand=dontinline,*Unsafe::array* -XX:DisableIntrinsic=_compareAndSetLong,_compareAndSetInt,_compareAndSetByte,_compareAndSetShort -DReproduce=true compiler.lib.ir_framework.test.TestVM compiler.valhalla.inlinetypes.TestCompareAndExchange
+ */
+@ForceCompileClassInitializer
+public class TestCompareAndExchange {
+
+
+    public static void main(String[] args) {
+        InlineTypes.getFramework()
+                .addFlags("--enable-preview",
+                          "--add-exports", "java.base/jdk.internal.value=ALL-UNNAMED",
+                          "--add-exports", "java.base/jdk.internal.vm.annotation=ALL-UNNAMED",
+                          "--add-exports", "java.base/jdk.internal.misc=ALL-UNNAMED")
+                .addFlags("-XX:-BackgroundCompilation -DWarmup=10000 -XX:CompileCommand=dontinline,*Unsafe*::getFlatValue* -XX:CompileCommand=dontinline,*Unsafe*::putFlatValue* -DIgnoreCompilerControls=true -XX:-TieredCompilation -XX:CompileOnly=*Small*::*,*::test70 -XX:CompileCommand=dontinline,*Unsafe::array* -XX:DisableIntrinsic=_compareAndSetLong,_compareAndSetInt,_compareAndSetByte,_compareAndSetShort -DReproduce=true".split(" "))
+                .start();
+    }
+
+    private static final Unsafe U = Unsafe.getUnsafe();
+
+    static public value class SmallValue {
+        byte a = (byte)0x12;
+        byte b = (byte)0x34;
+        byte c = (byte)0x56;
+        byte d = (byte)0x78;
+        byte e = (byte)0x9a;
+
+        @ForceInline
+        static SmallValue create() {
+            return new SmallValue();
+        }
+
+        @Override
+        public String toString() {
+            return "a: " + a + ", b: " + b;
+        }
+    }
+
+    SmallValue test63_vt;
+    private static final long TEST63_VT_OFFSET;
+    static {
+        try {
+            Field test63_vt_Field = TestCompareAndExchange.class.getDeclaredField("test63_vt");
+            TEST63_VT_OFFSET = U.objectFieldOffset(test63_vt_Field);
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    // compareAndExchange to flattened field in object, non-inline arguments to compare and set
+    @Test
+    public Object test70(Object expected, Object x) {
+        return U.compareAndExchangeFlatValue(this, TEST63_VT_OFFSET, 4, SmallValue.class, expected, x);
+    }
+
+    @Run(test = "test70")
+    public void test70_verifier() {
+        test63_vt = SmallValue.create();
+        test70(SmallValue.create(), SmallValue.create());
+    }
+}