openj9
openj9 copied to clipboard
Increment stackSlotCount for structs correctly in FFI Upcall on z/OS
This commit fixes a bug in FFI Upcall on z/OS where the counter for stack slots in memory is not always increased when iterating through arguments. Specifically, the counter was not being incremented when structs were present in the argument list. It will now be incremented correctly for every argument.
jenkins test sanity jdk21 zlinux
Jenkins line endings check
@dchopra001 Can you share the build number of the hyc jenkins build for z/OS ? Given that this is z/OS specific code, I do not think, I need to launch another build (Unless @keithc-ca thinks otherwise).
I do not think, I need to launch another build
I agree, assuming you verified that internal z/OS build was good.
I did launch an internal build yesterday that shows UpcallThunkGen.cpp was compiled successfully. However the build didn't complete due to the following issue:
00:52:29 make[10]: *** duping jobs pipe: EDC5129I No such file or directory.. Stop.
00:52:29 make[10]: *** Waiting for unfinished jobs....
00:52:29 make[9]: *** [CMakeFiles/Makefile2:83: CMakeFiles/j9ddr.dir/all] Error 2
00:52:29 make[8]: *** [Makefile:91: all] Error 2
00:52:29 make[7]: *** [runtime/CMakeFiles/j9ddr.dir/build.make:70: runtime/CMakeFiles/j9ddr] Error 2
00:52:29 make[6]: *** [CMakeFiles/Makefile2:3502: runtime/CMakeFiles/j9ddr.dir/all] Error 2
00:52:29 make[5]: *** [CMakeFiles/Makefile2:3509: runtime/CMakeFiles/j9ddr.dir/rule] Error 2
00:52:29 make[4]: *** [Makefile:264: runtime/CMakeFiles/j9ddr.dir/rule] Error 2
00:52:29 make[3]: *** [/jenkins/workspace/Build_JDK21_s390x_zos_Nightly/closed/OpenJ9.gmk:599: build-j9] Error 2
00:52:29 make[2]: *** [/jenkins/workspace/Build_JDK21_s390x_zos_Nightly/closed/custom/Main.gmk:49: j9vm-build] Error 2
I will try launching another one and confirm when it passes.
Still seeing the same issue:
14:52:52 20 warnings generated.
14:53:00 java version "21.0.4-internal" 2024-07-16
14:53:00 IBM Semeru Runtime Certified Edition for z/OS (build 21.0.4-internal-adhoc.JENKINS.BuildJDK21s390xzosNightly)
14:53:00 IBM J9 VM (build ffi_incStackCounterForStructs_internal-8cd5fb6ea9e, JRE 21 z/OS s390x-64-Bit Compressed References 20241011_292 (JIT enabled, AOT enabled)
14:53:00 OpenJ9 - 8cd5fb6ea9e
14:53:00 OMR - 55ddfd47ab0
14:53:00 IBM - 3c87141
14:53:00 JCL - 72d099bfae1 based on jdk-21.0.4+7)
14:53:39 Compiling up to 3 files for BUILD_DEMO_CodePointIM
14:53:39 Updating support/demos/image/jfc/CodePointIM/src.zip
...
...
...
...
14:55:04 Creating jdk image
14:55:31 make[3]: *** duping jobs pipe: EDC5113I Bad file descriptor.. Stop.
14:55:31 make[3]: *** Waiting for unfinished jobs....
14:55:31 make[2]: *** [/jenkins/workspace/Build_JDK21_s390x_zos_Nightly/closed/custom/Main.gmk:68: debug-image] Error 2
14:55:31 make[2]: *** Waiting for unfinished jobs....
I've launched another one. Still waiting on results.
@r30shah Build finally passed so this should be good to merge (URL: view/Nightly/job/Build_JDK21_s390x_zos_Nightly/294/)
The build referenced above (https://github.com/eclipse-openj9/openj9/pull/20322#issuecomment-2408287911) doesn't seem to have used the changes here (it consumed 8cd5fb6ea9e which I don't see in the history of this pull request); that build also didn't run any testing. Please launch a new build, making sure that it uses https://github.com/eclipse-openj9/openj9/commit/99f0667b4089a63c1f66ab018c1b0a9c0e171e0e and does FFI testing at least.
The build referenced above (https://github.com/eclipse-openj9/openj9/pull/20322#issuecomment-2408287911) doesn't seem to have used the changes here (it consumed 8cd5fb6ea9e which I don't see in the history of this pull request); that build also didn't run any testing. Please launch a new build, making sure that it uses https://github.com/eclipse-openj9/openj9/commit/99f0667b4089a63c1f66ab018c1b0a9c0e171e0e and does FFI testing at least.
Since I launched an internal build, I was using an internal branch for testing. That's why it's not seen here. I did internal testing as well with the original fix before opening the PR and documented the results of the tests in our internal GitHub here: runtimes/openj9-openjdk-jdk21-zos/issues/423#issuecomment-93599926.
The review changes requested here weren't revolving around the problem that this PR was intending to fix. Given the nature of the changes requested, I don't expect them to cause any new failures. I have launched another build and tests. I'll update here with links once it's complete.
@dchopra001 What is the status of your internal build ?
I'm seeing some infra related errors when trying to build the JDK on Jenkins:
Failed fetching commit message from git directory: /jenkins/workspace/Build_JDK21_s390x_zos_Nightly/.git
15:53:51 With the following error: ���z@��������@���@�����
I'll try again today until I get a successful compilation.
I see that couple of attempts from @dchopra001 have failed in getting the build, and the internal build that he had launched only misses last two changes recommended (After this comment https://github.com/eclipse-openj9/openj9/pull/20322#issuecomment-2408287911), which I think should make code more resilient with fixing Signed vs unsigned comparison.
I think these changes are safe to be merged (Only z/OS related changes), @keithc-ca if you agree with this.
@keithc-ca Ping to see if you are OK with this one getting merged?
I'd still like to see a build and some testing with this change.
@keithc-ca URLs for internal Jenkins (hyc-runtimes-jenkins) jobs:
Here's the build: view/Nightly/job/Build_JDK21_s390x_zos_Nightly/319/
Here's the tests: job/Grinder_CR/25682/
The failures are unrelated and documented/explained in runtimes/openj9-openjdk-jdk21-zos/issues/423#issuecomment-93599926
The change only affects 64-bit z/OS builds - tested internally.