8193508: Expressions in split literals must never be optimistic
Reviewed-by: jlaskey, sundar
--- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Dec 18 10:21:38 2017 +0000
+++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Wed Dec 20 21:40:11 2017 +0100
@@ -1279,7 +1279,7 @@
}
boolean useOptimisticTypes() {
- return !lc.inSplitNode() && compiler.useOptimisticTypes();
+ return !lc.inSplitLiteral() && compiler.useOptimisticTypes();
}
@Override
@@ -2917,12 +2917,12 @@
// to synthetic functions, and FunctionNode.needsCallee() will no longer need to test for isSplit().
final int literalSlot = fixScopeSlot(currentFunction, 3);
- lc.enterSplitNode();
+ lc.enterSplitLiteral();
creator.populateRange(method, literalType, literalSlot, splitRange.getLow(), splitRange.getHigh());
method._return();
- lc.exitSplitNode();
+ lc.exitSplitLiteral();
method.end();
lc.releaseSlots();
popMethodEmitter();
@@ -4654,10 +4654,12 @@
this.optimistic = optimistic;
this.expression = (Expression)optimistic;
this.resultBounds = resultBounds;
- this.isOptimistic = isOptimistic(optimistic) && useOptimisticTypes() &&
+ this.isOptimistic = isOptimistic(optimistic)
// Operation is only effectively optimistic if its type, after being coerced into the result bounds
// is narrower than the upper bound.
- resultBounds.within(Type.generic(((Expression)optimistic).getType())).narrowerThan(resultBounds.widest);
+ && resultBounds.within(Type.generic(((Expression)optimistic).getType())).narrowerThan(resultBounds.widest);
+ // Optimistic operations need to be executed in optimistic context, else unwarranted optimism will go unnoticed
+ assert !this.isOptimistic || useOptimisticTypes();
}
MethodEmitter emit() {
--- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java Mon Dec 18 10:21:38 2017 +0000
+++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java Wed Dec 20 21:40:11 2017 +0100
@@ -68,7 +68,7 @@
private final Deque<Map<String, Collection<Label>>> unwarrantedOptimismHandlers = new ArrayDeque<>();
private final Deque<StringBuilder> slotTypesDescriptors = new ArrayDeque<>();
- private final IntDeque splitNodes = new IntDeque();
+ private final IntDeque splitLiterals = new IntDeque();
/** A stack tracking the next free local variable slot in the blocks. There's one entry for every block
* currently on the lexical context stack. */
@@ -89,18 +89,18 @@
if (((FunctionNode)node).inDynamicContext()) {
dynamicScopeCount++;
}
- splitNodes.push(0);
+ splitLiterals.push(0);
}
return super.push(node);
}
- void enterSplitNode() {
- splitNodes.getAndIncrement();
+ void enterSplitLiteral() {
+ splitLiterals.getAndIncrement();
pushFreeSlots(methodEmitters.peek().getUsedSlotsWithLiveTemporaries());
}
- void exitSplitNode() {
- final int count = splitNodes.decrementAndGet();
+ void exitSplitLiteral() {
+ final int count = splitLiterals.decrementAndGet();
assert count >= 0;
}
@@ -115,8 +115,8 @@
dynamicScopeCount--;
assert dynamicScopeCount >= 0;
}
- assert splitNodes.peek() == 0;
- splitNodes.pop();
+ assert splitLiterals.peek() == 0;
+ splitLiterals.pop();
}
return popped;
}
@@ -125,8 +125,8 @@
return dynamicScopeCount > 0;
}
- boolean inSplitNode() {
- return !splitNodes.isEmpty() && splitNodes.peek() > 0;
+ boolean inSplitLiteral() {
+ return !splitLiterals.isEmpty() && splitLiterals.peek() > 0;
}
MethodEmitter pushMethodEmitter(final MethodEmitter newMethod) {
--- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/OptimisticTypesCalculator.java Mon Dec 18 10:21:38 2017 +0000
+++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/OptimisticTypesCalculator.java Wed Dec 20 21:40:11 2017 +0100
@@ -42,8 +42,10 @@
import jdk.nashorn.internal.ir.IfNode;
import jdk.nashorn.internal.ir.IndexNode;
import jdk.nashorn.internal.ir.JoinPredecessorExpression;
+import jdk.nashorn.internal.ir.LiteralNode;
import jdk.nashorn.internal.ir.LoopNode;
import jdk.nashorn.internal.ir.Node;
+import jdk.nashorn.internal.ir.ObjectNode;
import jdk.nashorn.internal.ir.Optimistic;
import jdk.nashorn.internal.ir.PropertyNode;
import jdk.nashorn.internal.ir.Symbol;
@@ -190,6 +192,23 @@
}
@Override
+ public boolean enterObjectNode(ObjectNode objectNode) {
+ if (objectNode.getSplitRanges() != null) {
+ return false;
+ }
+ return super.enterObjectNode(objectNode);
+ }
+
+ @Override
+ public boolean enterLiteralNode(LiteralNode<?> literalNode) {
+ if (literalNode.isArray() && ((LiteralNode.ArrayLiteralNode) literalNode).getSplitRanges() != null) {
+ return false;
+ }
+
+ return super.enterLiteralNode(literalNode);
+ }
+
+ @Override
public boolean enterWhileNode(final WhileNode whileNode) {
// Test is never optimistic (always coerced to boolean).
tagNeverOptimisticLoopTest(whileNode);
--- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Splitter.java Mon Dec 18 10:21:38 2017 +0000
+++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Splitter.java Wed Dec 20 21:40:11 2017 +0100
@@ -112,7 +112,7 @@
assert lc.isEmpty() : "LexicalContext not empty";
if (weight >= SPLIT_THRESHOLD) {
- log.info("Splitting '", functionNode.getName(), "' as its weight ", weight, " exceeds split threshold ", SPLIT_THRESHOLD);
+ log.info("Splitting function '", functionNode.getName(), "' as its weight ", weight, " exceeds split threshold ", SPLIT_THRESHOLD);
functionNode = (FunctionNode)functionNode.accept(this);
if (functionNode.isSplit()) {
@@ -287,7 +287,7 @@
@SuppressWarnings("rawtypes")
@Override
public Node leaveLiteralNode(final LiteralNode literal) {
- long weight = WeighNodes.weigh(literal);
+ final long weight = WeighNodes.weigh(literal);
if (weight < SPLIT_THRESHOLD) {
return literal;
@@ -310,14 +310,14 @@
final int postset = postsets[i];
final Node element = value[postset];
- weight = WeighNodes.weigh(element);
- totalWeight += WeighNodes.AASTORE_WEIGHT + weight;
+ final long elementWeight = WeighNodes.weigh(element);
+ totalWeight += WeighNodes.AASTORE_WEIGHT + elementWeight;
if (totalWeight >= SPLIT_THRESHOLD) {
- final CompileUnit unit = compiler.findUnit(totalWeight - weight);
+ final CompileUnit unit = compiler.findUnit(totalWeight - elementWeight);
ranges.add(new Splittable.SplitRange(unit, lo, i));
lo = i;
- totalWeight = weight;
+ totalWeight = elementWeight;
}
}
@@ -326,6 +326,8 @@
ranges.add(new Splittable.SplitRange(unit, lo, postsets.length));
}
+ log.info("Splitting array literal in '", functionNode.getName(), "' as its weight ", weight, " exceeds split threshold ", SPLIT_THRESHOLD);
+
return arrayLiteralNode.setSplitRanges(lc, ranges);
}
@@ -334,7 +336,7 @@
@Override
public Node leaveObjectNode(final ObjectNode objectNode) {
- long weight = WeighNodes.weigh(objectNode);
+ final long weight = WeighNodes.weigh(objectNode);
if (weight < SPLIT_THRESHOLD) {
return objectNode;
@@ -355,14 +357,14 @@
final boolean isConstant = LiteralNode.isConstant(property.getValue());
if (!isConstant || !isSpillObject) {
- weight = isConstant ? 0 : WeighNodes.weigh(property.getValue());
- totalWeight += WeighNodes.AASTORE_WEIGHT + weight;
+ final long propertyWeight = isConstant ? 0 : WeighNodes.weigh(property.getValue());
+ totalWeight += WeighNodes.AASTORE_WEIGHT + propertyWeight;
if (totalWeight >= SPLIT_THRESHOLD) {
- final CompileUnit unit = compiler.findUnit(totalWeight - weight);
+ final CompileUnit unit = compiler.findUnit(totalWeight - propertyWeight);
ranges.add(new Splittable.SplitRange(unit, lo, i));
lo = i;
- totalWeight = weight;
+ totalWeight = propertyWeight;
}
}
}
@@ -372,6 +374,8 @@
ranges.add(new Splittable.SplitRange(unit, lo, properties.size()));
}
+ log.info("Splitting object node in '", functionNode.getName(), "' as its weight ", weight, " exceeds split threshold ", SPLIT_THRESHOLD);
+
return objectNode.setSplitRanges(lc, ranges);
}
--- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/GetSplitState.java Mon Dec 18 10:21:38 2017 +0000
+++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/GetSplitState.java Wed Dec 20 21:40:11 2017 +0100
@@ -27,6 +27,7 @@
import jdk.nashorn.internal.codegen.CompilerConstants;
import jdk.nashorn.internal.codegen.types.Type;
+import jdk.nashorn.internal.ir.annotations.Ignore;
import jdk.nashorn.internal.ir.visitor.NodeVisitor;
import jdk.nashorn.internal.runtime.Scope;
@@ -39,6 +40,7 @@
private static final long serialVersionUID = 1L;
/** The sole instance of this AST node. */
+ @Ignore
public final static GetSplitState INSTANCE = new GetSplitState();
private GetSplitState() {
--- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/LiteralNode.java Mon Dec 18 10:21:38 2017 +0000
+++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/LiteralNode.java Wed Dec 20 21:40:11 2017 +0100
@@ -30,6 +30,7 @@
import java.util.List;
import jdk.nashorn.internal.codegen.types.ArrayType;
import jdk.nashorn.internal.codegen.types.Type;
+import jdk.nashorn.internal.ir.annotations.Ignore;
import jdk.nashorn.internal.ir.annotations.Immutable;
import jdk.nashorn.internal.ir.visitor.NodeVisitor;
import jdk.nashorn.internal.objects.NativeArray;
@@ -633,6 +634,7 @@
private final int[] postsets;
/** Ranges for splitting up large literals in code generation */
+ @Ignore
private final List<Splittable.SplitRange> splitRanges;
/** Does this array literal have a spread element? */
--- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/ObjectNode.java Mon Dec 18 10:21:38 2017 +0000
+++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/ObjectNode.java Wed Dec 20 21:40:11 2017 +0100
@@ -29,6 +29,7 @@
import java.util.List;
import java.util.RandomAccess;
import jdk.nashorn.internal.codegen.types.Type;
+import jdk.nashorn.internal.ir.annotations.Ignore;
import jdk.nashorn.internal.ir.annotations.Immutable;
import jdk.nashorn.internal.ir.visitor.NodeVisitor;
@@ -43,6 +44,7 @@
private final List<PropertyNode> elements;
/** Ranges for splitting large literals over multiple compile units in codegen. */
+ @Ignore
private final List<Splittable.SplitRange> splitRanges;
/**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nashorn/script/basic/JDK-8193508.js Wed Dec 20 21:40:11 2017 +0100
@@ -0,0 +1,93 @@
+/*
+ * Copyright (c) 2017, 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-8193508: Expressions in split literals must never be optimistic
+ *
+ * @test
+ * @run
+ * @option -Dnashorn.compiler.splitter.threshold=100
+ * @fork
+ */
+
+function f() {
+ return 'a';
+}
+
+var o = {
+ a: f(),
+ b: 1,
+ c: 2,
+ d: 3,
+ e: 4,
+ f: 5,
+ g: f(),
+ h: 1,
+ i: 2,
+ j: 3,
+ k: 4,
+ l: 5,
+ m: f(),
+ n: 1,
+ o: 2,
+ p: 3,
+ q: 4,
+ r: 5,
+ s: f(),
+ t: 1,
+ u: 2,
+ v: 3,
+ w: 4,
+ x: 5,
+ y: f(),
+ z: 1,
+ A: 2,
+ B: 3,
+ C: 4,
+ D: 5,
+ E: f(),
+ F: 1,
+ G: 2,
+ H: 3,
+ I: 4,
+ J: 5,
+ K: f(),
+ L: 1,
+ M: 2,
+ N: 3,
+ O: 4,
+ P: 5,
+ Q: f(),
+ R: 1,
+ S: 2,
+ T: 3,
+ U: 4,
+ V: 5,
+ W: f(),
+ X: 1,
+ Y: 2,
+ Z: 3
+};
+
+Assert.assertTrue(o.a === 'a');
+