Simplify state transition logic, fix dump logical error and comment e…

…rror

src/java.base/share/classes/jdk/internal/classfile/impl/RawBytecodeHelper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, 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
@@ -53,23 +53,20 @@ public RawBytecodeHelper start() {
     public static final int ILLEGAL = -1;
 
     /**
-     * The length of opcodes, 0 for
+     * The length of opcodes, or -1 for no fixed length.
      * This is generated as if:
      * {@snippet lang=java :
      * var lengths = new byte[0x100];
      * Arrays.fill(lengths, (byte) -1);
      * for (var op : Opcode.values()) {
      *     if (!op.isWide()) {
      *         lengths[op.bytecode()] = (byte) op.sizeIfFixed();
-     *     } else {
-     *         // Wide pseudo-opcodes have double the length as normal variants
-     *         // Must match logic in checkSpecialInstruction()
-     *         assert lengths[op.bytecode() & 0xFF] * 2 == op.sizeIfFixed();
      *     }
      * }
      * }
-     * Tested in UtilTest.
+     * Tested in UtilTest::testOpcodeLengthTable.
      */
+    // Note: Consider distinguishing non-opcode and non-fixed-length opcode
     public static final @Stable byte[] LENGTHS = new byte[] {
             1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
             2, 3, 2, 3, 3, 2, 2, 2, 2, 2, 1, 1, 1, 1, 1, 1,
@@ -118,16 +115,18 @@ private RawBytecodeHelper(CodeRange range) {
 
     // immutable states
 
-    public byte[] array() {
-        return code.array;
-    }
-
+    /** {@return the end of the code array} */
     public int endBci() {
         return code.length;
     }
 
     // setup
 
+    /**
+     * Sets the starting bci for bytecode reading. Can be set to
+     * {@link #endBci} to end scanning. Must be followed by a
+     * {@link #next} before getter access.
+     */
     public void reset(int nextBci) {
         Preconditions.checkIndex(nextBci, endBci() + 1, IAE_FORMATTER);
         this.nextBci = nextBci;
@@ -136,17 +135,17 @@ public void reset(int nextBci) {
     // getters after transition
 
     /**
-     * Returns the current functional opcode, or {@link #ILLEGAL} if the next instruction is invalid in format.
-     * If this returns a valid opcode, that instruction's format must be valid and can be accessed unchecked.
-     * Unspecified if called without a {@link #next} transition after object initialization or {@link #reset}.
+     * Returns the current functional opcode, or {@link #ILLEGAL} if
+     * the next instruction is invalid in format.
+     * If this returns a valid opcode, that instruction's format must
+     * be valid and can be accessed unchecked.
      */
     public int opcode() {
         return opcode;
     }
 
     /**
      * Returns whether the current functional opcode is in wide.
-     * Unspecified if called without a {@link #next} transition after object initialization or {@link #reset}.
      */
     public boolean isWide() {
         return isWide;
@@ -159,13 +158,6 @@ public int bci() {
         return bci;
     }
 
-    /**
-     * Returns whether the end of code array is reached.
-     */
-    public boolean isLastInstruction() {
-        return nextBci >= code.length;
-    }
-
     // general utilities
 
     public int getU1(int bci) {
@@ -178,7 +170,7 @@ public int getU2(int bci) {
         return getU2Unchecked(bci);
     }
 
-    public short getShort(int bci) {
+    public int getShort(int bci) {
         Preconditions.checkFromIndexSize(bci, 2, endBci(), IAE_FORMATTER);
         return getShortUnchecked(bci);
     }
@@ -191,20 +183,20 @@ public int getInt(int bci) {
     // Unchecked accessors: only if opcode() is validated
 
     public int getU1Unchecked(int bci) {
-        return Byte.toUnsignedInt(array()[bci]);
+        return Byte.toUnsignedInt(code.array[bci]);
     }
 
     public int getU2Unchecked(int bci) {
-        return UNSAFE.getCharUnaligned(array(), (long) Unsafe.ARRAY_BYTE_BASE_OFFSET + bci, true);
+        return UNSAFE.getCharUnaligned(code.array, (long) Unsafe.ARRAY_BYTE_BASE_OFFSET + bci, true);
     }
 
-    public short getShortUnchecked(int bci) {
-        return UNSAFE.getShortUnaligned(array(), (long) Unsafe.ARRAY_BYTE_BASE_OFFSET + bci, true);
+    public int getShortUnchecked(int bci) {
+        return UNSAFE.getShortUnaligned(code.array, (long) Unsafe.ARRAY_BYTE_BASE_OFFSET + bci, true);
     }
 
     // used after switch validation
     public int getIntUnchecked(int bci) {
-        return UNSAFE.getIntUnaligned(array(), (long) Unsafe.ARRAY_BYTE_BASE_OFFSET + bci, true);
+        return UNSAFE.getIntUnaligned(code.array, (long) Unsafe.ARRAY_BYTE_BASE_OFFSET + bci, true);
     }
 
     // non-wide branches
@@ -235,36 +227,42 @@ public int getIndexU2() {
     // Transition methods
 
     /**
-     * Transition to the next instruction. If the next instruction is
-     * malformed, returns {@link #ILLEGAL}, so we can perform value access
-     * without bound checks if we have a valid opcode. Must be guarded by
-     * {@link #isLastInstruction()} checks.
+     * Transitions to the next instruction and returns whether scanning should
+     * continue. If the next instruction is malformed, {@link #opcode()} returns
+     * {@link #ILLEGAL}, so we can perform value access without bound checks if
+     * we have a valid opcode.
      */
-    public void next() {
+    public boolean next() {
+        if (nextBci >= endBci()) {
+            return false;
+        }
+
         bci = nextBci;
         int code = getU1Unchecked(bci);
         int len = LENGTHS[code];
         opcode = code;
         isWide = false;
-        // Consider using 0 vs -1 to represent invalid vs special
         if (len <= 0) {
             len = checkSpecialInstruction(code);
         }
 
         if (len <= 0 || (nextBci += len) > endBci()) {
             opcode = ILLEGAL;
         }
+
+        return true;
     }
 
-    // pulls out rarely used code blocks to reduce code size
+    // Put rarely used code in another method to reduce code size
     private int checkSpecialInstruction(int code) {
         if (code == WIDE) {
             if (bci + 1 >= endBci()) {
                 return -1;
             }
             opcode = code = getIndexU1();
             isWide = true;
-            return LENGTHS[code] * 2; // must match static block assertion
+            // Validated in UtilTest.testOpcodeLengthTable
+            return LENGTHS[code] * 2;
         }
         if (code == TABLESWITCH) {
             int alignedBci = align(bci + 1);

src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java

@@ -133,8 +133,7 @@ public StackCounter(LabelContext labelContext,
         visited = new BitSet(bcs.endBci());
         targets.add(new Target(0, 0));
         while (next()) {
-            while (!bcs.isLastInstruction()) {
-                bcs.next();
+            while (bcs.next()) {
                 int opcode = bcs.opcode();
                 int bci = bcs.bci();
                 visited.set(bci);

src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java

@@ -407,8 +407,7 @@ private void processMethod() {
         int stackmapIndex = 0;
         var bcs = bytecode.start();
         boolean ncf = false;
-        while (!bcs.isLastInstruction()) {
-            bcs.next();
+        while (bcs.next()) {
             currentFrame.offset = bcs.bci();
             if (stackmapIndex < frames.size()) {
                 int thisOffset = frames.get(stackmapIndex).offset;
@@ -854,8 +853,7 @@ public void set(int i) {
         var bcs = bytecode.start();
         boolean no_control_flow = false;
         int opcode, bci = 0;
-        while (!bcs.isLastInstruction()) try {
-            bcs.next();
+        while (bcs.next()) try {
             opcode = bcs.opcode();
             bci = bcs.bci();
             if (no_control_flow) {

src/java.base/share/classes/jdk/internal/classfile/impl/Util.java

@@ -291,12 +291,12 @@ public void writeBody(BufWriterImpl b) {
             ClassPrinter.toYaml(clm.methods().get(0).code().get(), ClassPrinter.Verbosity.TRACE_ALL, dump);
         } catch (Error | Exception _) {
             // fallback to bytecode hex dump
-            dumpBytesHex(dump, bytecode.array());
+            dumpBytesHex(dump, bytecode.array(), bytecode.length());
         }
     }
 
-    public static void dumpBytesHex(Consumer<String> dump, byte[] bytes) {
-        for (int i = 0; i < bytes.length; i++) {
+    public static void dumpBytesHex(Consumer<String> dump, byte[] bytes, int length) {
+        for (int i = 0; i < length; i++) {
             if (i % 16 == 0) {
                 dump.accept("%n%04x:".formatted(i));
             }

src/java.base/share/classes/jdk/internal/classfile/impl/verifier/VerifierImpl.java

@@ -326,8 +326,7 @@ void verify_method(VerificationWrapper.MethodWrapper m) {
         var bcs = code.start();
         boolean no_control_flow = false;
         int opcode;
-        while (!bcs.isLastInstruction()) {
-            bcs.next();
+        while (bcs.next()) {
             opcode = bcs.opcode();
             bci = bcs.bci();
             current_frame.set_offset(bci);
@@ -1233,8 +1232,7 @@ void verify_method(VerificationWrapper.MethodWrapper m) {
     private byte[] generate_code_data(RawBytecodeHelper.CodeRange code) {
         byte[] code_data = new byte[code.length()];
         var bcs = code.start();
-        while (!bcs.isLastInstruction()) {
-            bcs.next();
+        while (bcs.next()) {
             if (bcs.opcode() != ILLEGAL) {
                 int bci = bcs.bci();
                 if (bcs.opcode() == ClassFile.NEW) {