search

eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.28k stars 721 forks source link

[JDK20/FFI_Jtreg] AssertionError captured in TestNormalize #17128

Closed ChengJin01 closed 1 year ago

ChengJin01 commented 1 year ago

A couple of AssertionError were captured in this failing test suite TestNormalize at https://github.com/ibmruntimes/openj9-openjdk-jdk20/blob/openj9/test/jdk/java/foreign/normalize/TestNormalize.java as follows:

test TestNormalize.testBool(2, true): failure
java.lang.AssertionError: expected [true] but found [false]
        at org.testng.Assert.fail(Assert.java:99)
        at org.testng.Assert.failNotEquals(Assert.java:1037)
        at org.testng.Assert.assertEqualsImpl(Assert.java:140)
        at org.testng.Assert.assertEquals(Assert.java:122)
        at org.testng.Assert.assertEquals(Assert.java:819)
        at org.testng.Assert.assertEquals(Assert.java:829)
        at TestNormalize.testBool(TestNormalize.java:191)
...
test TestNormalize.testNormalize(b8, 42, -256, MethodHandle(byte)int, MethodHandle(byte,int[])void): failure
java.lang.AssertionError: expected [0] but found [-256]
        at org.testng.Assert.fail(Assert.java:99)
        at org.testng.Assert.failNotEquals(Assert.java:1037)
        at org.testng.Assert.assertEqualsImpl(Assert.java:140)
        at org.testng.Assert.assertEquals(Assert.java:122)
        at org.testng.Assert.assertEquals(Assert.java:907)
        at org.testng.Assert.assertEquals(Assert.java:917)
        at TestNormalize.doCall(TestNormalize.java:134)
        at TestNormalize.testNormalize(TestNormalize.java:126)
...
test TestNormalize.testNormalize(s16, 42, -65536, MethodHandle(short)int, MethodHandle(short,int[])void): failure
java.lang.AssertionError: expected [0] but found [-65536]
        at org.testng.Assert.fail(Assert.java:99)
        at org.testng.Assert.failNotEquals(Assert.java:1037)
        at org.testng.Assert.assertEqualsImpl(Assert.java:140)
        at org.testng.Assert.assertEquals(Assert.java:122)
        at org.testng.Assert.assertEquals(Assert.java:907)
        at org.testng.Assert.assertEquals(Assert.java:917)
        at TestNormalize.doCall(TestNormalize.java:134)
        at TestNormalize.testNormalize(TestNormalize.java:126)
...
test TestNormalize.testNormalize(c16, 97, -65536, MethodHandle(char)int, MethodHandle(char,int[])void): failure
java.lang.AssertionError: expected [0] but found [-65536]
        at org.testng.Assert.fail(Assert.java:99)
        at org.testng.Assert.failNotEquals(Assert.java:1037)
        at org.testng.Assert.assertEqualsImpl(Assert.java:140)
        at org.testng.Assert.assertEquals(Assert.java:122)
        at org.testng.Assert.assertEquals(Assert.java:907)
        at org.testng.Assert.assertEquals(Assert.java:917)
        at TestNormalize.doCall(TestNormalize.java:134)
        at TestNormalize.testNormalize(TestNormalize.java:126)
...

FYI: @tajila, @pshipton

ChengJin01 commented 1 year ago

According to the description of the failing test at https://github.com/ibmruntimes/openj9-openjdk-jdk20/blob/openj9/test/jdk/java/foreign/normalize/TestNormalize.java#L109

// The idea of this test is that we pass a 'dirty' int value down to native code, and then receive it back
// as the argument to an upcall, as well as the result of the downcall, but with a sub-int type (boolean, byte, short, char).
// When we do either of those, argument normalization should take place, so that the resulting value is sane (1).
// After that we convert the value back to int again, the JVM can/will skip value normalization here.
// We then check the high order bits of the resulting int. If argument normalization took place at (1), they should all be 0.

It seems there might be missing code in handling the normalization for the return value in downcall and the passed-in argument in upcall. Will need to double-check the code involved to see how to deal with these cases.

ChengJin01 commented 1 year ago

With my fix in the upcall dispatcher at https://github.com/ChengJin01/openj9/commit/f320201f54076eff4d2c1958b68ee4947a4ecbec (we already addressed the return value in the existing code in downcall), the failing tests passed as expected: 

----------System.out:(12/757)----------
test TestNormalize.testBool(2, true): success
test TestNormalize.testBool(256, false): success
test TestNormalize.testNormalize(z8, 1, -2, MethodHandle(boolean)int, java.lang.invoke.DirectMethodHandle@c2f41c4b): success
test TestNormalize.testNormalize(b8, 42, -256, MethodHandle(byte)int, MethodHandle(byte,int[])void): success
test TestNormalize.testNormalize(s16, 42, -65536, MethodHandle(short)int, MethodHandle(short,int[])void): success
test TestNormalize.testNormalize(c16, 97, -65536, MethodHandle(char)int, MethodHandle(char,int[])void): success

===============================================
test/jdk/java/foreign/normalize/TestNormalize.java
Total tests run: 6, Passes: 6, Failures: 0, Skips: 0
===============================================

Will need to run personal builds & verify on all supported platforms to see how it goes.