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.

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

8335922: Incorrect @Stable usage of LambdaForm$Name.index #20178

Closed
wants to merge 8 commits into from
+23 −47
Closed

8335922: Incorrect @Stable usage of LambdaForm$Name.index #20178

Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7d111ba
8335922: Incorrect stable usage of LambdaForm$Name.index
liach Jul 15, 2024
0743c5f
We have sufficient space in short, use +1 offset
liach Jul 15, 2024
db80583
Encapsulate offsetIndex, share computation result
liach Jul 15, 2024
e779ce4
Revert "Encapsulate offsetIndex, share computation result"
liach Jul 15, 2024
7709d31
Revert "We have sufficient space in short, use +1 offset"
liach Jul 15, 2024
061f48d
Revert "8335922: Incorrect stable usage of LambdaForm$Name.index"
liach Jul 15, 2024
771755b
Make LambdaForm.Name index final
liach Jul 15, 2024
9846703
Update src/java.base/share/classes/java/lang/invoke/LambdaForm.java
liach Jul 16, 2024
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
92 changes: 33 additions & 59 deletions src/java.base/share/classes/java/lang/invoke/LambdaForm.java
View file
Original file line number Diff line number Diff line change
@@ -395,7 +395,7 @@ private static Name[] buildEmptyNames(int arity, MethodType mt, boolean isVoid)
Name[] names = arguments(isVoid ? 0 : 1, mt);
if (!isVoid) {
Name zero = new Name(constantZero(basicType(mt.returnType())));
names[arity] = zero.newIndex(arity);
names[arity] = zero.withIndex(arity);
}
assert(namesOK(arity, names));
return names;
@@ -495,28 +495,18 @@ LambdaForm uncustomize() {
* @return true if we can interpret
*/
private static boolean normalizeNames(int arity, Name[] names) {
Name[] oldNames = null;
Name[] oldNames = names.clone();
int maxOutArity = 0;
int changesStart = 0;
for (int i = 0; i < names.length; i++) {
Name n = names[i];
if (!n.initIndex(i)) {
if (oldNames == null) {
oldNames = names.clone();
changesStart = i;
}
names[i] = n.cloneWithIndex(i);
}
names[i] = n.withIndex(i);
if (n.arguments != null && maxOutArity < n.arguments.length)
maxOutArity = n.arguments.length;
}
if (oldNames != null) {
int startFixing = arity;
if (startFixing <= changesStart)
startFixing = changesStart+1;
for (int i = startFixing; i < names.length; i++) {
Name fixed = names[i].replaceNames(oldNames, names, changesStart, i);
names[i] = fixed.newIndex(i);
for (int i = Math.max(1, arity); i < names.length; i++) {
Name fixed = names[i].replaceNames(oldNames, names, 0, i);
names[i] = fixed.withIndex(i);
}
}
int maxInterned = Math.min(arity, INTERNED_ARGUMENT_LIMIT);
@@ -567,7 +557,7 @@ boolean nameRefsAreLegal() {
assert(n.index() == i);
for (Object arg : n.arguments) {
if (arg instanceof Name n2) {
int i2 = n2.index();
int i2 = n2.index;
assert(0 <= i2 && i2 < names.length) : n.debugString() + ": 0 <= i2 && i2 < names.length: 0 <= " + i2 + " < " + names.length;
assert(names[i2] == n2) : Arrays.asList("-1-", i, "-2-", n.debugString(), "-3-", i2, "-4-", n2.debugString(), "-5-", names[i2].debugString(), "-6-", this);
assert(i2 < i); // ref must come after def!
@@ -1339,30 +1329,24 @@ public static String shortenSignature(String signature) {

static final class Name {
final BasicType type;
private @Stable short offsetIndex; // slot + 1, reserves 0 for unset
final short index;
final NamedFunction function;
final Object constraint; // additional type information, if not null
@Stable final Object[] arguments;

private static final Object[] EMPTY_ARGS = new Object[0];

private Name(int index, BasicType type, NamedFunction function, Object[] arguments) {
this.offsetIndex = (short) (index + 1);
private Name(int index, BasicType type, NamedFunction function, Object[] arguments, Object constraint) {
this.index = (short)index;
this.type = type;
this.function = function;
this.arguments = arguments;
this.constraint = null;
assert(this.index() == index && typesMatch(function, this.arguments));
}
private Name(Name that, Object constraint) {
this.offsetIndex = that.offsetIndex;
this.type = that.type;
this.function = that.function;
this.arguments = that.arguments;
this.constraint = constraint;
assert(this.index == index && typesMatch(function, arguments));
assert(constraint == null || isParam()); // only params have constraints
assert(constraint == null || constraint instanceof ClassSpecializer.SpeciesData || constraint instanceof Class);
}

Name(MethodHandle function, Object... arguments) {
this(new NamedFunction(function), arguments);
}
@@ -1374,50 +1358,40 @@ private Name(Name that, Object constraint) {
this(new NamedFunction(function), arguments);
}
Name(NamedFunction function) {
this(-1, function.returnType(), function, EMPTY_ARGS);
this(-1, function.returnType(), function, EMPTY_ARGS, null);
}
Name(NamedFunction function, Object arg) {
this(-1, function.returnType(), function, new Object[] { arg });
this(-1, function.returnType(), function, new Object[] { arg }, null);
}
Name(NamedFunction function, Object arg0, Object arg1) {
this(-1, function.returnType(), function, new Object[] { arg0, arg1 });
this(-1, function.returnType(), function, new Object[] { arg0, arg1 }, null);
}
Name(NamedFunction function, Object... arguments) {
this(-1, function.returnType(), function, Arrays.copyOf(arguments, arguments.length, Object[].class));
this(-1, function.returnType(), function, Arrays.copyOf(arguments, arguments.length, Object[].class), null);
}
/** Create a raw parameter of the given type, with an expected index. */
Name(int index, BasicType type) {
this(index, type, null, null);
this(index, type, null, null, null);
}
/** Create a raw parameter of the given type. */
Name(BasicType type) { this(-1, type); }

BasicType type() { return type; }
int index() { return offsetIndex - 1; }
boolean initIndex(int i) {
short index = (short) (i + 1);
if (offsetIndex != index) {
if (offsetIndex != 0) return false;
offsetIndex = index;
}
return true;
}
int index() { return index; }

char typeChar() {
return type.btChar;
}

Name newIndex(int i) {
if (initIndex(i)) return this;
return cloneWithIndex(i);
}
Name cloneWithIndex(int i) {
Object[] newArguments = (arguments == null) ? null : arguments.clone();
return new Name(i, type, function, newArguments).withConstraint(constraint);
Name withIndex(int i) {
return new Name(i, type, function, arguments, constraint);
}

Name withConstraint(Object constraint) {
if (constraint == this.constraint) return this;
return new Name(this, constraint);
return new Name(index, type, function, arguments, constraint);
}

Name replaceName(Name oldName, Name newName) { // FIXME: use replaceNames uniformly
if (oldName == newName) return this;
@SuppressWarnings("LocalVariableHidesMemberVariable")
@@ -1447,7 +1421,7 @@ Name replaceNames(Name[] oldNames, Name[] newNames, int start, int end) {
eachArg:
for (int j = 0; j < arguments.length; j++) {
if (arguments[j] instanceof Name n) {
int check = n.index();
int check = n.index;
// harmless check to see if the thing is already in newNames:
if (check >= 0 && check < newNames.length && n == newNames[check])
continue eachArg;
@@ -1474,7 +1448,7 @@ void internArguments() {
Object[] arguments = this.arguments;
for (int j = 0; j < arguments.length; j++) {
if (arguments[j] instanceof Name n) {
if (n.isParam() && n.index() < INTERNED_ARGUMENT_LIMIT)
if (n.isParam() && n.index < INTERNED_ARGUMENT_LIMIT)
arguments[j] = internArgument(n);
}
}
@@ -1520,7 +1494,7 @@ boolean isLinkerMethodInvoke() {
}

public String toString() {
return (isParam() ? "a" : "t") + (offsetIndex != 0 ? index() : System.identityHashCode(this)) + ":" + typeChar();
return (isParam()?"a":"t")+(index >= 0 ? index : System.identityHashCode(this))+":"+typeChar();
}
public String debugString() {
String s = paramString();
@@ -1620,7 +1594,7 @@ public boolean equals(Object x) {
@Override
public int hashCode() {
if (isParam())
return index() | (type.ordinal() << 8);
return index | (type.ordinal() << 8);
return function.hashCode() ^ Arrays.hashCode(arguments);
}
}
@@ -1629,7 +1603,7 @@ public int hashCode() {
* Return -1 if the name is not used. Return names.length if it is the return value.
*/
int lastUseIndex(Name n) {
int ni = n.index(), nmax = names.length;
int ni = n.index, nmax = names.length;
assert(names[ni] == n);
if (result == ni) return nmax; // live all the way beyond the end
for (int i = nmax; --i > ni; ) {
@@ -1641,8 +1615,8 @@ int lastUseIndex(Name n) {

/** Return the number of times n is used as an argument or return value. */
int useCount(Name n) {
int count = (result == n.index()) ? 1 : 0;
int i = Math.max(n.index() + 1, arity);
int count = (result == n.index) ? 1 : 0;
int i = Math.max(n.index + 1, arity);
while (i < names.length) {
count += names[i++].useCount(n);
}
@@ -1656,9 +1630,9 @@ static Name argument(int which, BasicType type) {
}
static Name internArgument(Name n) {
assert(n.isParam()) : "not param: " + n;
assert(n.index() < INTERNED_ARGUMENT_LIMIT);
assert(n.index < INTERNED_ARGUMENT_LIMIT);
if (n.constraint != null) return n;
return argument(n.index(), n.type);
return argument(n.index, n.type);
}
static Name[] arguments(int extra, MethodType types) {
int length = types.parameterCount();
4 changes: 2 additions & 2 deletions src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 2023, 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
@@ -775,7 +775,7 @@ private LambdaForm makeRepeatedFilterForm(MethodType combinerType, int... positi
// and expressions are
var newParameters = new TreeMap<Name, Integer>(new Comparator<>() {
public int compare(Name n1, Name n2) {
return n1.index() - n2.index();
return n1.index - n2.index;
}
});