RFR: 8359920: Use names for frame types in stackmaps [v4]

David Holmes dholmes at openjdk.org
Mon Jun 23 06:35:31 UTC 2025


On Fri, 20 Jun 2025 15:05:03 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This uses names for frame types for stackmaps in the verifier and redefinition.
>> Tested with tier1-7.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix tags (running more tests)

Thanks for applying this @coleenp !

I have a couple of suggested changes that to me make it more obvious what case(s) we are dealing with, but up to you whether to apply them or not. The more I look at this code the more it cries out to be restructured, to me.

Thanks

src/hotspot/share/classfile/stackMapTable.cpp line 384:

> 382:     _first = false;
> 383:     return frame;
> 384:   } else if (frame_type < SAME_FRAME_EXTENDED + 4) {

Suggestion:

  } else if (frame_type <= APPEND_FRAME_END) {

src/hotspot/share/classfile/stackMapTable.cpp line 387:

> 385:     // append_frame
> 386:     assert(frame_type >= APPEND_FRAME_START && frame_type <= APPEND_FRAME_END, "should be");
> 387:     int appends = frame_type - SAME_FRAME_EXTENDED;

Suggestion:

    int appends = frame_type - APPEND_FRAME_START + 1;

src/hotspot/share/prims/jvmtiRedefineClasses.cpp line 3328:

> 3326:         "no room for offset_delta");
> 3327:       stackmap_p += 2;
> 3328:       u1 len = frame_type - StackMapReader::SAME_FRAME_EXTENDED;

Suggestion:

      u1 len = frame_type - StackMapReader::APPEND_FRAME_START + 1;

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

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25870#pullrequestreview-2948793293
PR Review Comment: https://git.openjdk.org/jdk/pull/25870#discussion_r2160802174
PR Review Comment: https://git.openjdk.org/jdk/pull/25870#discussion_r2160805107
PR Review Comment: https://git.openjdk.org/jdk/pull/25870#discussion_r2160809684


More information about the hotspot-dev mailing list