8352689: Allow for hash sum overrides when linking from the run-time image

[jerboaa]

Please review this enhancement which adds a hidden jlink option --sha-overrides that can be used to provide alternative hash sums for files in an image. Please see the bug for use-cases as to why this is needed. This patch allows for the --sha-overrides option to be either specified multiple times or separated by a comma to list multiple hash sum overrides. Alternatively when starting with the @ character the override options come from a file. The format is the same: <module>|<file-path>|<sha> triplets, either on the command line or in a file (one per line).

The added, linux only, test uses objcopy to fully strip libjvm.so of its symbols with the assumption that this would change the hash sum of the resulting file. Then, a link from the run-time image is being attempted with the added option --sha-overrides=java.base|lib/server/libjvm.so|<new-sha-sum> and verifies the link succeeds.

Having something like that is useful when it gets combined with e.g. --save-jlink-arg-files to produce a jlink which works out of the box on say JDK builds that modify binaries due to some debug symbols handling outside the JDK builds' control.

While using --ignore-modified-runtime is an option that is sub-optimal as that would spit out many warnings in the RPM build case, where the user wouldn't control that RPM build to begin with.

Testing:

• GHA
• Some manual tests together with JDK-8352692 on some JEP 493 enabled builds.
• jlink jtreg tests.

Thoughts?

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8352689: Allow for hash sum overrides when linking from the run-time image (Enhancement - P3)

Reviewers

• Christoph Langer (@RealCLanger - Reviewer) 🔄 Re-review required (review applies to e387bd64)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24190/head:pull/24190
$ git checkout pull/24190

Update a local copy of the PR:
$ git checkout pull/24190
$ git pull https://git.openjdk.org/jdk.git pull/24190/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24190

View PR using the GUI difftool:
$ git pr show -t 24190

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24190.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back sgehwolf! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@jerboaa This change is no longer ready for integration - check the PR body for details.

[openjdk]

@jerboaa The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (dbd7fe09)
• 01: Full - Incremental (e387bd64)
• 00: Full (a1dbb6d9)

[RealCLanger]

This looks nice and should help to address the issue with stripped pdb files on Windows, amongst others.

The only review thing I could find were the copyright years which need updates.

[jerboaa]

The only review thing I could find were the copyright years which need updates.

I've fixed copyright years in the latest update.

[jerboaa]

This now also has support for substitution of a JAVA_HOME token to read the overrides from a file in the JDK tree. That is useful if that overrides file needs to be baked into the JDK build, but the actual hash sums aren't yet known at the time. The idea is to allow for a flexible enough setup to produce a jlink possibly with needed sha-overrides built in, yet coming from an external file that can be created after (once the actual check sums are actually known). Test has been added for it.

[jerboaa]

@AlanBateman Would we need a CSR for this? This isn't any option that shows up anywhere user-visible, so I'm thinking not. Please let me know. Thanks!

[RealCLanger]

Still good.

[AlanBateman]

/reviewers 2

[openjdk]

@AlanBateman
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[AlanBateman]

@AlanBateman Would we need a CSR for this? This isn't any option that shows up anywhere user-visible, so I'm thinking not. Please let me know. Thanks!

There isn't any change to a supported interface so probably not. That said, I don't think we should integrate this change without further discussion and enumeration of the issues.

In the case of cacerts (mentioned in the JBS issue) then this requires more information to understand if it's "use system" or something is updating the cacerts in the JDK run-time image. I think TZ updates should be looked at too

[jerboaa]

@AlanBateman Would we need a CSR for this? This isn't any option that shows up anywhere user-visible, so I'm thinking not. Please let me know. Thanks!

There isn't any change to a supported interface so probably not.

Thanks.

That said, I don't think we should integrate this change without further discussion and enumeration of the issues.

Sure.

In the case of cacerts (mentioned in the JBS issue) then this requires more information to understand if it's "use system" or something is updating the cacerts in the JDK run-time image. I think TZ updates should be looked at too

The cacerts issue mentioned in the JBS issue relates to an RPM installation of the JDK where the cacerts file is a symlink to the distro provided one. So I think that's "use system" issue.

TZ updates would potentially break this too. If an external tool updates tzdb.dat then the hash sum computed at JDK build-time will no longer match. I believe this could also be solved with a sha-override (e.g. by coming from a file @${java.home}/sha-override.txt) which would get the update along with tzdb.dat.

[AlanBateman]

The cacerts issue mentioned in the JBS issue relates to an RPM installation of the JDK where the cacerts file is a symlink to the distro provided one. So I think that's "use system" issue.

TZ updates would potentially break this too. If an external tool updates tzdb.dat then the hash sum computed at JDK build-time will no longer match. I believe this could also be solved with a sha-override (e.g. by coming from a file @${java.home}/sha-override.txt) which would get the update along with tzdb.dat.

I think one direction to explore is configuring which files or directory are "upgradable". Upgradable modules aren't excluded from the hash computation to allow upgrade via the upgrade module path. Something similar here would allow jlink to report that tzdb.dat has been upgraded. Maybe cacerts is the same but need to look closer as the hash computation shouldn't be following sym links outside of the runtime image.

[jerboaa]

The cacerts issue mentioned in the JBS issue relates to an RPM installation of the JDK where the cacerts file is a symlink to the distro provided one. So I think that's "use system" issue.
TZ updates would potentially break this too. If an external tool updates tzdb.dat then the hash sum computed at JDK build-time will no longer match. I believe this could also be solved with a sha-override (e.g. by coming from a file @${java.home}/sha-override.txt) which would get the update along with tzdb.dat.

I think one direction to explore is configuring which files or directory are "upgradable". Upgradable modules aren't excluded from the hash computation to allow upgrade via the upgrade module path. Something similar here would allow jlink to report that tzdb.dat has been upgraded. Maybe cacerts is the same but need to look closer as the hash computation shouldn't be following sym links outside of the runtime image.

I'll keep looking into this specific case. However, it sounds a bit orthogonal to the patch at hand which I do believe we still need for the original reasons mentioned (RPM changing binaries after the JDK build is long done and the windows issue of the JDK build itself placing different *.pdb files into the image than was present at jlink time). So perhaps we should explore this in parallel?

[AlanBateman]

I'll keep looking into this specific case. However, it sounds a bit orthogonal to the patch at hand which I do believe we still need for the original reasons mentioned (RPM changing binaries after the JDK build is long done and the windows issue of the JDK build itself placing different *.pdb files into the image than was present at jlink time). So perhaps we should explore this in parallel?

I think upgradable files is something we can deal with. I'm not sure yet on the PDB issue, need to think more about about the scenarios to see what might make sense.

[RealCLanger]

I'll keep looking into this specific case. However, it sounds a bit orthogonal to the patch at hand which I do believe we still need for the original reasons mentioned (RPM changing binaries after the JDK build is long done and the windows issue of the JDK build itself placing different *.pdb files into the image than was present at jlink time). So perhaps we should explore this in parallel?

I think upgradable files is something we can deal with. I'm not sure yet on the PDB issue, need to think more about about the scenarios to see what might make sense.

So the suggestion would be that we could mark some files as upgradable which would exclude them from SHA checking? Or, check the sha but accept a diff while printing a warning?

Would it maybe make sense/be possible to offer some re-hash functionality for using in 2nd step builds?

[jerboaa]

Would it maybe make sense/be possible to offer some re-hash functionality for using in 2nd step builds?

What would that be? Right now linking from the run-time image doesn't allow for jdk.jlink to be included, which prevents a 2nd step build.

[RealCLanger]

Would it maybe make sense/be possible to offer some re-hash functionality for using in 2nd step builds?

What would that be? Right now linking from the run-time image doesn't allow for jdk.jlink to be included, which prevents a 2nd step build.

Well, it would have to be something that updates the jdk.jlink in the runtime image. But if that's not feasible, then it's no option.

[jerboaa]

I've created https://bugs.openjdk.org/browse/JDK-8353185 for the upgradeable files issue (tzdata and cacerts).