Skip to content
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.

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

8343846: [lworld] implement spec changes to stack map tables #1333

Draft
wants to merge 34 commits into
base: lworld
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0cdbcca
8343846: [lworld] implement spec changes to stack map tables
vicente-romero-oracle Nov 8, 2024
a47fbb2
Merge branch 'lworld' into JDK-8343846
vicente-romero-oracle Nov 14, 2024
00a22e3
additional changes
vicente-romero-oracle Nov 14, 2024
9f45b2b
Merge branch 'lworld' into JDK-8343846
vicente-romero-oracle Jan 21, 2025
87fd327
removing duplicate method after merge
vicente-romero-oracle Jan 21, 2025
3b4175f
experiments on classfile API side
vicente-romero-oracle Jan 27, 2025
dbcca52
fixing bugs
vicente-romero-oracle Jan 27, 2025
25a5838
additional cleanup
vicente-romero-oracle Jan 29, 2025
07ddeab
additional cleanup
vicente-romero-oracle Jan 29, 2025
943d644
Merge branch 'JDK-8343846' of https://github.com/vicente-romero-oracl…
vicente-romero-oracle Jan 29, 2025
fb7775c
support for loops, try, switch
vicente-romero-oracle Jan 29, 2025
739b111
fixing bug, array out of bounds
vicente-romero-oracle Jan 30, 2025
9a30940
small verification changes
vicente-romero-oracle Jan 30, 2025
cf906da
add option to generate new stackmap table frame
vicente-romero-oracle Jan 30, 2025
17cb485
Merge branch 'lworld' into JDK-8343846
vicente-romero-oracle Jan 30, 2025
64876e2
removing debug code
vicente-romero-oracle Jan 30, 2025
0d2b44a
minor changes to classfile API
vicente-romero-oracle Jan 31, 2025
dec0a31
fixing minor bug in Gen::visitAssign
vicente-romero-oracle Feb 3, 2025
437610a
clean-ups and refactorings
vicente-romero-oracle Feb 3, 2025
b8ba1dc
debug help
vicente-romero-oracle Feb 4, 2025
4994811
minor refactoring
vicente-romero-oracle Feb 4, 2025
30f9df3
improve javap support
vicente-romero-oracle Feb 4, 2025
26b1130
Some stack maps classfile work, need testing
liach Feb 5, 2025
546de3b
Restore stack map entries for asserts
liach Feb 5, 2025
6b54376
Missing clear
liach Feb 5, 2025
0055365
Merge pull request #1 from liach/fix/cf-unset-fields
vicente-romero-oracle Feb 5, 2025
630e241
adding some test cases
vicente-romero-oracle Feb 5, 2025
9d8a61f
merge with lworld
vicente-romero-oracle Feb 5, 2025
ada284b
refactorings
vicente-romero-oracle Feb 6, 2025
352b1de
additional refactoring
vicente-romero-oracle Feb 6, 2025
bf82dfd
another refactoring
vicente-romero-oracle Feb 7, 2025
55d4ec9
another simplification
vicente-romero-oracle Feb 7, 2025
b258586
refactoring n
vicente-romero-oracle Feb 7, 2025
f98e216
another refactoring
vicente-romero-oracle Feb 7, 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
2 changes: 1 addition & 1 deletion src/hotspot/share/classfile/stackMapTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ StackMapFrame* StackMapReader::next(

u2 offset_delta = _stream->get_u2(CHECK_NULL);

if (frame_type < SAME_LOCALS_1_STACK_ITEM_EXTENDED) {
if (frame_type < ASSERT_UNSET_FIELDS) {
// reserved frame types
_stream->stackmap_format_error(
"reserved frame type", CHECK_VERIFY_(_verifier, nullptr));
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/classfile/stackMapTable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class StackMapReader : StackObj {
}

enum {
ASSERT_UNSET_FIELDS = 246,
SAME_LOCALS_1_STACK_ITEM_EXTENDED = 247,
SAME_EXTENDED = 251,
FULL = 255
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import java.lang.classfile.Label;
import java.lang.classfile.constantpool.ClassEntry;
import java.lang.classfile.constantpool.NameAndTypeEntry;
import java.lang.constant.ClassDesc;
import java.util.List;

Expand Down Expand Up @@ -61,6 +62,13 @@ public sealed interface StackMapFrameInfo
*/
List<VerificationTypeInfo> stack();

/**
* {@return the expanded unset fields}
*
* @see <a href="https://cr.openjdk.org/~dlsmith/jep401/jep401-20241108/specs/value-objects-jvms.html">Specs</a>
*/
List<NameAndTypeEntry> unsetFields();

/**
* {@return a new stack map frame}
* @param target the location of the frame
Expand All @@ -71,7 +79,22 @@ public static StackMapFrameInfo of(Label target,
List<VerificationTypeInfo> locals,
List<VerificationTypeInfo> stack) {

return new StackMapDecoder.StackMapFrameImpl(255, target, locals, stack);
return of(target, locals, stack, List.of());
}

/**
* {@return a new stack map frame}
* @param target the location of the frame
* @param locals the complete list of frame locals
* @param stack the complete frame stack
* @param unsetFields the complete list of unset fields
*/
public static StackMapFrameInfo of(Label target,
List<VerificationTypeInfo> locals,
List<VerificationTypeInfo> stack,
List<NameAndTypeEntry> unsetFields) {

return new StackMapDecoder.StackMapFrameImpl(255, target, locals, stack, unsetFields);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ private void inflateJumpTargets() {
int p = stackMapPos + 2;
for (int i = 0; i < nEntries; ++i) {
int frameType = classReader.readU1(p);
int offsetDelta;
int offsetDelta = -1;
if (frameType < 64) {
offsetDelta = frameType;
++p;
Expand All @@ -289,8 +289,14 @@ else if (frameType < 128) {
offsetDelta = frameType & 0x3f;
p = adjustForObjectOrUninitialized(p + 1);
}
else
else {
switch (frameType) {
case 246 -> {
int numberOfUnsetFields = classReader.readU2(p + 1);
p += 3;
p += 2 * numberOfUnsetFields;
continue; // do not move bci/create label
}
case 247 -> {
offsetDelta = classReader.readU2(p + 1);
p = adjustForObjectOrUninitialized(p + 3);
Expand Down Expand Up @@ -323,6 +329,7 @@ else if (frameType < 128) {
}
default -> throw new IllegalArgumentException("Bad frame type: " + frameType);
}
}
bci += offsetDelta + 1;
inflateLabel(bci);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,30 @@
import java.lang.classfile.attribute.StackMapFrameInfo.UninitializedVerificationTypeInfo;
import java.lang.classfile.attribute.StackMapFrameInfo.VerificationTypeInfo;
import java.lang.classfile.constantpool.ClassEntry;
import java.lang.classfile.constantpool.NameAndTypeEntry;
import java.lang.classfile.constantpool.PoolEntry;
import java.lang.constant.ConstantDescs;
import java.lang.constant.MethodTypeDesc;
import java.lang.reflect.AccessFlag;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;

import jdk.internal.access.SharedSecrets;

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

public class StackMapDecoder {

private static final int
ASSERT_UNSET_FIELDS = 246,
SAME_LOCALS_1_STACK_ITEM_EXTENDED = 247,
SAME_EXTENDED = 251;
private static final int RESERVED_TAGS_UPPER_LIMIT = ASSERT_UNSET_FIELDS; // not inclusive
private static final StackMapFrameInfo[] NO_STACK_FRAME_INFOS = {};

private final ClassReader classReader;
Expand Down Expand Up @@ -181,12 +188,29 @@ private static void writeTypeInfo(BufWriterImpl bw, VerificationTypeInfo vti) {
}
}

// Copied from BoundAttribute
<E extends PoolEntry> List<E> readEntryList(int p, Class<E> type) {
int cnt = classReader.readU2(p);
p += 2;
var entries = new Object[cnt];
int end = p + (cnt * 2);
for (int i = 0; p < end; i++, p += 2) {
entries[i] = classReader.readEntry(p, type);
}
return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(entries);
}

List<StackMapFrameInfo> entries() {
p = pos;
List<VerificationTypeInfo> locals = initFrameLocals, stack = List.of();
List<NameAndTypeEntry> unsetFields = List.of();
int bci = -1;
var entries = new StackMapFrameInfo[u2()];
for (int ei = 0; ei < entries.length; ei++) {
int len = u2();
var entries = new ArrayList<StackMapFrameInfo>(len);
List<List<NameAndTypeEntry>> deferredUnsetFields = new ArrayList<>();
for (int ei = 0; ei < len; ei++) {
var oldLocals = locals;
var oldStack = stack;
int frameType = classReader.readU1(p++);
if (frameType < 64) {
bci += frameType + 1;
Expand All @@ -195,8 +219,14 @@ List<StackMapFrameInfo> entries() {
bci += frameType - 63;
stack = List.of(readVerificationTypeInfo());
} else {
if (frameType < SAME_LOCALS_1_STACK_ITEM_EXTENDED)
if (frameType < RESERVED_TAGS_UPPER_LIMIT)
throw new IllegalArgumentException("Invalid stackmap frame type: " + frameType);
if (frameType == ASSERT_UNSET_FIELDS) {
unsetFields = readEntryList(p, NameAndTypeEntry.class);
p += 2 + unsetFields.size() * 2;
deferredUnsetFields.add(unsetFields);
continue; // defer entry until we can get the bci
}
bci += u2() + 1;
if (frameType == SAME_LOCALS_1_STACK_ITEM_EXTENDED) {
stack = List.of(readVerificationTypeInfo());
Expand All @@ -223,12 +253,22 @@ List<StackMapFrameInfo> entries() {
stack = List.of(newStack);
}
}
entries[ei] = new StackMapFrameImpl(frameType,
ctx.getLabel(bci),
Label label = ctx.getLabel(bci);
if (!deferredUnsetFields.isEmpty()) {
// technically we only have one assert at once, just in case
// of duplicate asserts...
for (var deferredList : deferredUnsetFields) {
entries.add(new StackMapFrameImpl(ASSERT_UNSET_FIELDS,
label, oldLocals, oldStack, deferredList));
}
deferredUnsetFields.clear();
}
entries.add(new StackMapFrameImpl(frameType,
label,
locals,
stack);
stack));
}
return List.of(entries);
return List.copyOf(entries);
}

private VerificationTypeInfo readVerificationTypeInfo() {
Expand Down Expand Up @@ -299,12 +339,21 @@ private int u2() {
public static record StackMapFrameImpl(int frameType,
Label target,
List<VerificationTypeInfo> locals,
List<VerificationTypeInfo> stack)
List<VerificationTypeInfo> stack,
List<NameAndTypeEntry> unsetFields)
implements StackMapFrameInfo {
public StackMapFrameImpl {
requireNonNull(target);
locals = List.copyOf(locals);
stack = List.copyOf(stack);
unsetFields = List.copyOf(unsetFields);
}

public StackMapFrameImpl(int frameType,
Label target,
List<VerificationTypeInfo> locals,
List<VerificationTypeInfo> stack) {
this(frameType, target, locals, stack, List.of());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,11 @@ void check_verification_type_array_size(int size, int max_size) {
}

private static final int
ASSERT_UNSET_FIELDS = 246,
SAME_LOCALS_1_STACK_ITEM_EXTENDED = 247,
SAME_EXTENDED = 251,
FULL = 255;
private static final int RESERVED_TAGS_UPPER_LIMIT = ASSERT_UNSET_FIELDS; // not inclusive

public int get_frame_count() {
return _frame_count;
Expand Down Expand Up @@ -278,7 +280,7 @@ public VerificationFrame next(VerificationFrame pre_frame, boolean first, int ma
return frame;
}
int offset_delta = _stream.get_u2();
if (frame_type < SAME_LOCALS_1_STACK_ITEM_EXTENDED) {
if (frame_type < RESERVED_TAGS_UPPER_LIMIT) {
_verifier.classError("reserved frame type");
}
if (frame_type == SAME_LOCALS_1_STACK_ITEM_EXTENDED) {
Expand Down
37 changes: 32 additions & 5 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@

package com.sun.tools.javac.comp;

import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Map.Entry;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Consumer;

import com.sun.source.tree.CaseTree;
import com.sun.source.tree.LambdaExpressionTree.BodyKind;
import com.sun.tools.javac.code.*;
import com.sun.tools.javac.code.Scope.WriteableScope;
Expand All @@ -60,8 +60,6 @@
import static com.sun.tools.javac.tree.JCTree.Tag.*;
import com.sun.tools.javac.util.JCDiagnostic.Fragment;
import java.util.Arrays;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.function.Predicate;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -218,6 +216,7 @@ public class Flow {
private Env<AttrContext> attrEnv;
private Lint lint;
private final Infer infer;
private final UnsetFieldsInfo unsetFieldsInfo;

public static Flow instance(Context context) {
Flow instance = context.get(flowKey);
Expand Down Expand Up @@ -344,7 +343,7 @@ protected Flow(Context context) {
infer = Infer.instance(context);
rs = Resolve.instance(context);
diags = JCDiagnostic.Factory.instance(context);
Source source = Source.instance(context);
unsetFieldsInfo = UnsetFieldsInfo.instance(context);
}

/**
Expand Down Expand Up @@ -2298,11 +2297,21 @@ void uninit(VarSymbol sym) {
* record an initialization of the variable.
*/
void letInit(JCTree tree) {
letInit(tree, (JCAssign) null);
}

void letInit(JCTree tree, JCAssign assign) {
tree = TreeInfo.skipParens(tree);
if (tree.hasTag(IDENT) || tree.hasTag(SELECT)) {
Symbol sym = TreeInfo.symbol(tree);
if (sym.kind == VAR) {
letInit(tree.pos(), (VarSymbol)sym);
if (isConstructor && sym.isStrict()) {
/* we are initializing a strict field inside of a constructor, we now need to find which fields
* haven't been initialized yet
*/
unsetFieldsInfo.addUnsetFieldsInfo(classDef.sym, assign != null ? assign : tree, findUninitStrictFields());
}
}
}
}
Expand Down Expand Up @@ -2534,6 +2543,13 @@ public void visitMethodDef(JCMethodDecl tree) {
*/
initParam(def);
}
if (isConstructor) {
Set<VarSymbol> unsetFields = findUninitStrictFields();
if (unsetFields != null && !unsetFields.isEmpty()) {
unsetFieldsInfo.addUnsetFieldsInfo(classDef.sym, tree.body, unsetFields);
}
}

// else we are in an instance initializer block;
// leave caught unchanged.
scan(tree.body);
Expand Down Expand Up @@ -2589,6 +2605,17 @@ public void visitMethodDef(JCMethodDecl tree) {
}
}

Set<VarSymbol> findUninitStrictFields() {
Set<VarSymbol> unsetFields = new LinkedHashSet<>();
for (int i = uninits.nextBit(0); i >= 0; i = uninits.nextBit(i + 1)) {
JCVariableDecl variableDecl = vardecls[i];
if (variableDecl.sym.isStrict()) {
unsetFields.add(variableDecl.sym);
}
}
return unsetFields;
}

private void clearPendingExits(boolean inMethod) {
List<PendingExit> exits = pendingExits.toList();
pendingExits = new ListBuffer<>();
Expand Down Expand Up @@ -3153,7 +3180,7 @@ public void visitAssign(JCAssign tree) {
if (!TreeInfo.isIdentOrThisDotIdent(tree.lhs))
scanExpr(tree.lhs);
scanExpr(tree.rhs);
letInit(tree.lhs);
letInit(tree.lhs, tree);
}

// check fields accessed through this.<field> are definitely
Expand Down
Loading