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

[Serguei Spitsyn]

On Fri, 11 Jul 2025 22:18:09 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>> 
>>  - Merge
>>  - review: minor tweak of previous change
>>  - review: corrected OPAQUE_FRAME clarification for NotifyFramePop function
>>  - review: (1) remove vthread specific clarifications; unify GetLocal* and SetLocal* with other functions
>>  - review: tweak the OPAQUE_FRAME clarifications for ForceEarlyReturn* functions
>>  - review: tweak OPAQUE_FRAME clarification for NotifyFramePop function
>>  - 8309399: JVMTI spec needs to clarify when OPAQUE_FRAME is thrown for reasons other than a native method
>
> src/hotspot/share/prims/jvmti.xml line 5904:
> 
>> 5902:         <error id="JVMTI_ERROR_OPAQUE_FRAME">
>> 5903:           The implementation is unable to get the frame locals
>> 5904:           (e.g. the frame at <code>depth</code> is executing a native method).
> 
> Is this true, especially the native method handling? I'm looking at changes needed on the JDWP and JDI side, and currently they don't even handle OPAQUE_FRAME. I get the feeling native methods produce JVMTI_ERROR_INVALID_SLOT .
> 
> For JDWP and JDI things are similar for GetLocalXXX() with the exception that they expect OPAQUE_FRAME when not dealing with a mounted virtual thread suspended at a breakpoint. So basically that means OPAQUE_FRAME handling did not exist before virtual threads.

Thank you for the concern. But I'm kind of puzzled with your observation.
The JVMTI implementation should return `JVMTI_ERROR_OPAQUE_FRAME` for platform threads.
Though I wonder if it also was the case before virtual thread support was added.

The `VM_BaseGetOrSetLocal::doit()` have the following checks in place:


void VM_BaseGetOrSetLocal::doit() {
  . . .
  frame fr = _jvf->fr();
  if (_set && _depth != 0 && Continuation::is_frame_in_continuation(_jvf->thread(), fr)) {
    _result = JVMTI_ERROR_OPAQUE_FRAME; // deferred locals are not fully supported in continuations
    return;
  }
  Method* method = _jvf->method();
  if (getting_receiver()) {
    if (method->is_static()) {
      _result = JVMTI_ERROR_INVALID_SLOT;   ### This check is for GetLocalInstance
      return;
    }
  } else {
    if (method->is_native()) {
      _result = JVMTI_ERROR_OPAQUE_FRAME;   ### This check is before calls to
                               ### check_slot_type_no_lvt() and check_slot_type_lvt()
      return;
    }
    if (!check_slot_type_no_lvt(_jvf)) {    ### This has checks for JVMTI_ERROR_INVALID_SLOT
      return;
    }
    if (method->has_localvariable_table() &&
        !check_slot_type_lvt(_jvf)) {    ### This has checks for JVMTI_ERROR_INVALID_SLOT
      return;
    }
  }
  . . .

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26111#discussion_r2205878604