RFR: 8304006: jlink should create the jimage file in the native endian for the target platform [v10]

[Mandy Chung]

On Mon, 20 Mar 2023 14:21:55 GMT, Jaikiran Pai <jpai at 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:
> 
>   Alan's suggestions - don't parse arch out of osname-arch for determining endianness and reduce the number of supported/known target platforms for cross linking

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 548:

> 546:                                                      Path retainModulesPath,
> 547:                                                      boolean ignoreSigning,
> 548:                                                      boolean bindService,

formatting nit: align the parameters with the first one.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 822:

> 820:                     PrintWriter log) throws IOException {
> 821:             if (order != null) {
> 822:                 this.targetPlatform = Platform.runtime();

The target platform should always be fetched from `java.base` (as it was implemented in `DefaultImageBuilder`) or use the current runtime if `java.base` is from the module path of the current runtime.

If `--endian <order>` is specified, jlink can check if the target platform's endianness matches the specified value and emit an error if mismatch unless `Platform::getNativeByteOrder` is null.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 856:

> 854:                         // unsupported target platform
> 855:                         throw new IOException(
> 856:                                 taskHelper.getMessage("err.unsupported.target.platform",

IIUC, the current jlink implementation does not fail if the target platform is unknown as long as `--endian` option is specified.   So I think `--endian` may be useful to keep (rather than taking it out).   This error key should be `err.unknown.target.endianness` and the message can suggest to use `--endian` option to specify.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Platform.java line 192:

> 190:             case "s390x"           -> Architecture.s390x;
> 191:             case "sparc"           -> Architecture.SPARC;
> 192:             case "sparcv9"         -> Architecture.SPARCv9;

This list should be trimmed to the ones supported in the mainline as in `target.properties`.

src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties line 151:

> 149: err.signing=signed modular JAR {0} is currently not supported,\
> 150: \ use --ignore-signing-information to suppress error
> 151: err.cannot.determine.target.platform=cannot determine target platform for image generation

Suggestion:

err.cannot.determine.target.platform=cannot determine target platform from {0}


`{0}` can be the path to `java.base.jmod`  (i.e. `ModuleReference::location`)

src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties line 152:

> 150: \ use --ignore-signing-information to suppress error
> 151: err.cannot.determine.target.platform=cannot determine target platform for image generation
> 152: err.unsupported.target.platform=image generation for target platform {0} is not supported

Suggestion:

err.unsupported.target.endianness=Unknown native byte order for target platform {0}`


`{0}` is `targetPlatformVal`

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1143813371
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1143823833
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1143872844
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1143847888
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1143875777
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1143877925