Skip to content

8354871: Replace stack map frame type magics with constants #27029

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 5 commits into from
Closed

8354871: Replace stack map frame type magics with constants #27029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
02262d0
constants
fandreuz Aug 30, 2025
0324989
Merge branch 'master' into JDK-8354871
fandreuz Sep 1, 2025
0ca1e71
nn
fandreuz Sep 1, 2025
4a9bf43
Apply suggestions from code review
fandreuz Sep 6, 2025
0c6c04b
inclusive
fandreuz Sep 6, 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
19 changes: 10 additions & 9 deletions src/java.base/share/classes/java/lang/classfile/attribute/StackMapFrameInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.List;

import jdk.internal.classfile.impl.StackMapDecoder;
import jdk.internal.classfile.impl.StackMapGenerator;
import jdk.internal.classfile.impl.TemporaryConstantPool;

/**
Expand Down Expand Up @@ -101,31 +102,31 @@ public static StackMapFrameInfo of(Label target,
sealed interface VerificationTypeInfo {

/** The {@link #tag() tag} for verification type info {@link SimpleVerificationTypeInfo#TOP TOP}. */
int ITEM_TOP = 0;
int ITEM_TOP = StackMapGenerator.ITEM_TOP;

/** The {@link #tag() tag} for verification type info {@link SimpleVerificationTypeInfo#INTEGER INTEGER}. */
int ITEM_INTEGER = 1;
int ITEM_INTEGER = StackMapGenerator.ITEM_INTEGER;

/** The {@link #tag() tag} for verification type info {@link SimpleVerificationTypeInfo#FLOAT FLOAT}. */
int ITEM_FLOAT = 2;
int ITEM_FLOAT = StackMapGenerator.ITEM_FLOAT;

/** The {@link #tag() tag} for verification type info {@link SimpleVerificationTypeInfo#DOUBLE DOUBLE}. */
int ITEM_DOUBLE = 3;
int ITEM_DOUBLE = StackMapGenerator.ITEM_DOUBLE;

/** The {@link #tag() tag} for verification type info {@link SimpleVerificationTypeInfo#LONG LONG}. */
int ITEM_LONG = 4;
int ITEM_LONG = StackMapGenerator.ITEM_LONG;

/** The {@link #tag() tag} for verification type info {@link SimpleVerificationTypeInfo#NULL NULL}. */
int ITEM_NULL = 5;
int ITEM_NULL = StackMapGenerator.ITEM_NULL;

/** The {@link #tag() tag} for verification type info {@link SimpleVerificationTypeInfo#UNINITIALIZED_THIS UNINITIALIZED_THIS}. */
int ITEM_UNINITIALIZED_THIS = 6;
int ITEM_UNINITIALIZED_THIS = StackMapGenerator.ITEM_UNINITIALIZED_THIS;

/** The {@link #tag() tag} for verification type info {@link ObjectVerificationTypeInfo OBJECT}. */
int ITEM_OBJECT = 7;
int ITEM_OBJECT = StackMapGenerator.ITEM_OBJECT;

/** The {@link #tag() tag} for verification type info {@link UninitializedVerificationTypeInfo UNINITIALIZED}. */
int ITEM_UNINITIALIZED = 8;
int ITEM_UNINITIALIZED = StackMapGenerator.ITEM_UNINITIALIZED;

/**
* {@return the tag of the type info}
Expand Down
15 changes: 8 additions & 7 deletions src/java.base/share/classes/jdk/internal/classfile/impl/CodeImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.Optional;
import java.util.function.Consumer;

import static jdk.internal.classfile.impl.StackMapGenerator.*;
import static jdk.internal.classfile.impl.RawBytecodeHelper.*;

public final class CodeImpl
Expand Down Expand Up @@ -292,33 +293,33 @@ private void inflateJumpTargets() {
for (int i = 0; i < nEntries; ++i) {
int frameType = classReader.readU1(p);
int offsetDelta;
if (frameType < 64) {
if (frameType <= SAME_FRAME_END) {
offsetDelta = frameType;
++p;
}
else if (frameType < 128) {
else if (frameType <= SAME_LOCALS_1_STACK_ITEM_FRAME_END) {
offsetDelta = frameType & 0x3f;
p = adjustForObjectOrUninitialized(p + 1);
}
else
switch (frameType) {
case 247 -> {
case SAME_LOCALS_1_STACK_ITEM_EXTENDED -> {
offsetDelta = classReader.readU2(p + 1);
p = adjustForObjectOrUninitialized(p + 3);
}
case 248, 249, 250, 251 -> {
case CHOP_FRAME_START, CHOP_FRAME_START + 1, CHOP_FRAME_END, SAME_FRAME_EXTENDED -> {
offsetDelta = classReader.readU2(p + 1);
p += 3;
}
case 252, 253, 254 -> {
case APPEND_FRAME_START, APPEND_FRAME_START + 1, APPEND_FRAME_END -> {
offsetDelta = classReader.readU2(p + 1);
int k = frameType - 251;
int k = frameType - APPEND_FRAME_START + 1;
p += 3;
for (int c = 0; c < k; ++c) {
p = adjustForObjectOrUninitialized(p);
}
}
case 255 -> {
case FULL_FRAME -> {
offsetDelta = classReader.readU2(p + 1);
p += 3;
int k = classReader.readU2(p);
Expand Down
33 changes: 15 additions & 18 deletions src/java.base/share/classes/jdk/internal/classfile/impl/StackMapDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,11 @@
import java.util.Objects;

import static java.lang.classfile.ClassFile.ACC_STATIC;
import static java.lang.classfile.attribute.StackMapFrameInfo.VerificationTypeInfo.*;
import static java.util.Objects.requireNonNull;
import static jdk.internal.classfile.impl.StackMapGenerator.*;

public class StackMapDecoder {

private static final int
SAME_LOCALS_1_STACK_ITEM_EXTENDED = 247,
SAME_EXTENDED = 251;
private static final StackMapFrameInfo[] NO_STACK_FRAME_INFOS = {};

private final ClassReader classReader;
Expand Down Expand Up @@ -136,25 +133,25 @@ private static void writeFrame(BufWriterImpl out, int offsetDelta, List<Verifica
int commonLocalsSize = Math.min(prevLocals.size(), fr.locals().size());
int diffLocalsSize = fr.locals().size() - prevLocals.size();
if (-3 <= diffLocalsSize && diffLocalsSize <= 3 && equals(fr.locals(), prevLocals, commonLocalsSize)) {
if (diffLocalsSize == 0 && offsetDelta < 64) { //same frame
if (diffLocalsSize == 0 && offsetDelta <= SAME_FRAME_END) { //same frame
out.writeU1(offsetDelta);
} else { //chop, same extended or append frame
out.writeU1U2(251 + diffLocalsSize, offsetDelta);
out.writeU1U2(SAME_FRAME_EXTENDED + diffLocalsSize, offsetDelta);
for (int i=commonLocalsSize; i<fr.locals().size(); i++) writeTypeInfo(out, fr.locals().get(i));
}
return;
}
} else if (fr.stack().size() == 1 && fr.locals().equals(prevLocals)) {
if (offsetDelta < 64) { //same locals 1 stack item frame
out.writeU1(64 + offsetDelta);
if (offsetDelta <= SAME_LOCALS_1_STACK_ITEM_FRAME_END - SAME_LOCALS_1_STACK_ITEM_FRAME_START) { //same locals 1 stack item frame
out.writeU1(SAME_LOCALS_1_STACK_ITEM_FRAME_START + offsetDelta);
} else { //same locals 1 stack item extended frame
out.writeU1U2(247, offsetDelta);
out.writeU1U2(SAME_LOCALS_1_STACK_ITEM_EXTENDED, offsetDelta);
}
writeTypeInfo(out, fr.stack().get(0));
return;
}
//full frame
out.writeU1U2U2(255, offsetDelta, fr.locals().size());
out.writeU1U2U2(FULL_FRAME, offsetDelta, fr.locals().size());
for (var l : fr.locals()) writeTypeInfo(out, l);
out.writeU2(fr.stack().size());
for (var s : fr.stack()) writeTypeInfo(out, s);
Expand Down Expand Up @@ -188,26 +185,26 @@ List<StackMapFrameInfo> entries() {
var entries = new StackMapFrameInfo[u2()];
for (int ei = 0; ei < entries.length; ei++) {
int frameType = classReader.readU1(p++);
if (frameType < 64) {
if (frameType <= SAME_FRAME_END) {
bci += frameType + 1;
stack = List.of();
} else if (frameType < 128) {
bci += frameType - 63;
} else if (frameType <= SAME_LOCALS_1_STACK_ITEM_FRAME_END) {
bci += frameType - SAME_LOCALS_1_STACK_ITEM_FRAME_START + 1;
stack = List.of(readVerificationTypeInfo());
} else {
if (frameType < SAME_LOCALS_1_STACK_ITEM_EXTENDED)
throw new IllegalArgumentException("Invalid stackmap frame type: " + frameType);
bci += u2() + 1;
if (frameType == SAME_LOCALS_1_STACK_ITEM_EXTENDED) {
stack = List.of(readVerificationTypeInfo());
} else if (frameType < SAME_EXTENDED) {
locals = locals.subList(0, locals.size() + frameType - SAME_EXTENDED);
} else if (frameType < SAME_FRAME_EXTENDED) {
locals = locals.subList(0, locals.size() + frameType - SAME_FRAME_EXTENDED);
stack = List.of();
} else if (frameType == SAME_EXTENDED) {
} else if (frameType == SAME_FRAME_EXTENDED) {
stack = List.of();
} else if (frameType < SAME_EXTENDED + 4) {
} else if (frameType <= APPEND_FRAME_END) {
int actSize = locals.size();
var newLocals = locals.toArray(new VerificationTypeInfo[actSize + frameType - SAME_EXTENDED]);
var newLocals = locals.toArray(new VerificationTypeInfo[actSize + frameType - SAME_FRAME_EXTENDED]);
for (int i = actSize; i < newLocals.length; i++)
newLocals[i] = readVerificationTypeInfo();
locals = List.of(newLocals);
Expand Down
32 changes: 24 additions & 8 deletions src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ static StackMapGenerator of(DirectCodeBuilder dcb, BufWriterImpl buf) {
private static final int T_BOOLEAN = 4, T_LONG = 11;
private static final Frame[] EMPTY_FRAME_ARRAY = {};

private static final int ITEM_TOP = 0,
public static final int
ITEM_TOP = 0,
ITEM_INTEGER = 1,
ITEM_FLOAT = 2,
ITEM_DOUBLE = 3,
Expand All @@ -180,7 +181,22 @@ static StackMapGenerator of(DirectCodeBuilder dcb, BufWriterImpl buf) {
ITEM_SHORT = 11,
ITEM_CHAR = 12,
ITEM_LONG_2ND = 13,
ITEM_DOUBLE_2ND = 14;
ITEM_DOUBLE_2ND = 14,
ITEM_BOGUS = -1;

// Ranges represented by these constants are inclusive on both ends
public static final int
SAME_FRAME_END = 63,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment on these constants that they are inclusive on both ends? A lot of ranges in Java are open on their higher end, like range notations in Arrays.copyOf or String.substring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure: 0c6c04b

SAME_LOCALS_1_STACK_ITEM_FRAME_START = 64,
SAME_LOCALS_1_STACK_ITEM_FRAME_END = 127,
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;

private static final Type[] ARRAY_FROM_BASIC_TYPE = {null, null, null, null,
Type.BOOLEAN_ARRAY_TYPE, Type.CHAR_ARRAY_TYPE, Type.FLOAT_ARRAY_TYPE, Type.DOUBLE_ARRAY_TYPE,
Expand Down Expand Up @@ -1229,25 +1245,25 @@ void writeTo(BufWriterImpl out, Frame prevFrame, ConstantPoolBuilder cp) {
int commonLocalsSize = localsSize > prevFrame.localsSize ? prevFrame.localsSize : localsSize;
int diffLocalsSize = localsSize - prevFrame.localsSize;
if (-3 <= diffLocalsSize && diffLocalsSize <= 3 && equals(locals, prevFrame.locals, commonLocalsSize)) {
if (diffLocalsSize == 0 && offsetDelta < 64) { //same frame
if (diffLocalsSize == 0 && offsetDelta <= SAME_FRAME_END) { //same frame
out.writeU1(offsetDelta);
} else { //chop, same extended or append frame
out.writeU1U2(251 + diffLocalsSize, offsetDelta);
out.writeU1U2(SAME_FRAME_EXTENDED + diffLocalsSize, offsetDelta);
for (int i=commonLocalsSize; i<localsSize; i++) locals[i].writeTo(out, cp);
}
return;
}
} else if (stackSize == 1 && localsSize == prevFrame.localsSize && equals(locals, prevFrame.locals, localsSize)) {
if (offsetDelta < 64) { //same locals 1 stack item frame
out.writeU1(64 + offsetDelta);
if (offsetDelta <= SAME_LOCALS_1_STACK_ITEM_FRAME_END - SAME_LOCALS_1_STACK_ITEM_FRAME_START) { //same locals 1 stack item frame
out.writeU1(SAME_LOCALS_1_STACK_ITEM_FRAME_START + offsetDelta);
} else { //same locals 1 stack item extended frame
out.writeU1U2(247, offsetDelta);
out.writeU1U2(SAME_LOCALS_1_STACK_ITEM_EXTENDED, offsetDelta);
}
stack[0].writeTo(out, cp);
return;
}
//full frame
out.writeU1U2U2(255, offsetDelta, localsSize);
out.writeU1U2U2(FULL_FRAME, offsetDelta, localsSize);
for (int i=0; i<localsSize; i++) locals[i].writeTo(out, cp);
out.writeU2(stackSize);
for (int i=0; i<stackSize; i++) stack[i].writeTo(out, cp);
Expand Down
18 changes: 10 additions & 8 deletions src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

import static java.lang.constant.ConstantDescs.CLASS_INIT_NAME;
import static java.lang.constant.ConstantDescs.INIT_NAME;
import static jdk.internal.classfile.impl.StackMapGenerator.*;

/// ParserVerifier performs selected checks of the class file format according to
/// {@jvms 4.8 Format Checking}.
Expand Down Expand Up @@ -449,20 +450,21 @@ private static int moduleHashesSize(List<ModuleHashInfo> hashes) {

private int stackMapFrameSize(StackMapFrameInfo frame) {
int ft = frame.frameType();
if (ft < 64) return 1;
if (ft < 128) return 1 + verificationTypeSize(frame.stack().getFirst());
if (ft > 246) {
if (ft == 247) return 3 + verificationTypeSize(frame.stack().getFirst());
if (ft < 252) return 3;
if (ft < 255) {
if (ft <= SAME_FRAME_END) return 1;
if (ft <= SAME_LOCALS_1_STACK_ITEM_FRAME_END) return 1 + verificationTypeSize(frame.stack().getFirst());
if (ft > RESERVED_END) {
if (ft == SAME_LOCALS_1_STACK_ITEM_EXTENDED) return 3 + verificationTypeSize(frame.stack().getFirst());
if (ft <= SAME_FRAME_EXTENDED) return 3;
if (ft <= APPEND_FRAME_END) {
var loc = frame.locals();
int l = 3;
for (int i = loc.size() + 251 - ft; i < loc.size(); i++) {
var k = ft - APPEND_FRAME_START + 1;
for (int i = loc.size() - k; i < loc.size(); i++) {
l += verificationTypeSize(loc.get(i));
}
return l;
}
if (ft == 255) {
if (ft == FULL_FRAME) {
int l = 7;
for (var vt : frame.stack()) {
l += verificationTypeSize(vt);
Expand Down
35 changes: 14 additions & 21 deletions src/java.base/share/classes/jdk/internal/classfile/impl/verifier/VerificationTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
import java.util.ArrayList;
import java.util.List;

import static jdk.internal.classfile.impl.verifier.VerificationType.ITEM_Object;
import static jdk.internal.classfile.impl.verifier.VerificationType.ITEM_Uninitialized;
import static jdk.internal.classfile.impl.verifier.VerificationType.ITEM_UninitializedThis;
import static jdk.internal.classfile.impl.StackMapGenerator.*;

/// From `stackMapTable.cpp`.
class VerificationTable {
Expand Down Expand Up @@ -158,11 +156,6 @@ void check_verification_type_array_size(int size, int max_size) {
}
}

private static final int
SAME_LOCALS_1_STACK_ITEM_EXTENDED = 247,
SAME_EXTENDED = 251,
FULL = 255;

public int get_frame_count() {
return _frame_count;
}
Expand Down Expand Up @@ -252,24 +245,24 @@ int chop(VerificationType[] locals, int length, int chops) {

VerificationType parse_verification_type(int[] flags) {
int tag = _stream.get_u1();
if (tag < ITEM_UninitializedThis) {
if (tag < ITEM_UNINITIALIZED_THIS) {
return VerificationType.from_tag(tag, _verifier);
}
if (tag == ITEM_Object) {
if (tag == ITEM_OBJECT) {
int class_index = _stream.get_u2();
int nconstants = _cp.entryCount();
if (class_index <= 0 || class_index >= nconstants || _cp.tagAt(class_index) != VerifierImpl.JVM_CONSTANT_Class) {
_verifier.classError("bad class index");
}
return VerificationType.reference_type(_cp.classNameAt(class_index));
}
if (tag == ITEM_UninitializedThis) {
if (tag == ITEM_UNINITIALIZED_THIS) {
if (flags != null) {
flags[0] |= VerificationFrame.FLAG_THIS_UNINIT;
}
return VerificationType.uninitialized_this_type;
}
if (tag == ITEM_Uninitialized) {
if (tag == ITEM_UNINITIALIZED) {
int offset = _stream.get_u2();
if (offset >= _code_length || _code_data[offset] != VerifierImpl.NEW_OFFSET) {
_verifier.classError("StackMapTable format error: bad offset for Uninitialized");
Expand All @@ -285,7 +278,7 @@ VerificationFrame next_helper() {
int offset;
VerificationType[] locals = null;
int frame_type = _stream.get_u1();
if (frame_type < 64) {
if (frame_type <= SAME_FRAME_END) {
if (_first) {
offset = frame_type;
if (_prev_frame.locals_size() > 0) {
Expand All @@ -302,14 +295,14 @@ VerificationFrame next_helper() {
_first = false;
return frame;
}
if (frame_type < 128) {
if (frame_type <= SAME_LOCALS_1_STACK_ITEM_FRAME_END) {
if (_first) {
offset = frame_type - 64;
offset = frame_type - SAME_LOCALS_1_STACK_ITEM_FRAME_START;
if (_prev_frame.locals_size() > 0) {
locals = new 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 VerificationType[2];
Expand Down Expand Up @@ -356,10 +349,10 @@ VerificationFrame next_helper() {
_first = false;
return frame;
}
if (frame_type <= SAME_EXTENDED) {
if (frame_type <= 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;
int flags = _prev_frame.flags();
if (chops != 0) {
Expand Down Expand Up @@ -389,8 +382,8 @@ VerificationFrame next_helper() {
}
_first = false;
return frame;
} else if (frame_type < SAME_EXTENDED + 4) {
int appends = frame_type - SAME_EXTENDED;
} else if (frame_type <= APPEND_FRAME_END) {
int appends = frame_type - APPEND_FRAME_START + 1;
int real_length = _prev_frame.locals_size();
int new_length = real_length + appends*2;
locals = new VerificationType[new_length];
Expand Down Expand Up @@ -418,7 +411,7 @@ VerificationFrame next_helper() {
_first = false;
return frame;
}
if (frame_type == FULL) {
if (frame_type == FULL_FRAME) {
int flags[] = new int[]{0};
int locals_size = _stream.get_u2();
int real_locals_size = 0;
Expand Down
Loading