--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Dec 08 15:13:16 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Dec 08 15:14:11 2014 +0100
@@ -3698,8 +3698,7 @@
final Expression lhs = assignNode.lhs();
final Expression rhs = assignNode.rhs();
final Type widestOperationType = assignNode.getWidestOperationType();
- final Type widest = assignNode.isTokenType(TokenType.ASSIGN_ADD) ? Type.OBJECT : widestOperationType;
- final TypeBounds bounds = new TypeBounds(assignNode.getType(), widest);
+ final TypeBounds bounds = new TypeBounds(assignNode.getType(), widestOperationType);
new OptimisticOperation(assignNode, bounds) {
@Override
void loadStack() {
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Mon Dec 08 15:13:16 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Mon Dec 08 15:14:11 2014 +0100
@@ -28,6 +28,7 @@
import static jdk.nashorn.internal.codegen.CompilerConstants.RETURN;
import static jdk.nashorn.internal.ir.Expression.isAlwaysFalse;
import static jdk.nashorn.internal.ir.Expression.isAlwaysTrue;
+
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
@@ -82,7 +83,6 @@
import jdk.nashorn.internal.ir.VarNode;
import jdk.nashorn.internal.ir.WhileNode;
import jdk.nashorn.internal.ir.visitor.NodeVisitor;
-import jdk.nashorn.internal.parser.Token;
import jdk.nashorn.internal.parser.TokenType;
/**
@@ -398,48 +398,53 @@
@Override
public boolean enterBinaryNode(final BinaryNode binaryNode) {
+ // NOTE: regardless of operator's lexical associativity, lhs is always evaluated first.
final Expression lhs = binaryNode.lhs();
- final Expression rhs = binaryNode.rhs();
final boolean isAssignment = binaryNode.isAssignment();
-
- final TokenType tokenType = Token.descType(binaryNode.getToken());
- if(tokenType.isLeftAssociative()) {
- assert !isAssignment;
- final boolean isLogical = binaryNode.isLogical();
- final Label joinLabel = isLogical ? new Label("") : null;
- lhs.accept(this);
- if(isLogical) {
- jumpToLabel((JoinPredecessor)lhs, joinLabel);
- }
- rhs.accept(this);
- if(isLogical) {
- jumpToLabel((JoinPredecessor)rhs, joinLabel);
- }
- joinOnLabel(joinLabel);
- } else {
- rhs.accept(this);
- if(isAssignment) {
- if(lhs instanceof BaseNode) {
- ((BaseNode)lhs).getBase().accept(this);
- if(lhs instanceof IndexNode) {
- ((IndexNode)lhs).getIndex().accept(this);
- } else {
- assert lhs instanceof AccessNode;
- }
+ LvarType lhsTypeOnLoad = null;
+ if(isAssignment) {
+ if(lhs instanceof BaseNode) {
+ ((BaseNode)lhs).getBase().accept(this);
+ if(lhs instanceof IndexNode) {
+ ((IndexNode)lhs).getIndex().accept(this);
} else {
- assert lhs instanceof IdentNode;
- if(binaryNode.isSelfModifying()) {
- ((IdentNode)lhs).accept(this);
- }
+ assert lhs instanceof AccessNode;
}
} else {
- lhs.accept(this);
+ assert lhs instanceof IdentNode;
+ if(binaryNode.isSelfModifying()) {
+ final IdentNode ident = ((IdentNode)lhs);
+ ident.accept(this);
+ // Self-assignment can cause a change in the type of the variable. For purposes of evaluating
+ // the type of the operation, we must use its type as it was when it was loaded. If we didn't
+ // do this, some awkward expressions would end up being calculated incorrectly, e.g.
+ // "var x; x += x = 0;". In this case we have undefined+int so the result type is double (NaN).
+ // However, if we used the type of "x" on LHS after we evaluated RHS, we'd see int+int, so the
+ // result type would be either optimistic int or pessimistic long, which would be wrong.
+ lhsTypeOnLoad = getLocalVariableTypeIfBytecode(ident.getSymbol());
+ }
}
+ } else {
+ lhs.accept(this);
}
+ final boolean isLogical = binaryNode.isLogical();
+ assert !(isAssignment && isLogical); // there are no logical assignment operators in JS
+ final Label joinLabel = isLogical ? new Label("") : null;
+ if(isLogical) {
+ jumpToLabel((JoinPredecessor)lhs, joinLabel);
+ }
+
+ final Expression rhs = binaryNode.rhs();
+ rhs.accept(this);
+ if(isLogical) {
+ jumpToLabel((JoinPredecessor)rhs, joinLabel);
+ }
+ joinOnLabel(joinLabel);
+
if(isAssignment && lhs instanceof IdentNode) {
if(binaryNode.isSelfModifying()) {
- onSelfAssignment((IdentNode)lhs, binaryNode);
+ onSelfAssignment((IdentNode)lhs, binaryNode, lhsTypeOnLoad);
} else {
onAssignment((IdentNode)lhs, rhs);
}
@@ -919,7 +924,8 @@
if(unaryNode.isSelfModifying()) {
if(expr instanceof IdentNode) {
- onSelfAssignment((IdentNode)expr, unaryNode);
+ final IdentNode ident = (IdentNode)expr;
+ onSelfAssignment(ident, unaryNode, getLocalVariableTypeIfBytecode(ident.getSymbol()));
}
}
return false;
@@ -973,12 +979,41 @@
return types;
}
+ /**
+ * Returns the current type of the local variable represented by the symbol. This is the most strict of all
+ * {@code getLocalVariableType*} methods, as it will throw an assertion if the type is null. Therefore, it is only
+ * safe to be invoked on symbols known to be bytecode locals, and only after they have been initialized.
+ * Regardless, it is recommended to use this method in majority of cases, as because of its strictness it is the
+ * best suited for catching missing type calculation bugs early.
+ * @param symbol a symbol representing a bytecode local variable.
+ * @return the current type of the local variable represented by the symbol
+ */
private LvarType getLocalVariableType(final Symbol symbol) {
final LvarType type = getLocalVariableTypeOrNull(symbol);
assert type != null;
return type;
}
+ /**
+ * Gets the type for a local variable if it is a bytecode local, otherwise null. Can be used in circumstances where
+ * the type is irrelevant if the symbol is not a bytecode local. Note that for bytecode locals, it delegates to
+ * {@link #getLocalVariableType(Symbol)}, so it will still assert that the type for such variable is already
+ * defined (that is, not null).
+ * @param symbol the symbol representing the variable.
+ * @return the current variable type, if it is a bytecode local, otherwise null.
+ */
+ private LvarType getLocalVariableTypeIfBytecode(final Symbol symbol) {
+ return symbol.isBytecodeLocal() ? getLocalVariableType(symbol) : null;
+ }
+
+ /**
+ * Gets the type for a variable represented by a symbol, or null if the type is not know. This is the least strict
+ * of all local variable type getters, and as such its use is discouraged except in initialization scenarios (where
+ * a just-defined symbol might still be null).
+ * @param symbol the symbol
+ * @return the current type for the symbol, or null if the type is not known either because the symbol has not been
+ * initialized, or because the symbol does not represent a bytecode local variable.
+ */
private LvarType getLocalVariableTypeOrNull(final Symbol symbol) {
return localVariableTypes.get(symbol);
}
@@ -1358,13 +1393,13 @@
jumpToCatchBlock(identNode);
}
- private void onSelfAssignment(final IdentNode identNode, final Expression assignment) {
+ private void onSelfAssignment(final IdentNode identNode, final Expression assignment, final LvarType typeOnLoad) {
final Symbol symbol = identNode.getSymbol();
assert symbol != null : identNode.getName();
if(!symbol.isBytecodeLocal()) {
return;
}
- final LvarType type = toLvarType(getType(assignment));
+ final LvarType type = toLvarType(getType(assignment, symbol, typeOnLoad.type));
// Self-assignment never produce either a boolean or undefined
assert type != null && type != LvarType.UNDEFINED && type != LvarType.BOOLEAN;
setType(symbol, type);
@@ -1445,13 +1480,24 @@
symbolIsUsed(symbol, getLocalVariableType(symbol));
}
+ /**
+ * Gets the type of the expression, dependent on the current types of the local variables.
+ *
+ * @param expr the expression
+ * @return the current type of the expression dependent on the current types of the local variables.
+ */
private Type getType(final Expression expr) {
return expr.getType(getSymbolToType());
}
+ /**
+ * Returns a function object from symbols to their types, used by the expressions to evaluate their type.
+ * {@link BinaryNode} specifically uses identity of the function to cache type calculations. This method makes
+ * sure to return the same function object while the local variable types don't change, and create a new function
+ * object if the local variable types have been changed.
+ * @return a function object representing a mapping from symbols to their types.
+ */
private Function<Symbol, Type> getSymbolToType() {
- // BinaryNode uses identity of the function to cache type calculations. Therefore, we must use different
- // function instances for different localVariableTypes instances.
if(symbolToType.isStale()) {
symbolToType = new SymbolToType();
}
@@ -1469,4 +1515,41 @@
return boundTypes != localVariableTypes;
}
}
+
+ /**
+ * Gets the type of the expression, dependent on the current types of the local variables and a single overridden
+ * symbol type. Used by type calculation on compound operators to ensure the type of the LHS at the time it was
+ * loaded (which can potentially be different after RHS evaluation, e.g. "var x; x += x = 0;") is preserved for
+ * the calculation.
+ *
+ * @param expr the expression
+ * @param overriddenSymbol the overridden symbol
+ * @param overriddenType the overridden type
+ * @return the current type of the expression dependent on the current types of the local variables and the single
+ * potentially overridden type.
+ */
+ private Type getType(final Expression expr, final Symbol overriddenSymbol, final Type overriddenType) {
+ return expr.getType(getSymbolToType(overriddenSymbol, overriddenType));
+ }
+
+ private Function<Symbol, Type> getSymbolToType(final Symbol overriddenSymbol, final Type overriddenType) {
+ return getLocalVariableType(overriddenSymbol).type == overriddenType ? getSymbolToType() :
+ new SymbolToTypeOverride(overriddenSymbol, overriddenType);
+ }
+
+ private class SymbolToTypeOverride implements Function<Symbol, Type> {
+ private final Function<Symbol, Type> originalSymbolToType = getSymbolToType();
+ private final Symbol overriddenSymbol;
+ private final Type overriddenType;
+
+ SymbolToTypeOverride(final Symbol overriddenSymbol, final Type overriddenType) {
+ this.overriddenSymbol = overriddenSymbol;
+ this.overriddenType = overriddenType;
+ }
+
+ @Override
+ public Type apply(final Symbol symbol) {
+ return symbol == overriddenSymbol ? overriddenType : originalSymbolToType.apply(symbol);
+ }
+ }
}
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/BinaryNode.java Mon Dec 08 15:13:16 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/BinaryNode.java Mon Dec 08 15:14:11 2014 +0100
@@ -341,10 +341,7 @@
@Override
public Node accept(final NodeVisitor<? extends LexicalContext> visitor) {
if (visitor.enterBinaryNode(this)) {
- if(tokenType().isLeftAssociative()) {
- return visitor.leaveBinaryNode(setLHS((Expression)lhs.accept(visitor)).setRHS((Expression)rhs.accept(visitor)));
- }
- return visitor.leaveBinaryNode(setRHS((Expression)rhs.accept(visitor)).setLHS((Expression)lhs.accept(visitor)));
+ return visitor.leaveBinaryNode(setLHS((Expression)lhs.accept(visitor)).setRHS((Expression)rhs.accept(visitor)));
}
return this;
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8066227.js Mon Dec 08 15:14:11 2014 +0100
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2014 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8066227: CodeGenerator load unitialized slot
+ *
+ * @test
+ * @run
+ */
+
+print((function () { var x; (x += x = 0); return x; })());
+print((function () { var x; (x -= x = 0); return x; })());
+print((function () { var x; (x *= x = 0); return x; })());
+print((function () { var x; (x /= x = 0); return x; })());
+print((function () { var x; (x %= x = 0); return x; })());
+print((function () { var x; (x <<= x = 0); return x; })());
+print((function () { var x; (x >>= x = 0); return x; })());
+print((function () { var x; (x >>>= x = 0); return x; })());
+print((function () { var x; (x |= x = 0); return x; })());
+print((function () { var x; (x &= x = 0); return x; })());
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8066227.js.EXPECTED Mon Dec 08 15:14:11 2014 +0100
@@ -0,0 +1,10 @@
+NaN
+NaN
+NaN
+NaN
+NaN
+0
+0
+0
+0
+0