stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8168373: don't emit conversions for symbols outside their lexical scope

Browse Source 
Reviewed-by: hannesw, sundar
This commit is contained in:
Attila Szegedi 2016-11-11 15:50:51 +01:00
parent 5f91733e00
commit 1406cd4347
3 changed files with 98 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
View File

@ -33,6 +33,7 @@ import java.util.ArrayDeque;
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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
}
@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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
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 class LocalVariableTypesCalculator extends SimpleNodeVisitor {
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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
// 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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
@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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
// 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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
}
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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
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;
}
localVariableTypes.remove(symbol);
}
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);
}
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 @@ final class LocalVariableTypesCalculator extends SimpleNodeVisitor {
}
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

2
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/debug/PrintVisitor.java
View File

@ -397,7 +397,7 @@ public final class PrintVisitor extends SimpleNodeVisitor {
@Override
public boolean enterVarNode(final VarNode varNode) {
sb.append("var ");
sb.append(varNode.isConst() ? "const " : varNode.isLet() ? "let " : "var ");
varNode.getName().toString(sb, printTypes);
printLocalVariableConversion(varNode.getName());
final Node init = varNode.getInit();

44
nashorn/test/script/basic/es6/JDK-8168373.js Normal file
View File

@ -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
})()