On Fri, 20 Jun 2025 15:05:03 GMT, Coleen Phillimore <cole...@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
