Mon, 08 Dec 2014 15:14:11 +0100
8066227: CodeGenerator load unitialized slot
Reviewed-by: hannesw, lagergren
1.1 --- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Dec 08 15:13:16 2014 +0100 1.2 +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Dec 08 15:14:11 2014 +0100 1.3 @@ -3697,8 +3697,7 @@ 1.4 final Expression lhs = assignNode.lhs(); 1.5 final Expression rhs = assignNode.rhs(); 1.6 final Type widestOperationType = assignNode.getWidestOperationType(); 1.7 - final Type widest = assignNode.isTokenType(TokenType.ASSIGN_ADD) ? Type.OBJECT : widestOperationType; 1.8 - final TypeBounds bounds = new TypeBounds(assignNode.getType(), widest); 1.9 + final TypeBounds bounds = new TypeBounds(assignNode.getType(), widestOperationType); 1.10 new OptimisticOperation(assignNode, bounds) { 1.11 @Override 1.12 void loadStack() {
2.1 --- a/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Mon Dec 08 15:13:16 2014 +0100 2.2 +++ b/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Mon Dec 08 15:14:11 2014 +0100 2.3 @@ -28,6 +28,7 @@ 2.4 import static jdk.nashorn.internal.codegen.CompilerConstants.RETURN; 2.5 import static jdk.nashorn.internal.ir.Expression.isAlwaysFalse; 2.6 import static jdk.nashorn.internal.ir.Expression.isAlwaysTrue; 2.7 + 2.8 import java.util.ArrayDeque; 2.9 import java.util.ArrayList; 2.10 import java.util.Collections; 2.11 @@ -82,7 +83,6 @@ 2.12 import jdk.nashorn.internal.ir.VarNode; 2.13 import jdk.nashorn.internal.ir.WhileNode; 2.14 import jdk.nashorn.internal.ir.visitor.NodeVisitor; 2.15 -import jdk.nashorn.internal.parser.Token; 2.16 import jdk.nashorn.internal.parser.TokenType; 2.17 2.18 /** 2.19 @@ -398,48 +398,53 @@ 2.20 2.21 @Override 2.22 public boolean enterBinaryNode(final BinaryNode binaryNode) { 2.23 + // NOTE: regardless of operator's lexical associativity, lhs is always evaluated first. 2.24 final Expression lhs = binaryNode.lhs(); 2.25 - final Expression rhs = binaryNode.rhs(); 2.26 final boolean isAssignment = binaryNode.isAssignment(); 2.27 - 2.28 - final TokenType tokenType = Token.descType(binaryNode.getToken()); 2.29 - if(tokenType.isLeftAssociative()) { 2.30 - assert !isAssignment; 2.31 - final boolean isLogical = binaryNode.isLogical(); 2.32 - final Label joinLabel = isLogical ? new Label("") : null; 2.33 - lhs.accept(this); 2.34 - if(isLogical) { 2.35 - jumpToLabel((JoinPredecessor)lhs, joinLabel); 2.36 - } 2.37 - rhs.accept(this); 2.38 - if(isLogical) { 2.39 - jumpToLabel((JoinPredecessor)rhs, joinLabel); 2.40 - } 2.41 - joinOnLabel(joinLabel); 2.42 - } else { 2.43 - rhs.accept(this); 2.44 - if(isAssignment) { 2.45 - if(lhs instanceof BaseNode) { 2.46 - ((BaseNode)lhs).getBase().accept(this); 2.47 - if(lhs instanceof IndexNode) { 2.48 - ((IndexNode)lhs).getIndex().accept(this); 2.49 - } else { 2.50 - assert lhs instanceof AccessNode; 2.51 - } 2.52 + LvarType lhsTypeOnLoad = null; 2.53 + if(isAssignment) { 2.54 + if(lhs instanceof BaseNode) { 2.55 + ((BaseNode)lhs).getBase().accept(this); 2.56 + if(lhs instanceof IndexNode) { 2.57 + ((IndexNode)lhs).getIndex().accept(this); 2.58 } else { 2.59 - assert lhs instanceof IdentNode; 2.60 - if(binaryNode.isSelfModifying()) { 2.61 - ((IdentNode)lhs).accept(this); 2.62 - } 2.63 + assert lhs instanceof AccessNode; 2.64 } 2.65 } else { 2.66 - lhs.accept(this); 2.67 + assert lhs instanceof IdentNode; 2.68 + if(binaryNode.isSelfModifying()) { 2.69 + final IdentNode ident = ((IdentNode)lhs); 2.70 + ident.accept(this); 2.71 + // Self-assignment can cause a change in the type of the variable. For purposes of evaluating 2.72 + // the type of the operation, we must use its type as it was when it was loaded. If we didn't 2.73 + // do this, some awkward expressions would end up being calculated incorrectly, e.g. 2.74 + // "var x; x += x = 0;". In this case we have undefined+int so the result type is double (NaN). 2.75 + // However, if we used the type of "x" on LHS after we evaluated RHS, we'd see int+int, so the 2.76 + // result type would be either optimistic int or pessimistic long, which would be wrong. 2.77 + lhsTypeOnLoad = getLocalVariableTypeIfBytecode(ident.getSymbol()); 2.78 + } 2.79 } 2.80 + } else { 2.81 + lhs.accept(this); 2.82 } 2.83 2.84 + final boolean isLogical = binaryNode.isLogical(); 2.85 + assert !(isAssignment && isLogical); // there are no logical assignment operators in JS 2.86 + final Label joinLabel = isLogical ? new Label("") : null; 2.87 + if(isLogical) { 2.88 + jumpToLabel((JoinPredecessor)lhs, joinLabel); 2.89 + } 2.90 + 2.91 + final Expression rhs = binaryNode.rhs(); 2.92 + rhs.accept(this); 2.93 + if(isLogical) { 2.94 + jumpToLabel((JoinPredecessor)rhs, joinLabel); 2.95 + } 2.96 + joinOnLabel(joinLabel); 2.97 + 2.98 if(isAssignment && lhs instanceof IdentNode) { 2.99 if(binaryNode.isSelfModifying()) { 2.100 - onSelfAssignment((IdentNode)lhs, binaryNode); 2.101 + onSelfAssignment((IdentNode)lhs, binaryNode, lhsTypeOnLoad); 2.102 } else { 2.103 onAssignment((IdentNode)lhs, rhs); 2.104 } 2.105 @@ -919,7 +924,8 @@ 2.106 2.107 if(unaryNode.isSelfModifying()) { 2.108 if(expr instanceof IdentNode) { 2.109 - onSelfAssignment((IdentNode)expr, unaryNode); 2.110 + final IdentNode ident = (IdentNode)expr; 2.111 + onSelfAssignment(ident, unaryNode, getLocalVariableTypeIfBytecode(ident.getSymbol())); 2.112 } 2.113 } 2.114 return false; 2.115 @@ -973,12 +979,41 @@ 2.116 return types; 2.117 } 2.118 2.119 + /** 2.120 + * Returns the current type of the local variable represented by the symbol. This is the most strict of all 2.121 + * {@code getLocalVariableType*} methods, as it will throw an assertion if the type is null. Therefore, it is only 2.122 + * safe to be invoked on symbols known to be bytecode locals, and only after they have been initialized. 2.123 + * Regardless, it is recommended to use this method in majority of cases, as because of its strictness it is the 2.124 + * best suited for catching missing type calculation bugs early. 2.125 + * @param symbol a symbol representing a bytecode local variable. 2.126 + * @return the current type of the local variable represented by the symbol 2.127 + */ 2.128 private LvarType getLocalVariableType(final Symbol symbol) { 2.129 final LvarType type = getLocalVariableTypeOrNull(symbol); 2.130 assert type != null; 2.131 return type; 2.132 } 2.133 2.134 + /** 2.135 + * Gets the type for a local variable if it is a bytecode local, otherwise null. Can be used in circumstances where 2.136 + * the type is irrelevant if the symbol is not a bytecode local. Note that for bytecode locals, it delegates to 2.137 + * {@link #getLocalVariableType(Symbol)}, so it will still assert that the type for such variable is already 2.138 + * defined (that is, not null). 2.139 + * @param symbol the symbol representing the variable. 2.140 + * @return the current variable type, if it is a bytecode local, otherwise null. 2.141 + */ 2.142 + private LvarType getLocalVariableTypeIfBytecode(final Symbol symbol) { 2.143 + return symbol.isBytecodeLocal() ? getLocalVariableType(symbol) : null; 2.144 + } 2.145 + 2.146 + /** 2.147 + * Gets the type for a variable represented by a symbol, or null if the type is not know. This is the least strict 2.148 + * of all local variable type getters, and as such its use is discouraged except in initialization scenarios (where 2.149 + * a just-defined symbol might still be null). 2.150 + * @param symbol the symbol 2.151 + * @return the current type for the symbol, or null if the type is not known either because the symbol has not been 2.152 + * initialized, or because the symbol does not represent a bytecode local variable. 2.153 + */ 2.154 private LvarType getLocalVariableTypeOrNull(final Symbol symbol) { 2.155 return localVariableTypes.get(symbol); 2.156 } 2.157 @@ -1358,13 +1393,13 @@ 2.158 jumpToCatchBlock(identNode); 2.159 } 2.160 2.161 - private void onSelfAssignment(final IdentNode identNode, final Expression assignment) { 2.162 + private void onSelfAssignment(final IdentNode identNode, final Expression assignment, final LvarType typeOnLoad) { 2.163 final Symbol symbol = identNode.getSymbol(); 2.164 assert symbol != null : identNode.getName(); 2.165 if(!symbol.isBytecodeLocal()) { 2.166 return; 2.167 } 2.168 - final LvarType type = toLvarType(getType(assignment)); 2.169 + final LvarType type = toLvarType(getType(assignment, symbol, typeOnLoad.type)); 2.170 // Self-assignment never produce either a boolean or undefined 2.171 assert type != null && type != LvarType.UNDEFINED && type != LvarType.BOOLEAN; 2.172 setType(symbol, type); 2.173 @@ -1445,13 +1480,24 @@ 2.174 symbolIsUsed(symbol, getLocalVariableType(symbol)); 2.175 } 2.176 2.177 + /** 2.178 + * Gets the type of the expression, dependent on the current types of the local variables. 2.179 + * 2.180 + * @param expr the expression 2.181 + * @return the current type of the expression dependent on the current types of the local variables. 2.182 + */ 2.183 private Type getType(final Expression expr) { 2.184 return expr.getType(getSymbolToType()); 2.185 } 2.186 2.187 + /** 2.188 + * Returns a function object from symbols to their types, used by the expressions to evaluate their type. 2.189 + * {@link BinaryNode} specifically uses identity of the function to cache type calculations. This method makes 2.190 + * sure to return the same function object while the local variable types don't change, and create a new function 2.191 + * object if the local variable types have been changed. 2.192 + * @return a function object representing a mapping from symbols to their types. 2.193 + */ 2.194 private Function<Symbol, Type> getSymbolToType() { 2.195 - // BinaryNode uses identity of the function to cache type calculations. Therefore, we must use different 2.196 - // function instances for different localVariableTypes instances. 2.197 if(symbolToType.isStale()) { 2.198 symbolToType = new SymbolToType(); 2.199 } 2.200 @@ -1469,4 +1515,41 @@ 2.201 return boundTypes != localVariableTypes; 2.202 } 2.203 } 2.204 + 2.205 + /** 2.206 + * Gets the type of the expression, dependent on the current types of the local variables and a single overridden 2.207 + * symbol type. Used by type calculation on compound operators to ensure the type of the LHS at the time it was 2.208 + * loaded (which can potentially be different after RHS evaluation, e.g. "var x; x += x = 0;") is preserved for 2.209 + * the calculation. 2.210 + * 2.211 + * @param expr the expression 2.212 + * @param overriddenSymbol the overridden symbol 2.213 + * @param overriddenType the overridden type 2.214 + * @return the current type of the expression dependent on the current types of the local variables and the single 2.215 + * potentially overridden type. 2.216 + */ 2.217 + private Type getType(final Expression expr, final Symbol overriddenSymbol, final Type overriddenType) { 2.218 + return expr.getType(getSymbolToType(overriddenSymbol, overriddenType)); 2.219 + } 2.220 + 2.221 + private Function<Symbol, Type> getSymbolToType(final Symbol overriddenSymbol, final Type overriddenType) { 2.222 + return getLocalVariableType(overriddenSymbol).type == overriddenType ? getSymbolToType() : 2.223 + new SymbolToTypeOverride(overriddenSymbol, overriddenType); 2.224 + } 2.225 + 2.226 + private class SymbolToTypeOverride implements Function<Symbol, Type> { 2.227 + private final Function<Symbol, Type> originalSymbolToType = getSymbolToType(); 2.228 + private final Symbol overriddenSymbol; 2.229 + private final Type overriddenType; 2.230 + 2.231 + SymbolToTypeOverride(final Symbol overriddenSymbol, final Type overriddenType) { 2.232 + this.overriddenSymbol = overriddenSymbol; 2.233 + this.overriddenType = overriddenType; 2.234 + } 2.235 + 2.236 + @Override 2.237 + public Type apply(final Symbol symbol) { 2.238 + return symbol == overriddenSymbol ? overriddenType : originalSymbolToType.apply(symbol); 2.239 + } 2.240 + } 2.241 }
3.1 --- a/src/jdk/nashorn/internal/ir/BinaryNode.java Mon Dec 08 15:13:16 2014 +0100 3.2 +++ b/src/jdk/nashorn/internal/ir/BinaryNode.java Mon Dec 08 15:14:11 2014 +0100 3.3 @@ -341,10 +341,7 @@ 3.4 @Override 3.5 public Node accept(final NodeVisitor<? extends LexicalContext> visitor) { 3.6 if (visitor.enterBinaryNode(this)) { 3.7 - if(tokenType().isLeftAssociative()) { 3.8 - return visitor.leaveBinaryNode(setLHS((Expression)lhs.accept(visitor)).setRHS((Expression)rhs.accept(visitor))); 3.9 - } 3.10 - return visitor.leaveBinaryNode(setRHS((Expression)rhs.accept(visitor)).setLHS((Expression)lhs.accept(visitor))); 3.11 + return visitor.leaveBinaryNode(setLHS((Expression)lhs.accept(visitor)).setRHS((Expression)rhs.accept(visitor))); 3.12 } 3.13 3.14 return this;
4.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 4.2 +++ b/test/script/basic/JDK-8066227.js Mon Dec 08 15:14:11 2014 +0100 4.3 @@ -0,0 +1,40 @@ 4.4 +/* 4.5 + * Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved. 4.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 4.7 + * 4.8 + * This code is free software; you can redistribute it and/or modify it 4.9 + * under the terms of the GNU General Public License version 2 only, as 4.10 + * published by the Free Software Foundation. 4.11 + * 4.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 4.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 4.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 4.15 + * version 2 for more details (a copy is included in the LICENSE file that 4.16 + * accompanied this code). 4.17 + * 4.18 + * You should have received a copy of the GNU General Public License version 4.19 + * 2 along with this work; if not, write to the Free Software Foundation, 4.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 4.21 + * 4.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 4.23 + * or visit www.oracle.com if you need additional information or have any 4.24 + * questions. 4.25 + */ 4.26 + 4.27 +/** 4.28 + * JDK-8066227: CodeGenerator load unitialized slot 4.29 + * 4.30 + * @test 4.31 + * @run 4.32 + */ 4.33 + 4.34 +print((function () { var x; (x += x = 0); return x; })()); 4.35 +print((function () { var x; (x -= x = 0); return x; })()); 4.36 +print((function () { var x; (x *= x = 0); return x; })()); 4.37 +print((function () { var x; (x /= x = 0); return x; })()); 4.38 +print((function () { var x; (x %= x = 0); return x; })()); 4.39 +print((function () { var x; (x <<= x = 0); return x; })()); 4.40 +print((function () { var x; (x >>= x = 0); return x; })()); 4.41 +print((function () { var x; (x >>>= x = 0); return x; })()); 4.42 +print((function () { var x; (x |= x = 0); return x; })()); 4.43 +print((function () { var x; (x &= x = 0); return x; })());
5.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 5.2 +++ b/test/script/basic/JDK-8066227.js.EXPECTED Mon Dec 08 15:14:11 2014 +0100 5.3 @@ -0,0 +1,10 @@ 5.4 +NaN 5.5 +NaN 5.6 +NaN 5.7 +NaN 5.8 +NaN 5.9 +0 5.10 +0 5.11 +0 5.12 +0 5.13 +0