openj9 icon indicating copy to clipboard operation
openj9 copied to clipboard

jdk22 java/foreign/TestDowncallScope.java TestDowncallStack.java ASSERTION FAILED memoryCorruptionDetected

Open pshipton opened this issue 1 year ago • 1 comments

https://openj9-jenkins.osuosl.org/job/Test_openjdk22_j9_sanity.openjdk_ppc64_aix_Nightly_testList_2/4 https://openj9-jenkins.osuosl.org/job/Test_openjdk22_j9_sanity.openjdk_ppc64_aix_Nightly_testList_0/4 jdk_foreign java/foreign/TestDowncallScope.java java/foreign/TestDowncallStack.java

https://openj9-artifactory.osuosl.org/artifactory/ci-openj9/Test/Test_openjdk22_j9_sanity.openjdk_ppc64_aix_Nightly_testList_0/4/openjdk_test_output.tar.gz

03:27:36  08:14:50.695 0x10022364500 omrport.359    *   ** ASSERTION FAILED ** at /home/jenkins/workspace/Build_JDK22_ppc64_aix_Nightly/omr/port/common/omrmemtag.c:145: ((memoryCorruptionDetected))

@ChengJin01 fyi

pshipton avatar Feb 12 '24 19:02 pshipton

It is weird that the issue with downcall or upcall at https://github.com/eclipse-openj9/openj9/issues/18941, https://github.com/eclipse-openj9/openj9/issues/18942, https://github.com/eclipse-openj9/openj9/issues/18943 never occurred on other platforms or in JDK21. It is most likely something new happens to the JD22 test suites or something else we were unaware of before on AIX.

ChengJin01 avatar Feb 12 '24 19:02 ChengJin01

The crash was triggered by the following code that was added recently in test to enforce the 8-byte alignment for double in struct at https://github.com/ibmruntimes/openj9-openjdk-jdk22/blob/687a1453366566440768237f7c69361b2ec21406/test/jdk/java/foreign/shared.h#L37

#ifdef _AIX
#pragma align (natural)  <-------------
#endif
...
#ifdef _AIX
#pragma align (reset) <-------------
#endif

as mentioned at the new code on AIX at https://github.com/ibmruntimes/openj9-openjdk-jdk22/blob/687a1453366566440768237f7c69361b2ec21406/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/aix/AixPPC64Linker.java#L60

    protected void checkStructMember(MemoryLayout member, long offset) {
        // special case double members that are not the first member
        // see: https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes <-----
        // Note: It is possible to enforce 8-byte alignment by #pragma align (natural)
        // Therefore, we use normal checks if we are already 8-byte aligned.
        if ((offset % 8 != 0) && (member instanceof ValueLayout vl && vl.carrier() == double.class)) {
            if (vl.byteAlignment() != 4) {
                throw new IllegalArgumentException("double struct member " + vl + " at offset " + offset + " should be 4-byte aligned");
            }
            if (vl.order() != linkerByteOrder()) {
                throw new IllegalArgumentException("double struct member " + vl + " at offset " + offset + " has an unexpected byte order");
            }
        } else {
            super.checkStructMember(member, offset);
        }
    }

which conflicts with our previous changes in OpenJDK at https://github.com/ibmruntimes/openj9-openjdk-jdk22/blob/687a1453366566440768237f7c69361b2ec21406/src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java#L292

        public static OfDouble of(ByteOrder order) {
            return new OfDoubleImpl(order, Utils.IS_AIX ? 4 : ADDRESS_SIZE_BYTES, Optional.empty()); <---------
        } 

        @Override
        public boolean hasNaturalAlignment() {
            return Utils.IS_AIX ? (byteAlignment() == 4) : super.hasNaturalAlignment(); <--------
        }
    }

As discussed at https://github.com/eclipse-openj9/openj9/issues/18287#issuecomment-1769220477, we should always allow the default setting by the compiler and never assume that users are aware of the platform-specific difference by imposing #pragma in the applications for AIX.

To address the issue with alignment on double on AIX, I'd suggest to remove the #pragma related test code in the test header file without any modification in our code as to the 4-byte alignment adjustment on double.

FYI: @keithc-ca

ChengJin01 avatar Mar 06 '24 23:03 ChengJin01

The AIX compiler leaves us in a difficult position. We need to support types which use the default (power?) alignment rules as well as "natural" alignment. I don't think we should undo the fixes we've made to handle the default alignment rules.

I agree those pragma lines should be removed, unless tests specifically account for the non-default alignment.

keithc-ca avatar Mar 07 '24 15:03 keithc-ca

Close the issue as resolved via https://github.com/ibmruntimes/openj9-openjdk-jdk22/pull/32.

ChengJin01 avatar Mar 08 '24 23:03 ChengJin01