Skip to content

8359920: Use names for frame types in stackmaps #25870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

8359920: Use names for frame types in stackmaps #25870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
37b5c7e
8359920: Use names for frame types in stackmaps
coleenp Jun 18, 2025
c3d8d0f
Update src/hotspot/share/prims/jvmtiRedefineClasses.cpp
coleenp Jun 20, 2025
107382e
Apply David's suggested more complete and accurate frame type names. …
coleenp Jun 20, 2025
7d2e60c
Fix tags (running more tests)
coleenp Jun 20, 2025
3e363bc
Update src/hotspot/share/classfile/stackMapTable.cpp
coleenp Jun 23, 2025
c9e0723
Update src/hotspot/share/classfile/stackMapTable.cpp
coleenp Jun 23, 2025
edec7b8
Update src/hotspot/share/prims/jvmtiRedefineClasses.cpp
coleenp Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions src/hotspot/share/classfile/stackMapTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ StackMapFrame* StackMapReader::next_helper(TRAPS) {
int offset;
VerificationType* locals = nullptr;
u1 frame_type = _stream->get_u1(CHECK_NULL);
if (frame_type < 64) {
if (frame_type <= SAME_FRAME_END) {
// same_frame
if (_first) {
offset = frame_type;
Expand All @@ -266,17 +266,17 @@ StackMapFrame* StackMapReader::next_helper(TRAPS) {
_first = false;
return frame;
}
if (frame_type < 128) {
if (frame_type <= SAME_LOCALS_1_STACK_ITEM_FRAME_END) {
// same_locals_1_stack_item_frame
if (_first) {
offset = frame_type - 64;
offset = frame_type - SAME_LOCALS_1_STACK_ITEM_FRAME_START;
// Can't share the locals array since that is updated by the verifier.
if (_prev_frame->locals_size() > 0) {
locals = NEW_RESOURCE_ARRAY_IN_THREAD(
THREAD, VerificationType, _prev_frame->locals_size());
}
} else {
offset = _prev_frame->offset() + frame_type - 63;
offset = _prev_frame->offset() + frame_type - (SAME_LOCALS_1_STACK_ITEM_FRAME_START - 1);
locals = _prev_frame->locals();
}
VerificationType* stack = NEW_RESOURCE_ARRAY_IN_THREAD(
Expand Down Expand Up @@ -340,13 +340,14 @@ StackMapFrame* StackMapReader::next_helper(TRAPS) {
return frame;
}

if (frame_type <= SAME_EXTENDED) {
if (frame_type <= SAME_FRAME_EXTENDED) {
// chop_frame or same_frame_extended
locals = _prev_frame->locals();
int length = _prev_frame->locals_size();
int chops = SAME_EXTENDED - frame_type;
int chops = SAME_FRAME_EXTENDED - frame_type;
int new_length = length;
u1 flags = _prev_frame->flags();
assert(chops == 0 || (frame_type >= CHOP_FRAME_START && frame_type <= CHOP_FRAME_END), "should be");
if (chops != 0) {
new_length = chop(locals, length, chops);
check_verification_type_array_size(
Expand Down Expand Up @@ -380,9 +381,10 @@ StackMapFrame* StackMapReader::next_helper(TRAPS) {
}
_first = false;
return frame;
} else if (frame_type < SAME_EXTENDED + 4) {
} else if (frame_type <= APPEND_FRAME_END) {
// append_frame
int appends = frame_type - SAME_EXTENDED;
assert(frame_type >= APPEND_FRAME_START && frame_type <= APPEND_FRAME_END, "should be");
int appends = frame_type - APPEND_FRAME_START + 1;
int real_length = _prev_frame->locals_size();
int new_length = real_length + appends*2;
locals = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, VerificationType, new_length);
Expand Down Expand Up @@ -412,7 +414,7 @@ StackMapFrame* StackMapReader::next_helper(TRAPS) {
_first = false;
return frame;
}
if (frame_type == FULL) {
if (frame_type == FULL_FRAME) {
// full_frame
u1 flags = 0;
u2 locals_size = _stream->get_u2(CHECK_NULL);
Expand Down
17 changes: 14 additions & 3 deletions src/hotspot/share/classfile/stackMapTable.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -106,6 +106,7 @@ class StackMapStream : StackObj {
};

class StackMapReader : StackObj {
friend class VM_RedefineClasses;
private:
// information about the class and method
constantPoolHandle _cp;
Expand Down Expand Up @@ -148,9 +149,19 @@ class StackMapReader : StackObj {
}

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,
SAME_EXTENDED = 251,
FULL = 255
CHOP_FRAME_START = 248,
CHOP_FRAME_END = 250,
SAME_FRAME_EXTENDED = 251,
APPEND_FRAME_START = 252,
APPEND_FRAME_END = 254,
FULL_FRAME = 255
};

public:
Expand Down
27 changes: 16 additions & 11 deletions src/hotspot/share/prims/jvmtiRedefineClasses.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "classfile/classLoadInfo.hpp"
#include "classfile/javaClasses.inline.hpp"
#include "classfile/metadataOnStackMark.hpp"
#include "classfile/stackMapTable.hpp"
#include "classfile/symbolTable.hpp"
#include "classfile/klassFactory.hpp"
#include "classfile/verifier.hpp"
Expand Down Expand Up @@ -3266,21 +3267,23 @@ void VM_RedefineClasses::rewrite_cp_refs_in_stack_map_table(
// same_frame {
// u1 frame_type = SAME; /* 0-63 */
// }
if (frame_type <= 63) {
if (frame_type <= StackMapReader::SAME_FRAME_END) {
// nothing more to do for same_frame
}

// same_locals_1_stack_item_frame {
// u1 frame_type = SAME_LOCALS_1_STACK_ITEM; /* 64-127 */
// verification_type_info stack[1];
// }
else if (frame_type >= 64 && frame_type <= 127) {
else if (frame_type >= StackMapReader::SAME_LOCALS_1_STACK_ITEM_FRAME_START &&
frame_type <= StackMapReader::SAME_LOCALS_1_STACK_ITEM_FRAME_END) {
rewrite_cp_refs_in_verification_type_info(stackmap_p, stackmap_end,
calc_number_of_entries, frame_type);
}

// reserved for future use
else if (frame_type >= 128 && frame_type <= 246) {
else if (frame_type >= StackMapReader::RESERVED_START &&
frame_type <= StackMapReader::RESERVED_END) {
// nothing more to do for reserved frame_types
}

Expand All @@ -3289,7 +3292,7 @@ void VM_RedefineClasses::rewrite_cp_refs_in_stack_map_table(
// u2 offset_delta;
// verification_type_info stack[1];
// }
else if (frame_type == 247) {
else if (frame_type == StackMapReader::SAME_LOCALS_1_STACK_ITEM_EXTENDED) {
stackmap_p += 2;
rewrite_cp_refs_in_verification_type_info(stackmap_p, stackmap_end,
calc_number_of_entries, frame_type);
Expand All @@ -3299,28 +3302,30 @@ void VM_RedefineClasses::rewrite_cp_refs_in_stack_map_table(
// u1 frame_type = CHOP; /* 248-250 */
// u2 offset_delta;
// }
else if (frame_type >= 248 && frame_type <= 250) {
else if (frame_type >= StackMapReader::CHOP_FRAME_START &&
frame_type <= StackMapReader::CHOP_FRAME_END) {
stackmap_p += 2;
}

// same_frame_extended {
// u1 frame_type = SAME_FRAME_EXTENDED; /* 251*/
// u1 frame_type = SAME_EXTENDED; /* 251 */
// u2 offset_delta;
// }
else if (frame_type == 251) {
else if (frame_type == StackMapReader::SAME_FRAME_EXTENDED) {
stackmap_p += 2;
}

// append_frame {
// u1 frame_type = APPEND; /* 252-254 */
// u2 offset_delta;
// verification_type_info locals[frame_type - 251];
// verification_type_info locals[frame_type - SAME_EXTENDED];
// }
else if (frame_type >= 252 && frame_type <= 254) {
else if (frame_type >= StackMapReader::APPEND_FRAME_START &&
frame_type <= StackMapReader::APPEND_FRAME_END) {
assert(stackmap_p + 2 <= stackmap_end,
"no room for offset_delta");
stackmap_p += 2;
u1 len = frame_type - 251;
u1 len = frame_type - StackMapReader::APPEND_FRAME_START + 1;
for (u1 i = 0; i < len; i++) {
rewrite_cp_refs_in_verification_type_info(stackmap_p, stackmap_end,
calc_number_of_entries, frame_type);
Expand All @@ -3335,7 +3340,7 @@ void VM_RedefineClasses::rewrite_cp_refs_in_stack_map_table(
// u2 number_of_stack_items;
// verification_type_info stack[number_of_stack_items];
// }
else if (frame_type == 255) {
else if (frame_type == StackMapReader::FULL_FRAME) {
assert(stackmap_p + 2 + 2 <= stackmap_end,
"no room for smallest full_frame");
stackmap_p += 2;
Expand Down