8309399: JVMTI spec needs to clarify when OPAQUE_FRAME is thrown for reasons other than a native method

[sspitsyn]

It was decided in a local discussion with Chris and Alan to update the JVMTI spec to make descriptions/clarifications of some JVMTI_ERROR_OPAQUE_FRAME cases more consistent.
This impacts the following JVMTI spec sections:

• PopFrame
• NotifyFramePop
• ForceEarlyReturn<Type>
• GetLocal<Type>
• SetLocal<Type>
• general description of the JVMTI_ERROR_OPAQUE_FRAME error code

A related CSR is going to be filed for this spec update.

Testing:

• it is N/A in general but mach5 tiers 1-3 will be run to be completely safe

---

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
• Change requires CSR request JDK-8361327 to be approved

Issues

• JDK-8309399: JVMTI spec needs to clarify when OPAQUE_FRAME is thrown for reasons other than a native method (Enhancement - P4)
• JDK-8361327: JVMTI spec needs to clarify when OPAQUE_FRAME is thrown for reasons other than a native method (CSR)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26111

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back sspitsyn! 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]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@sspitsyn The following labels will be automatically applied to this pull request:

• hotspot
• serviceability

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

[mlbridge]

Webrevs

• 03: Full - Incremental (da3f20a8)
• 02: Full - Incremental (0c6ef896)
• 01: Full - Incremental (c2cbd0d3)
• 00: Full (1d017ea8)

[plummercj]

I don't think this sounds right. You don't "set" a FramePop event. You tell JVMTI to generate a FramePop event when this frame is popped. Maybe something like "A FramePop event cannot be generated for this frame".

[sspitsyn]

Thank you for the comment.
The NotifyFramePop is similar to SetBreakpoint. I'd even prefer it to be named accordingly: SetFramePop instead of NotifyFramePop. Unfortunately, we can't easily rename API's by compatibility reasons. We enable any kinds of events with SetEventNotoficationMode but these are two cases where we need to be precise in setting the events. The SetBreakpoint is to set a breakpoint notification at a specific method's bytecode. The NotifyFramePop is to set FramePop notification for a specific frame. So, I' prefer to keep the terminology similar for Breakpoints and FramePop events.

[sspitsyn]

Chris, please, let me know if it is okay to keep this at is or you still want an update for the NotifyFramePop.

[plummercj]

I understand what you are saying about the comparison to SetBreakpoint and that SetFramePop would have been a better name, but I don't see what this with the description. I said "set a FramePop event" doesn't sound right. Neither would "set a Breakpoint event". Also, the analogy with breakpoints doesn't really work when you consider that "a breakpoint" is meaningful, but "a FrampPop" does not.

A breakpoint is a location in the code where a breakpoint event will be triggered when executed. That analogy doesn't really work with FramePop. We don't have a word similar to breakpoint that means "a frame marked for generating a FramePop event when popped".

[sspitsyn]

I've decided to also unify the OPAQUE_FRAME clarifications of the GetLocal/SetLocal functions with the other OPAQUE_FRAME cases. It is fixed now.
The CSR has been updated to match the latest changes. It needs a review as well.