8280963: Incorrect PrintFlags formatting on Windows

[akashche]

A fix to 8u-only Windows-specific problem with -XX:+PrintFlags* output.

Problem description on Stack Overflow by Andrei Pangin: https://stackoverflow.com/a/63309395

The problem was fixed in jdk9 as part of JDK-8042893, but this jdk9 change doesn't look like a good candidate for the 8u backport according to 8u Best Practices. Still the problem manifests itself as a customer-visible bug on Windows because people do use -XX:+PrintFlagsFinal to find out the actual flags picked up by JVM and can be confused by unexpected zeros in the output. Proposed patch cherry-picks the minimal change from JDK-8042893 commit.

Mailing list review: https://mail.openjdk.java.net/pipermail/jdk8u-dev/2022-February/014532.html

Testing:

• regression test is included with the proposed patch
• checked that it builds on Windows 64-bit and 32-bit on VS2017 and VS2010
• ran jtreg:hotspot/test/runtime on Windows and Linux

---

Progress

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

Issue

• JDK-8280963: Incorrect PrintFlags formatting on Windows

Reviewers

• Andrew John Hughes (@gnu-andrew - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev pull/45/head:pull/45
$ git checkout pull/45

Update a local copy of the PR:
$ git checkout pull/45
$ git pull https://git.openjdk.org/jdk8u-dev pull/45/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 45

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/45.diff

[bridgekeeper]

👋 Welcome back akasko! 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.

[mlbridge]

Webrevs

• 01: Full - Incremental (a312b2ce)
• 00: Full (5ef280c4)

[gnu-andrew]

This mostly matches the changes to this file in JDK-8042893, but it doesn't remove the pragma

Can we include that change as well to make sure the code is now compiling without warnings?

Also, the PR needs to be rebased anyway to fix the GHA run on Linux (bad dependency issue now fixed by JDK-8284772). The Mac failure is more complicated (see #26) but is at least compiling the code before failing. Despite the title, this changes code shared by all platforms.

[akashche]

Would it be better if I add a PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC change as a separate backport?

I understand that it is a part of the same original change, just the motivation behind the change is completely different from the format strings change that are included now. I am fine with doing it either way, just wanted to point out that the inclusion of pragma here may be confusing.

[gnu-andrew]

Would it be better if I add a PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC change as a separate backport?

I understand that it is a part of the same original change, just the motivation behind the change is completely different from the format strings change that are included now. I am fine with doing it either way, just wanted to point out that the inclusion of pragma here may be confusing.

Yes, that makes sense. We're already deviating from the original enhancement issue with this subset to fix a bug. Please file a bug and I'll approve this once the repository has transitioned to 8u352.

[gnu-andrew]

I can approve this now once jdk8u-fix-request is added to the bug.

To clarify; I just meant the one PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC removal in the file being altered, not the rest of that huge patch. In theory, the changes mean that file should now compile without warnings and so the pragma is redundant. But this is something was can do under a separate bug.

[akashche]

Thanks for the review! I've added fix request label to the bug.

[gnu-andrew]

Sorry for the delay, approved.

[akashche]

Thanks for the approval! It is not clear whether macOS pre-submit check must pass for integrate command to work, will try it now.

[akashche]

/integrate

[openjdk]

@akashche This pull request has not yet been marked as ready for integration.

[akashche]

Apparently PR needs to be "ready" for integrate to work, any hints what can I do for it to have a "ready" mark?

[jerboaa]

Apparently PR needs to be "ready" for integrate to work, any hints what can I do for it to have a "ready" mark?

It needs an actual "Approve" via the Review changes button. I don't see it here.

[gnu-andrew]

Done. This would be simpler if the approval process was integrated with SKARA.

[openjdk]

@akashche This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8280963: Incorrect PrintFlags formatting on Windows

Reviewed-by: andrew

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 13 new commits pushed to the master branch:

• 820ab13: 8235385: Crash on aarch64 JDK due to long offset
• f3c87ce: 8287521: Bump update version of OpenJDK: 8u352
• e3b9a06: 8031567: Better model for storing source revision information
• 2194c53: 8202142: jfr/event/io/TestInstrumentation is unstable
• d35dea7: 8266187: Memory leak in appendBootClassPath()
• 83fbc96: 8283350: (tz) Update Timezone Data to 2022a
• c5f96be: 8286989: Build failure on macOS after 8281814
• 7961755: 8285727: [11u, 17u] Unify fix for JDK-8284920 with version from head
• eb5711b: 8284620: CodeBuffer may leak _overflow_arena
• db52470: 8285523: Improve test java/io/FileOutputStream/OpenNUL.java
• ... and 3 more: https://git.openjdk.org/jdk8u-dev/compare/51f69d9125e72adaf05fbd04a5ac17a9d2f6c6a0...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[gnu-andrew]

Thanks for the approval! It is not clear whether macOS pre-submit check must pass for integrate command to work, will try it now.

The MacOS failure is unrelated and is already fixed in jdk8u-dev. You can confirm this by merging with 8u-dev yourself, but it's not really necessary.

[gnu-andrew]

Specifically, "c5f96be: 8286989: Build failure on macOS after 8281814"

[akashche]

Thanks for the clarification!

[akashche]

/integrate

[openjdk]

Going to push as commit 83e9095.
Since your change was applied there have been 13 commits pushed to the master branch:

• 820ab13: 8235385: Crash on aarch64 JDK due to long offset
• f3c87ce: 8287521: Bump update version of OpenJDK: 8u352
• e3b9a06: 8031567: Better model for storing source revision information
• 2194c53: 8202142: jfr/event/io/TestInstrumentation is unstable
• d35dea7: 8266187: Memory leak in appendBootClassPath()
• 83fbc96: 8283350: (tz) Update Timezone Data to 2022a
• c5f96be: 8286989: Build failure on macOS after 8281814
• 7961755: 8285727: [11u, 17u] Unify fix for JDK-8284920 with version from head
• eb5711b: 8284620: CodeBuffer may leak _overflow_arena
• db52470: 8285523: Improve test java/io/FileOutputStream/OpenNUL.java
• ... and 3 more: https://git.openjdk.org/jdk8u-dev/compare/51f69d9125e72adaf05fbd04a5ac17a9d2f6c6a0...master

Your commit was automatically rebased without conflicts.

[openjdk]

@akashche Pushed as commit 83e9095.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.