8047078: Fuzzing bug discovered when ArrayLiteralNodes weren't immutable
authorlagergren
Thu, 19 Jun 2014 10:46:31 +0200
changeset 25234 e2f9df6b8797
parent 24996 a84a3cc48bd5
child 25235 5e800a7fd352
8047078: Fuzzing bug discovered when ArrayLiteralNodes weren't immutable Reviewed-by: attila, sundar
nashorn/src/jdk/nashorn/internal/codegen/CompilationPhase.java
nashorn/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
nashorn/src/jdk/nashorn/internal/codegen/Splitter.java
nashorn/src/jdk/nashorn/internal/codegen/WeighNodes.java
nashorn/src/jdk/nashorn/internal/ir/LiteralNode.java
nashorn/test/script/basic/JDK-8047057.js
nashorn/test/script/basic/JDK-8047078.js
--- a/nashorn/src/jdk/nashorn/internal/codegen/CompilationPhase.java	Wed Jun 18 10:54:57 2014 -0700
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CompilationPhase.java	Thu Jun 19 10:46:31 2014 +0200
@@ -173,7 +173,18 @@
         @Override
         FunctionNode transform(final Compiler compiler, final CompilationPhases phases, final FunctionNode fn) {
             final CompileUnit  outermostCompileUnit = compiler.addCompileUnit(0L);
-            final FunctionNode newFunctionNode      = new Splitter(compiler, fn, outermostCompileUnit).split(fn, true);
+
+            FunctionNode newFunctionNode;
+
+            //ensure elementTypes, postsets and presets exist for splitter and arraynodes
+            newFunctionNode = (FunctionNode)fn.accept(new NodeVisitor<LexicalContext>(new LexicalContext()) {
+                @Override
+                public LiteralNode<?> leaveLiteralNode(final LiteralNode<?> literalNode) {
+                    return literalNode.initialize(lc);
+                }
+            });
+
+            newFunctionNode = new Splitter(compiler, newFunctionNode, outermostCompileUnit).split(newFunctionNode, true);
 
             assert newFunctionNode.getCompileUnit() == outermostCompileUnit : "fn=" + fn.getName() + ", fn.compileUnit (" + newFunctionNode.getCompileUnit() + ") != " + outermostCompileUnit;
             assert newFunctionNode.isStrict() == compiler.isStrict() : "functionNode.isStrict() != compiler.isStrict() for " + quote(newFunctionNode.getName());
@@ -374,7 +385,7 @@
                             assert newUnit != null;
                             newArrayUnits.add(new ArrayUnit(newUnit, au.getLo(), au.getHi()));
                         }
-                        aln.setUnits(newArrayUnits);
+                        return aln.setUnits(lc, newArrayUnits);
                     }
                     return node;
                 }
--- a/nashorn/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java	Wed Jun 18 10:54:57 2014 -0700
+++ b/nashorn/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java	Thu Jun 19 10:46:31 2014 +0200
@@ -39,6 +39,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
+
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.ir.AccessNode;
 import jdk.nashorn.internal.ir.BaseNode;
@@ -63,7 +64,6 @@
 import jdk.nashorn.internal.ir.LexicalContext;
 import jdk.nashorn.internal.ir.LexicalContextNode;
 import jdk.nashorn.internal.ir.LiteralNode;
-import jdk.nashorn.internal.ir.LiteralNode.ArrayLiteralNode;
 import jdk.nashorn.internal.ir.LocalVariableConversion;
 import jdk.nashorn.internal.ir.LoopNode;
 import jdk.nashorn.internal.ir.Node;
@@ -1207,10 +1207,10 @@
 
             @Override
             public Node leaveLiteralNode(final LiteralNode<?> literalNode) {
-                if(literalNode instanceof ArrayLiteralNode) {
-                    ((ArrayLiteralNode)literalNode).analyze();
-                }
-                return literalNode;
+                //for e.g. ArrayLiteralNodes the initial types may have been narrowed due to the
+                //introduction of optimistic behavior - hence ensure that all literal nodes are
+                //reinitialized
+                return literalNode.initialize(lc);
             }
 
             @Override
--- a/nashorn/src/jdk/nashorn/internal/codegen/Splitter.java	Wed Jun 18 10:54:57 2014 -0700
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Splitter.java	Thu Jun 19 10:46:31 2014 +0200
@@ -307,7 +307,7 @@
                 units.add(new ArrayUnit(unit, lo, postsets.length));
             }
 
-            arrayLiteralNode.setUnits(units);
+            return arrayLiteralNode.setUnits(lc, units);
         }
 
         return literal;
--- a/nashorn/src/jdk/nashorn/internal/codegen/WeighNodes.java	Wed Jun 18 10:54:57 2014 -0700
+++ b/nashorn/src/jdk/nashorn/internal/codegen/WeighNodes.java	Thu Jun 19 10:46:31 2014 +0200
@@ -173,7 +173,6 @@
         if (functionNode == topFunction) {
             // the function being weighted; descend into its statements
             return true;
-//            functionNode.visitStatements(this);
         }
         // just a reference to inner function from outer function
         weight += FUNC_EXPR_WEIGHT;
--- a/nashorn/src/jdk/nashorn/internal/ir/LiteralNode.java	Wed Jun 18 10:54:57 2014 -0700
+++ b/nashorn/src/jdk/nashorn/internal/ir/LiteralNode.java	Thu Jun 19 10:46:31 2014 +0200
@@ -29,6 +29,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.function.Function;
+
 import jdk.nashorn.internal.codegen.CompileUnit;
 import jdk.nashorn.internal.codegen.types.ArrayType;
 import jdk.nashorn.internal.codegen.types.Type;
@@ -87,6 +88,17 @@
     }
 
     /**
+     * Initialization setter, if required for immutable state. This is used for
+     * things like ArrayLiteralNodes that need to carry state for the splitter.
+     * Default implementation is just a nop.
+     * @param lc lexical context
+     * @return new literal node with initialized state, or same if nothing changed
+     */
+    public LiteralNode<?> initialize(final LexicalContext lc) {
+        return this;
+    }
+
+    /**
      * Check if the literal value is null
      * @return true if literal value is null
      */
@@ -573,24 +585,26 @@
     /**
      * Array literal node class.
      */
+    @Immutable
     public static final class ArrayLiteralNode extends LiteralNode<Expression[]> implements LexicalContextNode {
 
         /** Array element type. */
-        private Type elementType;
+        private final Type elementType;
 
         /** Preset constant array. */
-        private Object presets;
+        private final Object presets;
 
         /** Indices of array elements requiring computed post sets. */
-        private int[] postsets;
+        private final int[] postsets;
 
-        private List<ArrayUnit> units;
+        /** Sub units with indexes ranges, in which to split up code generation, for large literals */
+        private final List<ArrayUnit> units;
 
         /**
          * An ArrayUnit is a range in an ArrayLiteral. ArrayLiterals can
          * be split if they are too large, for bytecode generation reasons
          */
-        public static class ArrayUnit {
+        public static final class ArrayUnit {
             /** Compile unit associated with the postsets range. */
             private final CompileUnit compileUnit;
 
@@ -634,6 +648,150 @@
             }
         }
 
+        private static final class ArrayLiteralInitializer {
+
+            static ArrayLiteralNode initialize(final ArrayLiteralNode node) {
+                final Type elementType = computeElementType(node.value, node.elementType);
+                final int[] postsets = computePostsets(node.value);
+                final Object presets = computePresets(node.value, elementType, postsets);
+                return new ArrayLiteralNode(node, node.value, elementType, postsets, presets, node.units);
+            }
+
+            private static Type computeElementType(final Expression[] value, final Type elementType) {
+                Type widestElementType = Type.INT;
+
+                for (final Expression elem : value) {
+                    if (elem == null) {
+                        widestElementType = widestElementType.widest(Type.OBJECT); //no way to represent undefined as number
+                        break;
+                    }
+
+                    final Type type = elem.getType().isUnknown() ? Type.OBJECT : elem.getType();
+                    if (type.isBoolean()) {
+                        //TODO fix this with explicit boolean types
+                        widestElementType = widestElementType.widest(Type.OBJECT);
+                        break;
+                    }
+
+                    widestElementType = widestElementType.widest(type);
+                    if (widestElementType.isObject()) {
+                        break;
+                    }
+                }
+                return widestElementType;
+            }
+
+            private static int[] computePostsets(final Expression[] value) {
+                final int[] computed = new int[value.length];
+                int nComputed = 0;
+
+                for (int i = 0; i < value.length; i++) {
+                    final Expression element = value[i];
+                    if (element == null || objectAsConstant(element) == POSTSET_MARKER) {
+                        computed[nComputed++] = i;
+                    }
+                }
+                return Arrays.copyOf(computed, nComputed);
+            }
+
+            private static boolean setArrayElement(final int[] array, final int i, final Object n) {
+                if (n instanceof Number) {
+                    array[i] = ((Number)n).intValue();
+                    return true;
+                }
+                return false;
+            }
+
+            private static boolean setArrayElement(final long[] array, final int i, final Object n) {
+                if (n instanceof Number) {
+                    array[i] = ((Number)n).longValue();
+                    return true;
+                }
+                return false;
+            }
+
+            private static boolean setArrayElement(final double[] array, final int i, final Object n) {
+                if (n instanceof Number) {
+                    array[i] = ((Number)n).doubleValue();
+                    return true;
+                }
+                return false;
+            }
+
+            private static int[] presetIntArray(final Expression[] value, final int[] postsets) {
+                final int[] array = new int[value.length];
+                int nComputed = 0;
+                for (int i = 0; i < value.length; i++) {
+                    if (!setArrayElement(array, i, objectAsConstant(value[i]))) {
+                        assert postsets[nComputed++] == i;
+                    }
+                }
+                assert postsets.length == nComputed;
+                return array;
+            }
+
+            private static long[] presetLongArray(final Expression[] value, final int[] postsets) {
+                final long[] array = new long[value.length];
+                int nComputed = 0;
+                for (int i = 0; i < value.length; i++) {
+                    if (!setArrayElement(array, i, objectAsConstant(value[i]))) {
+                        assert postsets[nComputed++] == i;
+                    }
+                }
+                assert postsets.length == nComputed;
+                return array;
+            }
+
+            private static double[] presetDoubleArray(final Expression[] value, final int[] postsets) {
+                final double[] array = new double[value.length];
+                int nComputed = 0;
+                for (int i = 0; i < value.length; i++) {
+                    if (!setArrayElement(array, i, objectAsConstant(value[i]))) {
+                        assert postsets[nComputed++] == i;
+                    }
+                }
+                assert postsets.length == nComputed;
+                return array;
+            }
+
+            private static Object[] presetObjectArray(final Expression[] value, final int[] postsets) {
+                final Object[] array = new Object[value.length];
+                int nComputed = 0;
+
+                for (int i = 0; i < value.length; i++) {
+                    final Node node = value[i];
+
+                    if (node == null) {
+                        assert postsets[nComputed++] == i;
+                        continue;
+                    }
+                    final Object element = objectAsConstant(node);
+
+                    if (element != POSTSET_MARKER) {
+                        array[i] = element;
+                    } else {
+                        assert postsets[nComputed++] == i;
+                    }
+                }
+
+                assert postsets.length == nComputed;
+                return array;
+            }
+
+            static Object computePresets(final Expression[] value, final Type elementType, final int[] postsets) {
+                assert !elementType.isUnknown();
+                if (elementType.isInteger()) {
+                    return presetIntArray(value, postsets);
+                } else if (elementType.isLong()) {
+                    return presetLongArray(value, postsets);
+                } else if (elementType.isNumeric()) {
+                    return presetDoubleArray(value, postsets);
+                } else {
+                    return presetObjectArray(value, postsets);
+                }
+            }
+        }
+
         /**
          * Constructor
          *
@@ -644,136 +802,21 @@
         protected ArrayLiteralNode(final long token, final int finish, final Expression[] value) {
             super(Token.recast(token, TokenType.ARRAY), finish, value);
             this.elementType = Type.UNKNOWN;
+            this.presets     = null;
+            this.postsets    = null;
+            this.units       = null;
         }
 
         /**
          * Copy constructor
          * @param node source array literal node
          */
-        private ArrayLiteralNode(final ArrayLiteralNode node, final Expression[] value) {
+        private ArrayLiteralNode(final ArrayLiteralNode node, final Expression[] value, final Type elementType, final int[] postsets, final Object presets, final List<ArrayUnit> units) {
             super(node, value);
-            this.elementType = node.elementType;
-            this.presets     = node.presets;
-            this.postsets    = node.postsets;
-            this.units       = node.units;
-        }
-
-        /**
-         * Compute things like widest element type needed. Internal use from compiler only
-         */
-        public void analyze() {
-            assert elementType.isUnknown();
-            elementType = getNarrowestElementType(value);
-        }
-
-        private int[] presetIntArray() {
-            final int[] array = new int[value.length];
-            int nComputed = 0;
-
-            for (int i = 0; i < value.length; i++) {
-                final Object element = objectAsConstant(value[i]);
-
-                if (element instanceof Number) {
-                    array[i] = ((Number)element).intValue();
-                } else {
-                    assert getPostsets()[nComputed++] == i;
-                }
-            }
-
-            assert getPostsets().length == nComputed;
-            return array;
-        }
-
-        private long[] presetLongArray() {
-            final long[] array = new long[value.length];
-            int nComputed = 0;
-
-            for (int i = 0; i < value.length; i++) {
-                final Object element = objectAsConstant(value[i]);
-
-                if (element instanceof Number) {
-                    array[i] = ((Number)element).longValue();
-                } else {
-                    assert getPostsets()[nComputed++] == i;
-                }
-            }
-
-            assert getPostsets().length == nComputed;
-            return array;
-        }
-
-        private double[] presetNumberArray() {
-            final double[] array = new double[value.length];
-            int nComputed = 0;
-
-            for (int i = 0; i < value.length; i++) {
-                final Object element = objectAsConstant(value[i]);
-
-                if (element instanceof Number) {
-                    array[i] = ((Number)element).doubleValue();
-                } else {
-                    assert getPostsets()[nComputed++] == i;
-                }
-            }
-
-            assert getPostsets().length == nComputed;
-            return array;
-        }
-
-        private Object[] presetObjectArray() {
-            final Object[] array = new Object[value.length];
-            int nComputed = 0;
-
-            for (int i = 0; i < value.length; i++) {
-                final Node node = value[i];
-
-                if (node == null) {
-                    assert getPostsets()[nComputed++] == i;
-                } else {
-                    final Object element = objectAsConstant(node);
-
-                    if (element != POSTSET_MARKER) {
-                        array[i] = element;
-                    } else {
-                        assert getPostsets()[nComputed++] == i;
-                    }
-                }
-            }
-
-            assert getPostsets().length == nComputed;
-            return array;
-        }
-
-        /**
-         * Returns the narrowest element type that is wide enough to represent all the expressions in the array.
-         * @param elementExpressions the array of expressions
-         * @return the narrowest element type that is wide enough to represent all the expressions in the array.
-         */
-        private static Type getNarrowestElementType(final Expression[] elementExpressions) {
-            Type widestElementType = Type.INT;
-            for (final Expression element : elementExpressions) {
-                if (element == null) {
-                    widestElementType = widestElementType.widest(Type.OBJECT); //no way to represent undefined as number
-                    break;
-                }
-
-                Type elementType = element.getType();
-                if (elementType.isUnknown()) {
-                    elementType = Type.OBJECT;
-                }
-
-                if (elementType.isBoolean()) {
-                    widestElementType = widestElementType.widest(Type.OBJECT);
-                    break;
-                }
-
-                widestElementType = widestElementType.widest(elementType);
-
-                if (widestElementType.isObject()) {
-                    break;
-                }
-            }
-            return widestElementType;
+            this.elementType = elementType;
+            this.postsets    = postsets;
+            this.presets     = presets;
+            this.units       = units;
         }
 
         @Override
@@ -782,6 +825,19 @@
         }
 
         /**
+         * Setter that initializes all code generation meta data for an
+         * ArrayLiteralNode. This acts a setter, so the return value may
+         * return a new node and must be handled
+         *
+         * @param lc lexical context
+         * @return new array literal node with postsets, presets and element types initialized
+         */
+        @Override
+        public ArrayLiteralNode initialize(final LexicalContext lc) {
+            return Node.replaceInLexicalContext(lc, this, ArrayLiteralInitializer.initialize(this));
+        }
+
+        /**
          * Get the array element type as Java format, e.g. [I
          * @return array element type
          */
@@ -811,7 +867,7 @@
          * @return element type
          */
         public Type getElementType() {
-            assert !elementType.isUnknown();
+            assert !elementType.isUnknown() : this + " has elementType=unknown";
             return elementType;
         }
 
@@ -821,19 +877,20 @@
          * @return post set indices
          */
         public int[] getPostsets() {
-            if(postsets == null) {
-                final int[] computed = new int[value.length];
-                int nComputed = 0;
+            assert postsets != null : this + " elementType=" + elementType + " has no postsets";
+            return postsets;
+        }
 
-                for (int i = 0; i < value.length; i++) {
-                    final Expression element = value[i];
-                    if(element == null || objectAsConstant(element) == POSTSET_MARKER) {
-                        computed[nComputed++] = i;
-                    }
-                }
-                postsets = Arrays.copyOf(computed, nComputed);
+        private boolean presetsMatchElementType() {
+            if (elementType == Type.INT) {
+                return presets instanceof int[];
+            } else if (elementType == Type.LONG) {
+                return presets instanceof long[];
+            } else if (elementType == Type.NUMBER) {
+                return presets instanceof double[];
+            } else {
+                return presets instanceof Object[];
             }
-            return postsets;
         }
 
         /**
@@ -841,18 +898,7 @@
          * @return presets array, always returns an array type
          */
         public Object getPresets() {
-            if(presets == null) {
-                final Type type = getElementType();
-                if (type.isInteger()) {
-                    presets = presetIntArray();
-                } else if (type.isLong()) {
-                    presets = presetLongArray();
-                } else if (type.isNumeric()) {
-                    presets = presetNumberArray();
-                } else {
-                    presets = presetObjectArray();
-                }
-            }
+            assert presets != null && presetsMatchElementType() : this + " doesn't have presets, or invalid preset type: " + presets;
             return presets;
         }
 
@@ -867,11 +913,16 @@
 
         /**
          * Set the ArrayUnits that make up this ArrayLiteral
+         * @param lc lexical context
          * @see ArrayUnit
          * @param units list of array units
+         * @return new or changed arrayliteralnode
          */
-        public void setUnits(final List<ArrayUnit> units) {
-            this.units = units;
+        public ArrayLiteralNode setUnits(final LexicalContext lc, final List<ArrayUnit> units) {
+            if (this.units == units) {
+                return this;
+            }
+            return Node.replaceInLexicalContext(lc, this, new ArrayLiteralNode(this, value, elementType, postsets, presets, units));
         }
 
         @Override
@@ -889,8 +940,15 @@
             return this;
         }
 
+        private ArrayLiteralNode setValue(final LexicalContext lc, final Expression[] value) {
+            if (this.value == value) {
+                return this;
+            }
+            return Node.replaceInLexicalContext(lc, this, new ArrayLiteralNode(this, value, elementType, postsets, presets, units));
+        }
+
         private ArrayLiteralNode setValue(final LexicalContext lc, final List<Expression> value) {
-            return (ArrayLiteralNode)lc.replace(this, new ArrayLiteralNode(this, value.toArray(new Expression[value.size()])));
+            return setValue(lc, value.toArray(new Expression[value.size()]));
         }
 
         @Override
--- a/nashorn/test/script/basic/JDK-8047057.js	Wed Jun 18 10:54:57 2014 -0700
+++ b/nashorn/test/script/basic/JDK-8047057.js	Thu Jun 19 10:46:31 2014 +0200
@@ -29,8 +29,7 @@
  */
 
 // commented out makeFuncAndCall calls are still result in crash
-// Tests commented with //** fail only within test framework.
-// Pass fine with standalone "jjs" mode.
+// Tests commented with //** fail only when assertions are turned on
 
 function makeFuncAndCall(code) {
     Function(code)();
@@ -52,19 +51,19 @@
 makeFuncExpectError("L: {while(0) break L; return [](); }", TypeError);
 // makeFuncAndCall("do with({}) break ; while(0);");
 makeFuncAndCall("while(0) with({}) continue ;");
-//** makeFuncAndCall("eval([]);");
-//** makeFuncAndCall("try{} finally{[]}");
+makeFuncAndCall("eval([]);");
+makeFuncAndCall("try{} finally{[]}");
 makeFuncAndCall("try { } catch(x if 1) { try { } catch(x2) { } }");
 makeFuncAndCall("try { } catch(x if 1) { try { return; } catch(x2) { { } } }");
 makeFuncAndCall("Error() * (false)[-0]--");
 makeFuncAndCall("try { var x = 1, x = null; } finally { }");
 makeFuncAndCall("try { var x = {}, x = []; } catch(x3) { }");
-//** makeFuncAndCall("[delete this]");
+makeFuncAndCall("[delete this]");
 // makeFuncAndCall("if(eval('', eval('', function() {}))) { }");
 // makeFuncAndCall("if(eval('', eval('', function() {}))) { }");
 // makeFuncAndCall("eval(\"[,,];\", [11,12,13,14].some)");
 // makeFuncAndCall("eval(\"1.2e3\", ({})[ /x/ ])");
-// makeFuncAndCall("eval(\"x4\", x3);");
+makeFuncExpectError("eval(\"x4\", x3);", ReferenceError);
 makeFuncAndCall("with({5.0000000000000000000000: String()}){(false); }");
 makeFuncAndCall("try { var x = undefined, x = 5.0000000000000000000000; } catch(x) { x = undefined; }");
 makeFuncAndCall("(function (x){ x %= this}(false))");
@@ -73,3 +72,4 @@
 makeFuncAndCall("with({8: 'fafafa'.replace()}){ }");
 makeFuncAndCall("(function (x) '' )(true)");
 makeFuncExpectError("new eval(function(){})", TypeError);
+//** makeFuncAndCall('eval("23", ({})[/x/])');
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8047078.js	Thu Jun 19 10:46:31 2014 +0200
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2014, 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-8047078: ArrayLiteral mutability caused trouble in optimistic types
+ *
+ * @test
+ * @run
+ */
+
+function makeFuncAndCall(code) {
+    Function(code)();
+}
+
+makeFuncAndCall("eval([]);");
+makeFuncAndCall("eval([1]);");
+makeFuncAndCall("eval([1,2,3,,4]);");
+makeFuncAndCall("try{} finally{[]}");