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

Reviewed-by: mcimadamore

Author: liach

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;
@@ -1711,15 +1710,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
@@ -1753,15 +1745,9 @@ 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);
+        compareAndSetFlatValueAsBytes(array, o, offset, layout, valueType, expected, x);
+        return array[0];
     }
 
     @IntrinsicCandidate
@@ -2877,38 +2863,75 @@ 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) {
-        // 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);
+    private boolean compareAndSetFlatValueAsBytes(Object[] array, Object o, long offset, int layout, Class<?> valueType, Object expected, Object x) {
+        // We can convert between a value object and a binary value (of suitable size) using array elements.
+        // This only works if the payload contains no oops (see VarHandles::isAtomicFlat).
+        // Thus, we can implement the CAS with a plain numeric CAS.
+
+        // array[0]: witness (put as binary, get as object), at base
+        // array[1]: x (put as object, get as binary), at base + scale
+        // When witness == expected, the witness binary may be different from the expected binary.
+        // This happens when compiler does not zero unused positions in the witness.
+        // So we must obtain the witness binary and use it as expected binary for the numeric CAS.
+        long base = arrayInstanceBaseOffset(array);
+        int scale = arrayInstanceIndexScale(array);
+        putFlatValue(array, base + scale, layout, valueType, x); // put x as object
         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); // put witness as binary
+                    Object witness = getFlatValue(array, base, layout, valueType); // get witness as object
+                    if (witness != expected) {
+                        return false;
+                    }
+                    byte xByte = getByte(array, base + scale); // get x as binary
+                    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); // put witness as binary
+                    Object witness = getFlatValue(array, base, layout, valueType); // get witness as object
+                    if (witness != expected) {
+                        return false;
+                    }
+                    short xShort = getShort(array, base + scale); // get x as binary
+                    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); // put witness as binary
+                    Object witness = getFlatValue(array, base, layout, valueType); // get witness as object
+                    if (witness != expected) {
+                        return false;
+                    }
+                    int xInt = getInt(array, base + scale); // get x as binary
+                    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); // put witness as binary
+                    Object witness = getFlatValue(array, base, layout, valueType);
+                    if (witness != expected) {
+                        return false;
+                    }
+                    long xLong = getLong(array, base + scale); // get x as binary
+                    if (compareAndSetLong(o, offset, witnessLong, xLong)) {
+                        return true;
+                    }
+                } while (true);
             }
             default: {
                 throw new UnsupportedOperationException();

test/jdk/ProblemList.txt

@@ -796,7 +796,6 @@ tools/sincechecker/modules/jdk.management.jfr/JdkManagementJfrCheckSince.java 83
 
 # valhalla
 build/CheckFiles.java 8376088  generic-all
-java/lang/invoke/VarHandles/VarHandleTestMethodHandleAccessValue.java 8367346 generic-all
 
 jdk/classfile/AccessFlagsTest.java 8366270 generic-all
 jdk/jfr/event/runtime/TestClassLoaderStatsEvent.java 8366820 generic-all