# HG changeset patch # User hannesw # Date 1513802411 -3600 # Node ID 597f69e5f1e36d49e0b0d8761d6f12ab5eef2da9 # Parent 5382baab83711a84934662242eee11444057b31f 8193508: Expressions in split literals must never be optimistic Reviewed-by: jlaskey, sundar diff -r 5382baab8371 -r 597f69e5f1e3 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java --- 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() { diff -r 5382baab8371 -r 597f69e5f1e3 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java --- 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>> unwarrantedOptimismHandlers = new ArrayDeque<>(); private final Deque 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) { diff -r 5382baab8371 -r 597f69e5f1e3 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/OptimisticTypesCalculator.java --- 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); diff -r 5382baab8371 -r 597f69e5f1e3 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Splitter.java --- 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); } diff -r 5382baab8371 -r 597f69e5f1e3 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/GetSplitState.java --- 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() { diff -r 5382baab8371 -r 597f69e5f1e3 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/LiteralNode.java --- 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 splitRanges; /** Does this array literal have a spread element? */ diff -r 5382baab8371 -r 597f69e5f1e3 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/ObjectNode.java --- 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 elements; /** Ranges for splitting large literals over multiple compile units in codegen. */ + @Ignore private final List splitRanges; /** diff -r 5382baab8371 -r 597f69e5f1e3 test/nashorn/script/basic/JDK-8193508.js --- /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'); +