8041625: AccessorProperty currentType must only by Object.class when non-primitive, and scoping followup problem for lazily generated with bodies
Reviewed-by: jlaskey, attila
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Tue Apr 29 16:00:53 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri May 02 18:22:29 2014 +0200
@@ -409,7 +409,7 @@
}
previousWasBlock = true;
} else {
- if (node instanceof WithNode && previousWasBlock || node instanceof FunctionNode && CodeGeneratorLexicalContext.isFunctionDynamicScope((FunctionNode)node)) {
+ if (node instanceof WithNode && previousWasBlock || node instanceof FunctionNode && ((FunctionNode)node).needsDynamicScope()) {
// If we hit a scope that can have symbols introduced into it at run time before finding the defining
// block, the symbol can't be fast scoped. A WithNode only counts if we've immediately seen a block
// before - its block. Otherwise, we are currently processing the WithNode's expression, and that's
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java Tue Apr 29 16:00:53 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java Fri May 02 18:22:29 2014 +0200
@@ -31,6 +31,7 @@
import java.util.Deque;
import java.util.HashMap;
import java.util.Map;
+
import jdk.nashorn.internal.IntDeque;
import jdk.nashorn.internal.codegen.types.Type;
import jdk.nashorn.internal.ir.Block;
@@ -75,14 +76,20 @@
/** size of next free slot vector */
private int nextFreeSlotsSize;
+ private boolean isWithBoundary(final LexicalContextNode node) {
+ return node instanceof Block && !isEmpty() && peek() instanceof WithNode;
+ }
+
@Override
public <T extends LexicalContextNode> T push(final T node) {
- if (isDynamicScopeBoundary(node)) {
- ++dynamicScopeCount;
- }
- if(node instanceof FunctionNode) {
+ if (isWithBoundary(node)) {
+ dynamicScopeCount++;
+ } else if (node instanceof FunctionNode) {
+ if (((FunctionNode)node).inDynamicContext()) {
+ dynamicScopeCount++;
+ }
splitNodes.push(0);
- } else if(node instanceof SplitNode) {
+ } else if (node instanceof SplitNode) {
enterSplitNode();
}
return super.push(node);
@@ -99,32 +106,22 @@
@Override
public <T extends LexicalContextNode> T pop(final T node) {
final T popped = super.pop(node);
- if (isDynamicScopeBoundary(popped)) {
- --dynamicScopeCount;
- }
- if(node instanceof FunctionNode) {
+ if (isWithBoundary(node)) {
+ dynamicScopeCount--;
+ assert dynamicScopeCount >= 0;
+ } else if (node instanceof FunctionNode) {
+ if (((FunctionNode)node).inDynamicContext()) {
+ dynamicScopeCount--;
+ assert dynamicScopeCount >= 0;
+ }
assert splitNodes.peek() == 0;
splitNodes.pop();
- } else if(node instanceof SplitNode) {
+ } else if (node instanceof SplitNode) {
exitSplitNode();
}
return popped;
}
- private boolean isDynamicScopeBoundary(final LexicalContextNode node) {
- if (node instanceof Block) {
- // Block's immediate parent is a with node. Note we aren't testing for a WithNode, as that'd capture
- // processing of WithNode.expression too, but it should be unaffected.
- return !isEmpty() && peek() instanceof WithNode;
- } else if (node instanceof FunctionNode) {
- // Function has a direct eval in it (so a top-level "var ..." in the eval code can introduce a new
- // variable into the function's scope), and it isn't strict (as evals in strict functions get an
- // isolated scope).
- return isFunctionDynamicScope((FunctionNode)node);
- }
- return false;
- }
-
boolean inDynamicScope() {
return dynamicScopeCount > 0;
}
@@ -133,10 +130,6 @@
return !splitNodes.isEmpty() && splitNodes.peek() > 0;
}
- static boolean isFunctionDynamicScope(FunctionNode fn) {
- return fn.hasEval() && !fn.isStrict();
- }
-
MethodEmitter pushMethodEmitter(final MethodEmitter newMethod) {
methodEmitters.push(newMethod);
return newMethod;
@@ -230,7 +223,7 @@
return nextFreeSlots[nextFreeSlotsSize - 1];
}
- void releaseBlockSlots(boolean optimistic) {
+ void releaseBlockSlots(final boolean optimistic) {
--nextFreeSlotsSize;
if(optimistic) {
slotTypesDescriptors.peek().setLength(nextFreeSlots[nextFreeSlotsSize]);
--- a/nashorn/src/jdk/nashorn/internal/codegen/FindScopeDepths.java Tue Apr 29 16:00:53 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/FindScopeDepths.java Fri May 02 18:22:29 2014 +0200
@@ -27,6 +27,7 @@
import static jdk.nashorn.internal.codegen.ObjectClassGenerator.getClassName;
import static jdk.nashorn.internal.codegen.ObjectClassGenerator.getPaddedFieldCount;
+import static jdk.nashorn.internal.runtime.logging.DebugLogger.quote;
import java.util.HashMap;
import java.util.HashSet;
@@ -37,6 +38,7 @@
import jdk.nashorn.internal.ir.Block;
import jdk.nashorn.internal.ir.Expression;
import jdk.nashorn.internal.ir.FunctionNode;
+import jdk.nashorn.internal.ir.WithNode;
import jdk.nashorn.internal.ir.FunctionNode.CompilationState;
import jdk.nashorn.internal.ir.LexicalContext;
import jdk.nashorn.internal.ir.Node;
@@ -63,9 +65,12 @@
private final Map<Integer, Map<Integer, RecompilableScriptFunctionData>> fnIdToNestedFunctions = new HashMap<>();
private final Map<Integer, Map<String, Integer>> externalSymbolDepths = new HashMap<>();
private final Map<Integer, Set<String>> internalSymbols = new HashMap<>();
+ private final Set<Block> withBodies = new HashSet<>();
private final DebugLogger log;
+ private int dynamicScopeCount;
+
FindScopeDepths(final Compiler compiler) {
super(new LexicalContext());
this.compiler = compiler;
@@ -151,12 +156,24 @@
return globalBlock;
}
+ private static boolean isDynamicScopeBoundary(final FunctionNode fn) {
+ return fn.needsDynamicScope();
+ }
+
+ private boolean isDynamicScopeBoundary(final Block block) {
+ return withBodies.contains(block);
+ }
+
@Override
public boolean enterFunctionNode(final FunctionNode functionNode) {
if (env.isOnDemandCompilation()) {
return true;
}
+ if (isDynamicScopeBoundary(functionNode)) {
+ increaseDynamicScopeCount(functionNode);
+ }
+
final int fnId = functionNode.getId();
Map<Integer, RecompilableScriptFunctionData> nestedFunctions = fnIdToNestedFunctions.get(fnId);
if (nestedFunctions == null) {
@@ -170,13 +187,24 @@
//external symbols hold the scope depth of sc11 from global at the start of the method
@Override
public Node leaveFunctionNode(final FunctionNode functionNode) {
- final FunctionNode newFunctionNode = functionNode.setState(lc, CompilationState.SCOPE_DEPTHS_COMPUTED);
+ final String name = functionNode.getName();
+ FunctionNode newFunctionNode = functionNode.setState(lc, CompilationState.SCOPE_DEPTHS_COMPUTED);
+
if (env.isOnDemandCompilation()) {
final RecompilableScriptFunctionData data = env.getScriptFunctionData(newFunctionNode.getId());
assert data != null : newFunctionNode.getName() + " lacks data";
+ if (data.inDynamicContext()) {
+ log.fine("Reviving scriptfunction ", quote(name), " as defined in previous (now lost) dynamic scope.");
+ newFunctionNode = newFunctionNode.setInDynamicContext(lc);
+ }
return newFunctionNode;
}
+ if (inDynamicScope()) {
+ log.fine("Tagging ", quote(name), " as defined in dynamic scope");
+ newFunctionNode = newFunctionNode.setInDynamicContext(lc);
+ }
+
//create recompilable scriptfunctiondata
final int fnId = newFunctionNode.getId();
final Map<Integer, RecompilableScriptFunctionData> nestedFunctions = fnIdToNestedFunctions.get(fnId);
@@ -207,15 +235,49 @@
env.setData(data);
}
+ if (isDynamicScopeBoundary(functionNode)) {
+ decreaseDynamicScopeCount(functionNode);
+ }
+
return newFunctionNode;
}
+ private boolean inDynamicScope() {
+ return dynamicScopeCount > 0;
+ }
+
+ private void increaseDynamicScopeCount(final Node node) {
+ assert dynamicScopeCount >= 0;
+ ++dynamicScopeCount;
+ if (log.isEnabled()) {
+ log.finest(quote(lc.getCurrentFunction().getName()), " ++dynamicScopeCount = ", dynamicScopeCount, " at: ", node, node.getClass());
+ }
+ }
+
+ private void decreaseDynamicScopeCount(final Node node) {
+ --dynamicScopeCount;
+ assert dynamicScopeCount >= 0;
+ if (log.isEnabled()) {
+ log.finest(quote(lc.getCurrentFunction().getName()), " --dynamicScopeCount = ", dynamicScopeCount, " at: ", node, node.getClass());
+ }
+ }
+
+ @Override
+ public boolean enterWithNode(final WithNode node) {
+ withBodies.add(node.getBody());
+ return true;
+ }
+
@Override
public boolean enterBlock(final Block block) {
if (env.isOnDemandCompilation()) {
return true;
}
+ if (isDynamicScopeBoundary(block)) {
+ increaseDynamicScopeCount(block);
+ }
+
if (!lc.isFunctionBody()) {
return true;
}
@@ -288,6 +350,17 @@
return true;
}
+ @Override
+ public Node leaveBlock(final Block block) {
+ if (env.isOnDemandCompilation()) {
+ return block;
+ }
+ if (isDynamicScopeBoundary(block)) {
+ decreaseDynamicScopeCount(block);
+ }
+ return block;
+ }
+
private void addInternalSymbols(final FunctionNode functionNode, final Set<String> symbols) {
final int fnId = functionNode.getId();
assert internalSymbols.get(fnId) == null || internalSymbols.get(fnId).equals(symbols); //e.g. cloned finally block
@@ -303,4 +376,5 @@
}
depths.put(symbol.getName(), depthAtStart);
}
+
}
--- a/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java Tue Apr 29 16:00:53 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java Fri May 02 18:22:29 2014 +0200
@@ -209,7 +209,10 @@
public static final int IS_PROGRAM = 1 << 15;
/** Does this function use the "this" keyword? */
- public static final int USES_THIS = 1 << 16;
+ public static final int USES_THIS = 1 << 16;
+
+ /** Is this declared in a dynamic context */
+ public static final int IN_DYNAMIC_CONTEXT = 1 << 17;
/** Does this function or any nested functions contain an eval? */
private static final int HAS_DEEP_EVAL = HAS_EVAL | HAS_NESTED_EVAL;
@@ -649,6 +652,36 @@
}
/**
+ * Was this function declared in a dynamic context, i.e. in a with or eval style
+ * chain
+ * @return true if in dynamic context
+ */
+ public boolean inDynamicContext() {
+ return getFlag(IN_DYNAMIC_CONTEXT);
+ }
+
+ /**
+ * Check whether a function would need dynamic scope, which is does if it has
+ * evals and isn't strict.
+ * @return true if dynamic scope is needed
+ */
+ public boolean needsDynamicScope() {
+ // Function has a direct eval in it (so a top-level "var ..." in the eval code can introduce a new
+ // variable into the function's scope), and it isn't strict (as evals in strict functions get an
+ // isolated scope).
+ return hasEval() && !isStrict();
+ }
+
+ /**
+ * Flag this function as declared in a dynamic context
+ * @param lc lexical context
+ * @return new function node, or same if unmodified
+ */
+ public FunctionNode setInDynamicContext(final LexicalContext lc) {
+ return setFlag(lc, IN_DYNAMIC_CONTEXT);
+ }
+
+ /**
* Returns true if this function needs to have an Arguments object defined as a local variable named "arguments".
* Functions that use "arguments" as identifier and don't define it as a name of a parameter or a nested function
* (see ECMAScript 5.1 Chapter 10.5), as well as any function that uses eval or with, or has a nested function that
--- a/nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java Tue Apr 29 16:00:53 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java Fri May 02 18:22:29 2014 +0200
@@ -631,7 +631,7 @@
mh = ObjectClassGenerator.createGuardBoxedPrimitiveSetter(ct, generateSetter(ct, ct), mh);
}
} else {
- mh = generateSetter(forType, type);
+ mh = generateSetter(!forType.isPrimitive() ? Object.class : forType, type);
}
/**
@@ -681,7 +681,7 @@
@Override
public final void setCurrentType(final Class<?> currentType) {
assert currentType != boolean.class : "no boolean storage support yet - fix this";
- this.currentType = currentType;
+ this.currentType = currentType == null ? null : currentType.isPrimitive() ? currentType : Object.class;
}
@Override
--- a/nashorn/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Tue Apr 29 16:00:53 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Fri May 02 18:22:29 2014 +0200
@@ -269,6 +269,11 @@
return functionName;
}
+ @Override
+ public boolean inDynamicContext() {
+ return (flags & IN_DYNAMIC_CONTEXT) != 0;
+ }
+
private static String functionName(final FunctionNode fn) {
if (fn.isAnonymous()) {
return "";
@@ -304,6 +309,9 @@
if (functionNode.isVarArg()) {
flags |= IS_VARIABLE_ARITY;
}
+ if (functionNode.inDynamicContext()) {
+ flags |= IN_DYNAMIC_CONTEXT;
+ }
return flags;
}
@@ -346,6 +354,7 @@
if (isAnonymous) {
parser.setFunctionName(functionName);
}
+
final FunctionNode program = parser.parse(scriptName, descPosition, Token.descLength(token), true);
// Parser generates a program AST even if we're recompiling a single function, so when we are only recompiling a
// single function, extract it from the program.
@@ -373,7 +382,7 @@
return sb.toString();
}
- FunctionNodeTransform getNopTransform() {
+ private FunctionNodeTransform getNopTransform() {
return new FunctionNodeTransform() {
@Override
FunctionNode apply(final FunctionNode functionNode) {
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunctionData.java Tue Apr 29 16:00:53 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunctionData.java Fri May 02 18:22:29 2014 +0200
@@ -85,6 +85,8 @@
public static final int USES_THIS = 1 << 4;
/** Is this a variable arity function? */
public static final int IS_VARIABLE_ARITY = 1 << 5;
+ /** Is this declared in a dynamic context */
+ public static final int IN_DYNAMIC_CONTEXT = 1 << 6;
/** Flag for strict or built-in functions */
public static final int IS_STRICT_OR_BUILTIN = IS_STRICT | IS_BUILTIN;
@@ -748,6 +750,14 @@
return type.parameterType(type.parameterCount() - 1).isArray();
}
+ /**
+ * Is this ScriptFunction declared in a dynamic context
+ * @return true if in dynamic context, false if not or irrelevant
+ */
+ public boolean inDynamicContext() {
+ return false;
+ }
+
@SuppressWarnings("unused")
private static Object[] bindVarArgs(final Object[] array1, final Object[] array2) {
if (array2 == null) {
--- a/nashorn/test/script/basic/run-octane.js Tue Apr 29 16:00:53 2014 +0200
+++ b/nashorn/test/script/basic/run-octane.js Fri May 02 18:22:29 2014 +0200
@@ -25,7 +25,13 @@
* @subtest
*/
-var read = readFully;
+var read;
+try {
+ read = readFully;
+} catch (e) {
+ print("ABORTING: Cannot find 'readFully'. You must have scripting enabled to use this test harness. (-scripting)");
+ throw e;
+}
function initZlib() {
zlib = new BenchmarkSuite('zlib', [152815148], [