8041625: AccessorProperty currentType must only by Object.class when non-primitive, and scoping followup problem for lazily generated with bodies

Reviewed-by: jlaskey, attila
This commit is contained in:
Marcus Lagergren 2014-05-02 18:22:29 +02:00
parent e9e18d5614
commit 2f01820f6d
8 changed files with 161 additions and 36 deletions

View File

@ -409,7 +409,7 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
}
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

View File

@ -31,6 +31,7 @@ import java.util.Collections;
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 @@ final class CodeGeneratorLexicalContext extends LexicalContext {
/** 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 @@ final class CodeGeneratorLexicalContext extends LexicalContext {
@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 @@ final class CodeGeneratorLexicalContext extends LexicalContext {
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 @@ final class CodeGeneratorLexicalContext extends LexicalContext {
return nextFreeSlots[nextFreeSlotsSize - 1];
}
void releaseBlockSlots(boolean optimistic) {
void releaseBlockSlots(final boolean optimistic) {
--nextFreeSlotsSize;
if(optimistic) {
slotTypesDescriptors.peek().setLength(nextFreeSlots[nextFreeSlotsSize]);

View File

@ -27,6 +27,7 @@ package jdk.nashorn.internal.codegen;
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 java.util.Set;
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 @@ final class FindScopeDepths extends NodeVisitor<LexicalContext> implements Logga
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 @@ final class FindScopeDepths extends NodeVisitor<LexicalContext> implements Logga
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 @@ final class FindScopeDepths extends NodeVisitor<LexicalContext> implements Logga
//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 @@ final class FindScopeDepths extends NodeVisitor<LexicalContext> implements Logga
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 @@ final class FindScopeDepths extends NodeVisitor<LexicalContext> implements Logga
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 @@ final class FindScopeDepths extends NodeVisitor<LexicalContext> implements Logga
}
depths.put(symbol.getName(), depthAtStart);
}
}

View File

@ -209,7 +209,10 @@ public final class FunctionNode extends LexicalContextExpression implements Flag
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;
@ -648,6 +651,36 @@ public final class FunctionNode extends LexicalContextExpression implements Flag
return needsArguments() || parameters.size() > LinkerCallSite.ARGLIMIT;
}
/**
* 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

View File

@ -631,7 +631,7 @@ public class AccessorProperty extends Property {
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 @@ public class AccessorProperty extends Property {
@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

View File

@ -269,6 +269,11 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
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 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
if (functionNode.isVarArg()) {
flags |= IS_VARIABLE_ARITY;
}
if (functionNode.inDynamicContext()) {
flags |= IN_DYNAMIC_CONTEXT;
}
return flags;
}
@ -346,6 +354,7 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
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 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
return sb.toString();
}
FunctionNodeTransform getNopTransform() {
private FunctionNodeTransform getNopTransform() {
return new FunctionNodeTransform() {
@Override
FunctionNode apply(final FunctionNode functionNode) {

View File

@ -85,6 +85,8 @@ public abstract class ScriptFunctionData {
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 @@ public abstract class ScriptFunctionData {
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) {

View File

@ -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], [