8359920: Use names for frame types in stackmaps

Reviewed-by: dholmes, jsjolen, matsaave, sspitsyn

Author: coleenp

src/hotspot/share/classfile/stackMapTable.cpp

@@ -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;
@@ -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(
@@ -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(
@@ -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);
@@ -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);

src/hotspot/share/classfile/stackMapTable.hpp

@@ -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
@@ -106,6 +106,7 @@ class StackMapStream : StackObj {
 };
 
 class StackMapReader : StackObj {
+  friend class VM_RedefineClasses;
  private:
   // information about the class and method
   constantPoolHandle  _cp;
@@ -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:

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -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"
@@ -3267,21 +3268,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
     }
 
@@ -3290,7 +3293,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);
@@ -3300,28 +3303,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);
@@ -3336,7 +3341,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;