8193508: Expressions in split literals must never be optimistic
authorhannesw
Wed, 20 Dec 2017 21:40:11 +0100
changeset 48380 597f69e5f1e3
parent 48379 5382baab8371
child 48404 177e1783d886
8193508: Expressions in split literals must never be optimistic Reviewed-by: jlaskey, sundar
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/OptimisticTypesCalculator.java
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Splitter.java
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/GetSplitState.java
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/LiteralNode.java
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/ObjectNode.java
test/nashorn/script/basic/JDK-8193508.js
--- 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');
+