On Sat, 11 Mar 2023 12:16:04 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review for this change which proposes to fix the issue >> reported in https://bugs.openjdk.org/browse/JDK-8206890? >> >> The `jlink` command allows a `--endian` option to specify the byte order in >> the generated image. Before this change, when such a image was being >> launched, the code would assume the byte order in the image to be the native >> order of the host where the image is being launched. That would result in >> failure to launch java, as noted in the linked issue. >> >> The commit in this PR, changes relevant places to not assume native order >> and instead determine the byte order by reading the magic bytes in the image >> file's header content. >> >> A new jtreg test has been added which reproduces the issue and verifies the >> fix. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > missed ppc64 from the arch mapping Thanks for dropping the changes to the jimage code and changing this to choose the endianness when cross linking. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 827: > 825: if (ref instanceof ModuleReferenceImpl modRefImpl > 826: && modRefImpl.moduleTarget() != null) { > 827: targetPlatform = > modRefImpl.moduleTarget().targetPlatform(); It might be better to only look for the ModuleTarget attribute cross linking. modsPaths.get("java.base") will give you the path to the packaged module for java.base, getDefaultModulePath() will give you the location of the packaged modules in the current execution environment. If getDefaultModulePath() is the parent of the location containing java.base then it can use the current endianness. Also just to say that if it would be an error if java.base is not in the Configuration, same thing if there is no packaged module for java.base. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 928: > 926: "s390x", "sh", > 927: "sparc", "sparcv9" -> ByteOrder.BIG_ENDIAN; > 928: default -> ByteOrder.nativeOrder(); When cross linking and the target platform is unknown then it might be better to have this fail rather than defaulting to the endianness of the current platform, otherwise this risks create a run-time image that is not usable. I see you've listed a bunch of unknown platforms in the list. If getNativeEndianOfTargetPlatform fails when it encounters an unknown platform then help downstream porters to know that there is something to look at here. ------------- PR: https://git.openjdk.org/jdk/pull/11943
