Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Mon, 8 Apr 2024 13:20:03 GMT, Thomas Stuefe wrote: > Thank you for answering our questions. No problem, thanks Thomas and Andrew. 8-) - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2043005639
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Thu, 28 Mar 2024 17:15:26 GMT, Kevin Walls wrote: >>> In my opinion, UnlockDiagnosticVMOptions is not a good enough safeguard >>> since it guards a whole swathe of switches that we may instruct the >>> customer to enable. Once enabled, my experience is that >>> UnlockDiagnosticVMOptions often lingers around. It is not unusual for >>> customer scenarios to have set +UnlockDiagnosticVMOptions because of some >>> years ago support cases. >> >> I think we also need to consider the flip side of this argument. Is this >> something that some customers might want to always enable, but don't want to >> always have UnlockDiagnosticVMOptions enabled. A new command line flag would >> be needed in that case. >> >> Also, isn't UnlockDiagnosticVMOptions meant for enabling the use of >> diagnostic command line flags? Do we have examples of it enabling a hotspot >> feature that does not also require setting a diagnostic command line flag? > >> I think we also need to consider the flip side of this argument. Is this >> something that some customers might want to always enable, but don't want to >> always have UnlockDiagnosticVMOptions enabled. A new command line flag would >> be needed in that case. >> >> Also, isn't UnlockDiagnosticVMOptions meant for enabling the use of >> diagnostic command line flags? Do we have examples of it enabling a hotspot >> feature that does not also require setting a diagnostic command line flag? > > Thanks Chris - that is correct now I check the wording, > UnlockDiagnosticVMOptions: "Enable normal processing of flags relating to > field diagnostics" > > Yes it makes flags which are DIAGNOSTIC available at the command-line. We > have UnlockExperimentalVMOptions also. > > My original reason for the guard, is that the risk of fooling the "good oop" > check is that we could possibly crash (so that's answering your other > question). Inspecting an arbitrary pointer that looks enough like a Java > heap object to fool the test, means it might try and resolve bad addresses > for Klass pointer or field data. > > I also have a stress test which examines every address in a Java heap to test > for crashes. That's obviously long-running as it does a jcmd for every > iteration. But I can't currently get a crash, trying G1, ZGC, > ZGCGenerational. > > Having a global guard to prevent usage of VM.inspect may be needed less than > I thought, but will wait on the other thread before saying that is really the > case. We could have a new -XX flag to guard it (whether specific to this > command or a more general UnlockDiagnosticFeatures. I would definitely stick > to this being in all builds, as we frequently "debug" non-debug builds. > > Also, for your comment above about the new version of dbg_is_good_oop: this > was added because although there are not many callers of it, I did not want > to upset them. During my earlier testing the detailed version of this method > did upset something, although I cannot reproduce that today. > > I will rerun some tests on these items... > @kevinjwalls Thanks for clarifying the relevant behaviours and code paths as > well as identifying a few places where documentation might help devs avoid > tripping over any issues. Much appreciated. @kevinjwalls I am fine with this as well. Thank you for answering our questions. - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2042743210
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Wed, 27 Mar 2024 05:26:09 GMT, Chris Plummer wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Undo include
>
> src/hotspot/share/utilities/debug.cpp line 680:
>
>> 678:
>> 679: // Additional "good oop" checks, separate method to not disturb
>> existing asserts.
>> 680: extern "C" bool dbg_is_good_oop_detailed(oopDesc* o) {
>
> What was the motivation for adding this? Is this copied from other HS code?
> How do we know it's doing a good enough job of verification? What happens if
> verification is not thorough enough?
On use of dbg_is_good_oop() and do we need our own dbg_is_good_oop_detailed():
The worst case is a crash if the data looks enough like an oop to pass basic
checks, but if the klass pointer or a field turns out to be a wild pointer. So
I wanted a stricter and more careful "is good oop" check, e.g. to make it
verify the klass pointer is safe, before calling is_klass(). I cannot now see
a crash when taking a valid object and examining thousands of what must be
wrong pointers in memory after it.
I kept these separate, as had seen an assert when making the existing
dbg_is_good_oop more strict. But that was an error, I had the pointer
arithmetic wrong.
This check is called by asserts in two existing places, plus the new
diagnosticCommand in this PR. With this update I'm running all of tier1 OK,
which includes ContFramePopTest.java where I could previously trigger an assert.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1551632113
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Thu, 28 Mar 2024 17:15:26 GMT, Kevin Walls wrote: >>> In my opinion, UnlockDiagnosticVMOptions is not a good enough safeguard >>> since it guards a whole swathe of switches that we may instruct the >>> customer to enable. Once enabled, my experience is that >>> UnlockDiagnosticVMOptions often lingers around. It is not unusual for >>> customer scenarios to have set +UnlockDiagnosticVMOptions because of some >>> years ago support cases. >> >> I think we also need to consider the flip side of this argument. Is this >> something that some customers might want to always enable, but don't want to >> always have UnlockDiagnosticVMOptions enabled. A new command line flag would >> be needed in that case. >> >> Also, isn't UnlockDiagnosticVMOptions meant for enabling the use of >> diagnostic command line flags? Do we have examples of it enabling a hotspot >> feature that does not also require setting a diagnostic command line flag? > >> I think we also need to consider the flip side of this argument. Is this >> something that some customers might want to always enable, but don't want to >> always have UnlockDiagnosticVMOptions enabled. A new command line flag would >> be needed in that case. >> >> Also, isn't UnlockDiagnosticVMOptions meant for enabling the use of >> diagnostic command line flags? Do we have examples of it enabling a hotspot >> feature that does not also require setting a diagnostic command line flag? > > Thanks Chris - that is correct now I check the wording, > UnlockDiagnosticVMOptions: "Enable normal processing of flags relating to > field diagnostics" > > Yes it makes flags which are DIAGNOSTIC available at the command-line. We > have UnlockExperimentalVMOptions also. > > My original reason for the guard, is that the risk of fooling the "good oop" > check is that we could possibly crash (so that's answering your other > question). Inspecting an arbitrary pointer that looks enough like a Java > heap object to fool the test, means it might try and resolve bad addresses > for Klass pointer or field data. > > I also have a stress test which examines every address in a Java heap to test > for crashes. That's obviously long-running as it does a jcmd for every > iteration. But I can't currently get a crash, trying G1, ZGC, > ZGCGenerational. > > Having a global guard to prevent usage of VM.inspect may be needed less than > I thought, but will wait on the other thread before saying that is really the > case. We could have a new -XX flag to guard it (whether specific to this > command or a more general UnlockDiagnosticFeatures. I would definitely stick > to this being in all builds, as we frequently "debug" non-debug builds. > > Also, for your comment above about the new version of dbg_is_good_oop: this > was added because although there are not many callers of it, I did not want > to upset them. During my earlier testing the detailed version of this method > did upset something, although I cannot reproduce that today. > > I will rerun some tests on these items... @kevinjwalls Thanks for clarifying the relevant behaviours and code paths as well as identifying a few places where documentation might help devs avoid tripping over any issues. Much appreciated. - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2031966028
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Wed, 27 Mar 2024 18:39:38 GMT, Chris Plummer wrote: > I think we also need to consider the flip side of this argument. Is this > something that some customers might want to always enable, but don't want to > always have UnlockDiagnosticVMOptions enabled. A new command line flag would > be needed in that case. > > Also, isn't UnlockDiagnosticVMOptions meant for enabling the use of > diagnostic command line flags? Do we have examples of it enabling a hotspot > feature that does not also require setting a diagnostic command line flag? Thanks Chris - that is correct now I check the wording, UnlockDiagnosticVMOptions: "Enable normal processing of flags relating to field diagnostics" Yes it makes flags which are DIAGNOSTIC available at the command-line. We have UnlockExperimentalVMOptions also. My original reason for the guard, is that the risk of fooling the "good oop" check is that we could possibly crash (so that's answering your other question). Inspecting an arbitrary pointer that looks enough like a Java heap object to fool the test, means it might try and resolve bad addresses for Klass pointer or field data. I also have a stress test which examines every address in a Java heap to test for crashes. That's obviously long-running as it does a jcmd for every iteration. But I can't currently get a crash, trying G1, ZGC, ZGCGenerational. Having a global guard to prevent usage of VM.inspect may be needed less than I thought, but will wait on the other thread before saying that is really the case. We could have a new -XX flag to guard it (whether specific to this command or a more general UnlockDiagnosticFeatures. I would definitely stick to this being in all builds, as we frequently "debug" non-debug builds. Also, for your comment above about the new version of dbg_is_good_oop: this was added because although there are not many callers of it, I did not want to upset them. During my earlier testing the detailed version of this method did upset something, although I cannot reproduce that today. I will rerun some tests on these items... - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2025726278
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Thu, 28 Mar 2024 14:12:08 GMT, Andrew Dinn wrote: >>> It looks like the test only inspects a thread and a java object. Perhaps >>> you could add tests for additional VM objects. Maybe grab a frame PC from a >>> thread stack trace. >> >> Yes - added a couple of metadata tests, and a compiled method test, which >> gets an address from Events info. We know that should resolve to a compiled >> method (if we have such an event, so this copes if there are none). >> >> >> Also the VM.info and Thread.print runs with the CommandExecutor are now >> silent. They are long, and max out the test log file and make it truncate. >> That output could mainly be useful if the regexes can't match items, but >> the output format should be reproducible in a new run. Also if we fail to >> find something we need, like a thread id, it will print the full output it >> searched. > > @kevinjwalls Hi Kevin. Sorry to offer a drive-by comment but I have been > watching this thread and I can understand why @tstuefe is expressing his > concern about the potential for security issues when it comes to using jcmd > to expose JVM internals. > > Exposure of details like a thread/stack/metadata value address or a datum > embedded at such a location does look to me like something bad agents might > be able to profit from. The danger is not just being able to retrieve > specific details of the layout or content of JVM structures themselves, but > the opportunity to use that knowledge to upgrade a weak security hole like, > say, a memory exposure, into a stronger targeted attack that knows where or > to what it might want to apply the crowbar. > > So, first off, please understand that I am not suggesting there *is* a > problem here. I am happy to accept your conclusion that your proposed jcmd > changes do not expose new data to users who ought not to be able to view > either via local or via remote accesses. However, not being an expert when it > comes to the jcmd/DCmd implementation, I'm not sure I really understand why > that is so from your current explanation. In particular I don't understand > what you said about visibility of different commands for local and remote > access. > > That confession of my own ignorance is not really of any immediate importance > as I have no intention of trying to review this change. However, I still feel > it might be useful if you could summarize on this JIRA precisely what > security safeguards are currently in place when it comes to running specific > jcmds/DCmds and why that means exposure of JVM internal details via jcmd, > whether locally or remotely via JMX, will only be to a safe audience of > users. That would help any maintainer/developer who refers to this JIRA to > follow how those safeguards apply to the proposed commands (in particular it > might be of great help to those performing and assessing the correctness of > backports). However, it would probably also as a prophylactic to ensure that > any future development work does not inadvertently lead to unexpected > exposure, which I think is the thing Thomas is more worried about. Indeed, as > Thomas suggested, a clear statement of what policies and mechanisms are (or > should be) in plac e to ensure jcmd content is securely viewable might be a good starting point for you and/or Thomas to raise follow-up JIRAs if needed to: > 1. add comments to the code base to ensure devs to not inadvertently add or > modify new DCmds ... Hi @adinn Drive-bys are welcome. 8-) Providing new crowbars to attackers is not something we want to do. jcmd is protected by the attach api and is not open to other users. We are trusting in that. If you can satisfy the attach API connection, you have all the jcmds/DiagnosticCommands available. No limits. Likewise you can use other tools to examine abritrary memory in the target process, take a gcore, kill it, etc... Why will VM.inspect not be available remotely? Because this marks the jcmd as "hidden": src/hotspot/share/services/diagnosticCommand.cpp: DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl(full_export, true, true /* hidden */ )); (The new test for VM.inspect checks it does not appear in the jcmd help output, to ensure it stays hidden.) In JMX, DiagnosticCommandMBean(Impl) does not expose hidden commands: The route for that is DiagnosticCommandImpl.java calling its native getDiagnosticCommands(), that gets into management.cpp's jmm_GetDiagnosticCommands() calling DCmdFactory::DCmd_list(). In diagnosticFramework.cpp, this iterates DCmdFactories, choosing those which are not hidden. Querying over JMX I cannot see or access vmInspect, like I can see vmInfo, gcClassHistogram etc... through the exposed DiagnosticCommandMBean. That "hidden" flag could be more obvious or better documented perhaps. Also I don't think we actually say in the DiagnosticCommandMBean API docs that it provides access to the DiagnosticCommand
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Wed, 27 Mar 2024 18:28:07 GMT, Kevin Walls wrote: >> It looks like the test only inspects a thread and a java object. Perhaps you >> could add tests for additional VM objects. Maybe grab a frame PC from a >> thread stack trace. > >> It looks like the test only inspects a thread and a java object. Perhaps you >> could add tests for additional VM objects. Maybe grab a frame PC from a >> thread stack trace. > > Yes - added a couple of metadata tests, and a compiled method test, which > gets an address from Events info. We know that should resolve to a compiled > method (if we have such an event, so this copes if there are none). > > > Also the VM.info and Thread.print runs with the CommandExecutor are now > silent. They are long, and max out the test log file and make it truncate. > That output could mainly be useful if the regexes can't match items, but the > output format should be reproducible in a new run. Also if we fail to find > something we need, like a thread id, it will print the full output it > searched. @kevinjwalls Hi Kevin. Sorry to offer a drive-by comment but I have been watching this thread and I can understand why @tstuefe is expressing his concern about the potential for security issues when it comes to using jcmd to expose JVM internals. Exposure of details like a thread/stack/metadata value address or a datum embedded at such a location does look to me like something bad agents might be able to profit from. The danger is not just being able to retrieve specific details of the layout or content of JVM structures themselves, but the opportunity to use that knowledge to upgrade a weak security hole like, say, a memory exposure, into a stronger targeted attack that knows where or to what it might want to apply the crowbar. So, first off, please understand that I am not suggesting there *is* a problem here. I am happy to accept your conclusion that your proposed jcmd changes do not expose new data to users who ought not to be able to view either via local or via remote accesses. However, not being an expert when it comes to the jcmd/DCmd implementation, I'm not sure I really understand why that is so from your current explanation. In particular I don't understand what you said about visibility of different commands for local and remote access. That confession of my own ignorance is not really of any immediate importance as I have no intention of trying to review this change. However, I still feel it might be useful if you could summarize on this JIRA precisely what security safeguards are currently in place when it comes to running specific jcmds/DCmds and why that means exposure of JVM internal details via jcmd, whether locally or remotely via JMX, will only be to a safe audience of users. That would help any maintainer/developer who refers to this JIRA to follow how those safeguards apply to the proposed commands (in particular it might be of great help to those performing and assessing the correctness of backports). However, it would probably also as a prophylactic to ensure that any future development work does not inadvertently lead to unexpected exposure, which I think is the thing Thomas is more worried about. Indeed, as Thomas suggested, a clear statement of what policies and mechanisms are (or should be) in place to ensure jcmd content is securely viewable might be a good starting point for you and/or Thomas to raise follow-up JIRAs if needed to: 1. add comments to the code base to ensure devs to not inadvertently add or modify new DCmds in a way that prejudices security 2. enable a review of the existing DCmd organization to see if a better grouping of commands or assignent of access restrictions might be warranted - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2025282602
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Wed, 27 Mar 2024 09:33:43 GMT, Thomas Stuefe wrote: > In my opinion, UnlockDiagnosticVMOptions is not a good enough safeguard since > it guards a whole swathe of switches that we may instruct the customer to > enable. Once enabled, my experience is that UnlockDiagnosticVMOptions often > lingers around. It is not unusual for customer scenarios to have set > +UnlockDiagnosticVMOptions because of some years ago support cases. I think we also need to consider the flip side of this argument. Is this something that some customers might want to always enable, but don't want to always have UnlockDiagnosticVMOptions enabled. A new command line flag would be needed in that case. Also, isn't UnlockDiagnosticVMOptions meant for enabling the use of diagnostic command line flags? Do we have examples of it enabling a hotspot feature that does not also require setting a diagnostic command line flag? - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2023675637
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Wed, 27 Mar 2024 05:44:25 GMT, Chris Plummer wrote: > It looks like the test only inspects a thread and a java object. Perhaps you > could add tests for additional VM objects. Maybe grab a frame PC from a > thread stack trace. Yes - added a couple of metadata tests, and a compiled method test, which gets an address from Events info. We know that should resolve to a compiled method (if we have such an event, so this copes if there are none). Also the VM.info and Thread.print runs with the CommandExecutor are now silent. They are long, and max out the test log file and make it truncate. That output could mainly be useful if the regexes can't match items, but the output format should be reproducible in a new run. Also if we fail to find something we need, like a thread id, it will print the full output it searched. - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2023578758
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Tue, 26 Mar 2024 21:55:38 GMT, Kevin Walls wrote: >> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object >> information. >> >> Not recommended for live production use. Requires UnlockDiagnosticVMOptions >> and not included in jcmd help output, to remind us this is not a >> general-purpose customer-facing tool. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Undo include I am still feeling uneasy about this new command. I can see its potential usefulness, but as stated before would prefer it being limited to debug-only JVMs. - jcmd can be exposed remotely, e.g., via jmx. I am not sure whether other solutions exist, but exposing jcmd beyond the immediate box the JVM runs on is something many folks want, and that is technically easy to do. So, customer-local solutions may exist for that, and the reach of jcmd may be larger than we think. - the underlying functionality for print_location was written with debugging and error analysis in mind. We keep adding functionality there. I don't think developers adding to that function have in mind that this functionality may be exposed, possibly remotely. In my opinion, UnlockDiagnosticVMOptions is not a good enough safeguard since it guards a whole swathe of switches that we may instruct the customer to enable. Once enabled, my experience is that UnlockDiagnosticVMOptions often lingers around. It is not unusual for customer scenarios to have set +UnlockDiagnosticVMOptions because of some years ago support cases. If others feel this command is safe enough, I'll shut up and be quiet, since I cannot think up a concrete attack scenario. - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2022310874
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Tue, 26 Mar 2024 21:55:38 GMT, Kevin Walls wrote:
>> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object
>> information.
>>
>> Not recommended for live production use. Requires UnlockDiagnosticVMOptions
>> and not included in jcmd help output, to remind us this is not a
>> general-purpose customer-facing tool.
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Undo include
It looks like the test only inspects a thread and a java object. Perhaps you
could add tests for additional VM objects. Maybe grab a frame PC from a thread
stack trace.
src/hotspot/share/services/diagnosticCommand.hpp line 590:
> 588: }
> 589: static const char* description() {
> 590: return "Inspect VM object at address.";
Maybe distinguishing between java heap objects and VM objects would be useful
to the reader/user.
src/hotspot/share/utilities/debug.cpp line 679:
> 677: }
> 678:
> 679: // Additional "good oop" checks, separate method to not disturb existing
> asserts.
Suggestion:
// Additional "good oop" checks. Separate method to not disturb existing
asserts.
src/hotspot/share/utilities/debug.cpp line 680:
> 678:
> 679: // Additional "good oop" checks, separate method to not disturb existing
> asserts.
> 680: extern "C" bool dbg_is_good_oop_detailed(oopDesc* o) {
What was the motivation for adding this? Is this copied from other HS code? How
do we know it's doing a good enough job of verification? What happens if
verification is not thorough enough?
test/hotspot/jtreg/serviceability/dcmd/vm/VMInspectTest.java line 83:
> 81: }
> 82:
> 83: // Tests where enabled:
Suggestion:
// Tests run with VM.inspect support enabled:
-
PR Review: https://git.openjdk.org/jdk/pull/17655#pullrequestreview-1962319603
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540483489
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540485389
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540486518
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540492385
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Tue, 26 Mar 2024 21:55:38 GMT, Kevin Walls wrote:
>> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object
>> information.
>>
>> Not recommended for live production use. Requires UnlockDiagnosticVMOptions
>> and not included in jcmd help output, to remind us this is not a
>> general-purpose customer-facing tool.
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Undo include
test/hotspot/jtreg/serviceability/dcmd/vm/VMInspectTest.java line 123:
> 121: ptr = findPointer(threadPrintOutput, waiting_on_mylock, 1);
> 122: output = executor.execute("VM.inspect " + pointerText(ptr));
> 123: System.out.println(output);
Nit: May I ask you to add empty lines after the lines 101 and 123 to make the
code more readable?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540205562
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Tue, 26 Mar 2024 21:55:38 GMT, Kevin Walls wrote: >> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object >> information. >> >> Not recommended for live production use. Requires UnlockDiagnosticVMOptions >> and not included in jcmd help output, to remind us this is not a >> general-purpose customer-facing tool. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Undo include test/hotspot/jtreg/serviceability/dcmd/vm/VMInspectTest.java line 217: > 215: t.start(); > 216: } > 217: } Nit: The indents in the `work()` method has to be 4, not 2. - PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540202705
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Undo include - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/739bcbfa..bf5e4b26 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=08 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=07-08 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
