--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Fri Nov 11 05:11:55 2016 +0000
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java Fri Nov 11 15:50:51 2016 +0100
@@ -33,6 +33,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.Deque;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
@@ -122,9 +123,9 @@
private final List<JumpOrigin> origins = new LinkedList<>();
private Map<Symbol, LvarType> types = Collections.emptyMap();
- void addOrigin(final JoinPredecessor originNode, final Map<Symbol, LvarType> originTypes) {
+ void addOrigin(final JoinPredecessor originNode, final Map<Symbol, LvarType> originTypes, final LocalVariableTypesCalculator calc) {
origins.add(new JumpOrigin(originNode, originTypes));
- this.types = getUnionTypes(this.types, originTypes);
+ this.types = calc.getUnionTypes(this.types, originTypes);
}
}
private enum LvarType {
@@ -185,12 +186,15 @@
}
@SuppressWarnings("unchecked")
- private static IdentityHashMap<Symbol, LvarType> cloneMap(final Map<Symbol, LvarType> map) {
- return (IdentityHashMap<Symbol, LvarType>)((IdentityHashMap<?,?>)map).clone();
+ private static HashMap<Symbol, LvarType> cloneMap(final Map<Symbol, LvarType> map) {
+ return (HashMap<Symbol, LvarType>)((HashMap<?,?>)map).clone();
}
private LocalVariableConversion createConversion(final Symbol symbol, final LvarType branchLvarType,
final Map<Symbol, LvarType> joinLvarTypes, final LocalVariableConversion next) {
+ if (invalidatedSymbols.contains(symbol)) {
+ return next;
+ }
final LvarType targetType = joinLvarTypes.get(symbol);
assert targetType != null;
if(targetType == branchLvarType) {
@@ -208,7 +212,7 @@
return new LocalVariableConversion(symbol, branchLvarType.type, targetType.type, next);
}
- private static Map<Symbol, LvarType> getUnionTypes(final Map<Symbol, LvarType> types1, final Map<Symbol, LvarType> types2) {
+ private Map<Symbol, LvarType> getUnionTypes(final Map<Symbol, LvarType> types1, final Map<Symbol, LvarType> types2) {
if(types1 == types2 || types1.isEmpty()) {
return types2;
} else if(types2.isEmpty()) {
@@ -261,6 +265,11 @@
final LvarType type2 = types2.get(symbol);
union.put(symbol, widestLvarType(type1, type2));
}
+ // If the two sets of symbols differ, there's a good chance that some of
+ // symbols only appearing in one of the sets are lexically invalidated,
+ // so we remove them from further consideration.
+ // This is not strictly necessary, just a working set size optimization.
+ union.keySet().removeAll(invalidatedSymbols);
return union;
}
@@ -359,8 +368,6 @@
if(t1.ordinal() < LvarType.INT.ordinal() || t2.ordinal() < LvarType.INT.ordinal()) {
return LvarType.OBJECT;
}
- // NOTE: we allow "widening" of long to double even though it can lose precision. ECMAScript doesn't have an
- // Int64 type anyway, so this loss of precision is actually more conformant to the specification...
return LvarType.values()[Math.max(t1.ordinal(), t2.ordinal())];
}
private final Compiler compiler;
@@ -368,7 +375,10 @@
// Local variable type mapping at the currently evaluated point. No map instance is ever modified; setLvarType() always
// allocates a new map. Immutability of maps allows for cheap snapshots by just keeping the reference to the current
// value.
- private Map<Symbol, LvarType> localVariableTypes = new IdentityHashMap<>();
+ private Map<Symbol, LvarType> localVariableTypes = Collections.emptyMap();
+ // Set of symbols whose lexical scope has already ended.
+ private final Set<Symbol> invalidatedSymbols = new HashSet<>();
+
// Stack for evaluated expression types.
private final Deque<LvarType> typeStack = new ArrayDeque<>();
@@ -464,9 +474,19 @@
@Override
public boolean enterBlock(final Block block) {
+ boolean cloned = false;
for(final Symbol symbol: block.getSymbols()) {
- if(symbol.isBytecodeLocal() && getLocalVariableTypeOrNull(symbol) == null) {
- setType(symbol, LvarType.UNDEFINED);
+ if(symbol.isBytecodeLocal()) {
+ if (getLocalVariableTypeOrNull(symbol) == null) {
+ if (!cloned) {
+ cloneOrNewLocalVariableTypes();
+ cloned = true;
+ }
+ localVariableTypes.put(symbol, LvarType.UNDEFINED);
+ }
+ // In case we're repeating analysis of a lexical scope (e.g. it's in a loop),
+ // make sure all symbols lexically scoped by the block become valid again.
+ invalidatedSymbols.remove(symbol);
}
}
return true;
@@ -1046,15 +1066,11 @@
// throw an exception.
reachable = true;
catchBody.accept(this);
- final Symbol exceptionSymbol = exception.getSymbol();
if(reachable) {
- localVariableTypes = cloneMap(localVariableTypes);
- localVariableTypes.remove(exceptionSymbol);
jumpToLabel(catchBody, endLabel);
canExit = true;
}
- localVariableTypes = cloneMap(afterConditionTypes);
- localVariableTypes.remove(exceptionSymbol);
+ localVariableTypes = afterConditionTypes;
}
// NOTE: if we had one or more conditional catch blocks with no unconditional catch block following them, then
// there will be an unconditional rethrow, so the join point can never be reached from the last
@@ -1204,7 +1220,7 @@
}
private void jumpToLabel(final JoinPredecessor jumpOrigin, final Label label, final Map<Symbol, LvarType> types) {
- getOrCreateJumpTarget(label).addOrigin(jumpOrigin, types);
+ getOrCreateJumpTarget(label).addOrigin(jumpOrigin, types, this);
}
@Override
@@ -1226,16 +1242,18 @@
boolean cloned = false;
for(final Symbol symbol: block.getSymbols()) {
- // Undefine the symbol outside the block
- if(localVariableTypes.containsKey(symbol)) {
- if(!cloned) {
- localVariableTypes = cloneMap(localVariableTypes);
- cloned = true;
+ if(symbol.hasSlot()) {
+ // Invalidate the symbol when its defining block ends
+ if (symbol.isBytecodeLocal()) {
+ if(localVariableTypes.containsKey(symbol)) {
+ if(!cloned) {
+ localVariableTypes = cloneMap(localVariableTypes);
+ cloned = true;
+ }
+ }
+ invalidateSymbol(symbol);
}
- localVariableTypes.remove(symbol);
- }
- if(symbol.hasSlot()) {
final SymbolConversions conversions = symbolConversions.get(symbol);
if(conversions != null) {
// Potentially make some currently dead types live if they're needed as a source of a type
@@ -1605,10 +1623,19 @@
}
assert symbol.hasSlot();
assert !symbol.isGlobal();
- localVariableTypes = localVariableTypes.isEmpty() ? new IdentityHashMap<Symbol, LvarType>() : cloneMap(localVariableTypes);
+ cloneOrNewLocalVariableTypes();
localVariableTypes.put(symbol, type);
}
+ private void cloneOrNewLocalVariableTypes() {
+ localVariableTypes = localVariableTypes.isEmpty() ? new HashMap<Symbol, LvarType>() : cloneMap(localVariableTypes);
+ }
+
+ private void invalidateSymbol(final Symbol symbol) {
+ localVariableTypes.remove(symbol);
+ invalidatedSymbols.add(symbol);
+ }
+
/**
* Set a flag in the symbol marking it as needing to be able to store a value of a particular type. Every symbol for
* a local variable will be assigned between 1 and 6 local variable slots for storing all types it is known to need
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/es6/JDK-8168373.js Fri Nov 11 15:50:51 2016 +0100
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2016 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-8168373: don't emit conversions for symbols outside their lexical scope
+ *
+ * @test
+ * @run
+ * @option --language=es6
+ */
+
+function p() { return false } // "predicate"
+function r(x) { return x } // "read"
+
+(function() {
+ try { // Try creates control flow edges from assignments into catch blocks.
+ // Lexically scoped, never read int variable (undefined at catch block) but still with a cf edge into catch block.
+ // Since it's never read, it's not written either (Nashorn optimizes some dead writes).
+ let x = 0;
+ if (p()) { throw {}; } // We need `p()` so this block doesn't get optimized away, for possibility of a `throw`
+ x = 0.0; // change the type of x to double
+ r(x); // read x otherwise it's optimized away
+ } catch (e) {} // under the bug, "throw" will try to widen unwritten int x to double for here and cause a verifier error
+})()