While debugging a test timeout for compiler/valhalla/inlinetypes/TestIntrinsics.java with my new COH changes and enabled COH, I could trace the problem back to a wrong assumption about padding bytes in Unsafe.compareAndExchangeFlatValue() that is actually unrelated to COH.

I simplified TestIntrinsics.java and additionally modified Unsafe.java to reduce it and dump some more information and actually throw a RuntimeException to show the problem. The changes to Unsafe.java are not actually needed - the test will instead just be stuck. The changes for TestIntrinsics.java and Unsafe.java can be found in 'reduced.patch' attached to this bug.

Details with simplified TestIntrinsics.java when applying 'reduced.patch':

test70() calls:

    U.compareAndExchangeFlatValue(this, TEST63_VT_OFFSET, 4, SmallValue.class, expected, x);

where 'expected' and 'x' are both assigned by calling SmallValue.create():

    test70(SmallValue.create(), SmallValue.create());
    
And thus 'expected' and 'x' are equal with the same field values.

Unsafe.compareAndSetFlatValueAsBytes() looks like this with added comments just for this JBS issue:

    // We create two flat arrays of length 1 and put 'expected' and 'x' in the only slot - they are flattened.
    Object expectedArray = newSpecialArray(valueType, 1, layout);
    Object xArray = newSpecialArray(valueType, 1, layout);
    ...
    putFlatValue(expectedArray, base, layout, valueType, expected);
    putFlatValue(xArray, base, layout, valueType, x);
    ...
    case 8: {
        // We read the payload as a long from 'expectedArray' and 'xArray' which include the byte fields from 'expected' and 'x'.
        long expectedLong = getLong(expectedArray, base);
        long xLong = getLong(xArray, base);
        // Based on that we perform CAS.
        return compareAndSetLong(o, offset, expectedLong, xLong);
    
Now the problem is that one time SmallValue.create() is called with the interpreter in test70() which zeroes the padding bytes:

    # header
    0x00000021
    0x00000000
    0x71096c48
    # length
    0x00000001
    # payload
    0x78563412
    0x0000019a


And one time, SmallValue.create() is created by the pack handler for value objects (SharedRuntime::generate_buffered_inline_type_adapter()) which only writes the fields without zeroing:

    # header
    0x00000021
    0x00000000
    0x71096c48
    0x00000001
    # length
    0x78563412
    0xbaad019a

As a result, 'expectedLong' and 'xLong' are not equal:

    expectedLong = 0x0000019a78563412
    xLong = 0xbaad019a78563412

and we loop endlessly or throw an exception in the modified Unsafe version. If we only run the program with -Xint, then it will always use the zeroing version and the program terminates sanely.

My first thought was that we forgot to zero the padding bytes in generate_buffered_inline_type_adapter(). But then again, I could not find that there is anything specified anywhere what the padding bytes should look like. And it makes sense to leave it to the JVM implementations to decide what to do with them because it should not matter to a Java program what they are. So, it looks like we cannot assume anything about their value even though they are mostly zero.

I therefore conclude that Unsafe.compareAndExchangeFlatValue() relies on a wrong assumption that padding bytes are always the same for the same value object. The current code only seems to work for payloads of exact power of 2s but not for the odd cases like 3, 5, 6 and 7.


How to reproduce:

1) Either build the branch above with the changed Unsafe.java to get additional output and an exception or just use a build without which will result in a timeout instead.
2) Run TestIntrinsics.java with -Xint (just to be sure).
3) Look at the output and search for the command line for "Run Test VM". For me it is:
/home/chagedor/valhalla2/build/linux-x64-slowdebug/jdk/bin/java -cp /home/chagedor/JTwork/classes/compiler/valhalla/inlinetypes/TestIntrinsics.d:/home/chagedor/valhalla2/open/test/hotspot/jtreg/compiler/valhalla/inlinetypes:/home/chagedor/JTwork/classes/compiler/valhalla/inlinetypes/TestIntrinsics.d/test/lib:/home/chagedor/valhalla2/open/test/lib:/home/chagedor/valhalla2/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 -Xint -Dir.framework.server.port=34277 --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 -XX:CompileCommand=quiet -DWarmup=251 compiler.lib.ir_framework.test.TestVM compiler.valhalla.inlinetypes.TestIntrinsics
4) Remove everything from -Xint and coming afterwards and use this instead:
-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.TestIntrinsics
5) When running with the Unsafe changes, it will throw the following exception:

expected:
a: 18, b: 52
0x00000021
0x00000000
0x55096c48
0x00000001
0x78563412
0x0000019a

xArray:
a: 18, b: 52
0x00000021
0x00000000
0x55096c48
0x00000001
0x78563412
0xbaad019a
Exception in thread "main" compiler.lib.ir_framework.shared.TestRunException:

Test Failures (1)
-----------------
Custom Run Test: @Run: test70_verifier - @Test: test70:
compiler.lib.ir_framework.shared.TestRunException: There was an error while invoking @Run method public void compiler.valhalla.inlinetypes.TestIntrinsics.test70_verifier()
at compiler.lib.ir_framework.test.CustomRunTest.invokeTest(CustomRunTest.java:162)
at compiler.lib.ir_framework.test.AbstractTest.run(AbstractTest.java:100)
at compiler.lib.ir_framework.test.CustomRunTest.run(CustomRunTest.java:89)
at compiler.lib.ir_framework.test.TestVM.runTests(TestVM.java:869)
at compiler.lib.ir_framework.test.TestVM.start(TestVM.java:256)
at compiler.lib.ir_framework.test.TestVM.main(TestVM.java:169)
Caused by: java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:119)
at java.base/java.lang.reflect.Method.invoke(Method.java:565)
at compiler.lib.ir_framework.test.CustomRunTest.invokeTest(CustomRunTest.java:159)
... 5 more
Caused by: java.lang.RuntimeException:
expectedArray: 0x0000019a78563412, xArray: 0xbaad019a78563412

at java.base/jdk.internal.misc.Unsafe.compareAndSetFlatValueAsBytes(Unsafe.java:2915)
at java.base/jdk.internal.misc.Unsafe.compareAndExchangeFlatValue(Unsafe.java:1753)
at compiler.valhalla.inlinetypes.TestIntrinsics.test70(TestIntrinsics.java:100)
at compiler.valhalla.inlinetypes.TestIntrinsics.test70_verifier(TestIntrinsics.java:106)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
... 7 more



at compiler.lib.ir_framework.test.TestVM.runTests(TestVM.java:905)
at compiler.lib.ir_framework.test.TestVM.start(TestVM.java:256)
at compiler.lib.ir_framework.test.TestVM.main(TestVM.java:169)