On Wed, 18 Jun 2025 12:13:38 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
> This uses names for frame types for stackmaps in the verifier and
> redefinition.
> Tested with tier1-7.
src/hotspot/share/classfile/stackMapTable.hpp line 154:
> 152: SAME_FRAME = 64,
> 153: SAME_LOCALS_1_STACK_ITEM_FRAME = 128,
> 154: SAME_LOCALS_1_STACK_ITEM_EXTENDED = 247,
I find these definitions a little confusing. SAME_FRAME is actually 0-63, with
SAME_LOCALS_1_STACK_ITEM_FRAME being 64-127. Given many of these frame types
imply tag ranges it may be clearer to define enum's for the start and end of
ranges as applicable eg.
enum {
SAME_FRAME_START = 0,
SAME_FRAME_END = 63,
SAME_LOCALS_1_STACK_ITEM_FRAME_START = 64,
SAME_LOCALS_1_STACK_ITEM_FRAME_END = 127,
RESERVED_START = 128,
RESERVED_END = 246,
SAME_LOCALS_1_STACK_ITEM_EXTENDED = 247,
CHOP_FRAME_START = 248,
CHOP_FRAME_END = 250,
SAME_FRAME_EXTENDED = 251,
APPEND_FRAME_START = 252,
APPEND_FRAME_END = 254,
FULL_FRAME = 255
}
and then adjust the code usage as appropriate e.g.
if (frame_type <= SAME_FRAME_END) {
...
if (frame_type <= SAME_LOCALS_1_STACK_ITEM_FRAME_END) {
if (_first) {
offset = frame_type - SAME_LOCALS_1_STACK_ITEM_FRAME_START;
...
What do you think?
src/hotspot/share/prims/jvmtiRedefineClasses.cpp line 3321:
> 3319: // u1 frame_type = APPEND; /* 252-254 */
> 3320: // u2 offset_delta;
> 3321: // verification_type_info locals[frame_type - 251];
Suggestion:
// verification_type_info locals[frame_type - SAME_EXTENDED];
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25870#discussion_r2156218466
PR Review Comment: https://git.openjdk.org/jdk/pull/25870#discussion_r2156184106
