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

Test SCC range checks #12141

Open tajila opened 3 years ago

tajila commented 3 years ago

We had an issue last year where the SCC range checks caused a gpf. None of our tests where able to catch this issue:

        j9vm29!j9shr_Query_IsAddressInCache+0x29
        j9vm29!ComparingCursor::isRangeValidForUTF8Ptr()
        j9vm29!ComparingCursor::writeSRP()
        j9vm29!ROMClassWriter::ConstantPoolWriter::visitString()
        j9vm29!ConstantPoolMap::constantPoolDo()
        j9vm29!ROMClassWriter::writeConstantPool+0x7a
        j9vm29!ROMClassWriter::writeROMClass()
        j9vm29!ROMClassBuilder::compareROMClassForEquality+0x136
        j9vm29!ROMClassBuilder::prepareAndLaydown()
        j9vm29!ROMClassBuilder::buildROMClass()
        j9vm29!j9bcutil_buildRomClass()

https://github.com/eclipse/openj9/issues/11504

We need to introduce a test that ensures core SCC functionality such as this is working.

tajila commented 3 years ago

@hangshao0 can you add a test that would have exposed this issue?

hangshao0 commented 3 years ago

The crash actually happened in ComparingCursor when it is using J9UTF8_TOTAL_SIZE() to get the utf8 size, before calling into SCC API j9shr_Query_IsAddressInCache().

ComparingCursor::isRangeValidForUTF8Ptr(J9UTF8 *utf8)
{
    ......
    return FALSE != j9shr_Query_IsAddressInCache(_javaVM, utf8, J9UTF8_TOTAL_SIZE(utf8));

It needs to be a test case that passes a "bad" utf8 to ComparingCursor::isRangeValidForUTF8Ptr(). It will be easy if we have native test suite for ComparingCursor. If not, it may not be easy to let JVM generate a bad utf8 to ComparingCursor.

tajila commented 3 years ago

Can be done by adding a new test to /openj9/test/functional/NativeTest/playlist.xml with new test cases in /openj9/runtime/tests/shared ?

hangshao0 commented 3 years ago

The native tests in /openj9/runtime/tests/shared are covering the native code in the j9shr29 dll. The crash here was not in j9shr29 dll. It was ComparingCursor in j9vm29 (before calling into the actual SCC API). If we want to add a new test, it should be in native test suite for things in j9vm29 dll.

tajila commented 3 years ago

Okay, we have native tests for j9vm29 in the /openj9/runtime/tests folder. We usually add a jni entry point in openj9/runtime/tests/jni so you can load System.LoadLibrary("j9ben") in java code and call a native in j9vm29 that way. See openj9/test/functional/Java8andUp/src/j9vm/test/monitor/Helpers.java for an example.