8041995: Problems when loading tree expressions with several optimistic program points when optimistically initializing ObjectNodes

Reviewed-by: jlaskey, attila

nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java

@@ -75,6 +75,7 @@ import java.util.Map;
 import java.util.RandomAccess;
 import java.util.Set;
 import java.util.TreeMap;
+import java.util.function.Supplier;
 
 import jdk.nashorn.internal.codegen.ClassEmitter.Flag;
 import jdk.nashorn.internal.codegen.CompilerConstants.Call;
@@ -1898,6 +1899,37 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
         return false;
     }
 
+    /**
+     * Check if a property value contains a particular program point
+     * @param value value
+     * @param pp    program point
+     * @return true if it's there.
+     */
+    private static boolean propertyValueContains(final Expression value, final int pp) {
+        return new Supplier<Boolean>() {
+            boolean contains;
+
+            @Override
+            public Boolean get() {
+                value.accept(new NodeVisitor<LexicalContext>(new LexicalContext()) {
+                    @Override
+                    public boolean enterDefault(final Node node) {
+                        if (contains) {
+                            return false;
+                        }
+                        if (node instanceof Optimistic && ((Optimistic)node).getProgramPoint() == pp) {
+                            contains = true;
+                            return false;
+                        }
+                        return true;
+                    }
+                });
+
+                return contains;
+            }
+        }.get();
+    }
+
     @Override
     public boolean enterObjectNode(final ObjectNode objectNode) {
         final List<PropertyNode> elements = objectNode.getElements();
@@ -1926,7 +1958,7 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
                 value != null &&
                 isValid(ccp) &&
                 value instanceof Optimistic &&
-                ((Optimistic)value).getProgramPoint() == ccp;
+                propertyValueContains(value, ccp);
 
             //for literals, a value of null means object type, i.e. the value null or getter setter function
             //(I think)
@@ -1949,11 +1981,14 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
                 }};
         }
         oc.makeObject(method);
+
         //if this is a rest of method and our continuation point was found as one of the values
         //in the properties above, we need to reset the map to oc.getMap() in the continuation
         //handler
         if (restOfProperty) {
-            getContinuationInfo().setObjectLiteralMap(oc.getMap());
+            final ContinuationInfo ci = getContinuationInfo();
+            ci.setObjectLiteralMap(oc.getMap());
+            ci.setObjectLiteralStackDepth(method.getStackSize());
         }
 
         method.dup();
@@ -4460,6 +4495,8 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
         private Type returnValueType;
         // If we are in the middle of an object literal initialization, we need to update the map
         private PropertyMap objectLiteralMap;
+        // Object literal stack depth for object literal - not necessarly top if property is a tree
+        private int objectLiteralStackDepth = -1;
 
         ContinuationInfo() {
             this.handlerLabel = new Label("continuation_handler");
@@ -4513,6 +4550,14 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
             this.returnValueType = returnValueType;
         }
 
+        int getObjectLiteralStackDepth() {
+            return objectLiteralStackDepth;
+        }
+
+        void setObjectLiteralStackDepth(final int objectLiteralStackDepth) {
+            this.objectLiteralStackDepth = objectLiteralStackDepth;
+        }
+
         PropertyMap getObjectLiteralMap() {
             return objectLiteralMap;
         }
@@ -4573,20 +4618,21 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
             // Store the RewriteException into an unused local variable slot.
             method.store(rewriteExceptionType, lvarCount);
             // Load arguments on the stack
+            final int objectLiteralStackDepth = ci.getObjectLiteralStackDepth();
             for(int i = 0; i < stackStoreSpec.length; ++i) {
                 final int slot = stackStoreSpec[i];
                 method.load(lvarTypes[slot], slot);
                 method.convert(stackTypes[i]);
-            }
-
-            // stack: s0=object literal being initialized
-            // change map of s0 so that the property we are initilizing when we failed
-            // is now ci.returnValueType
-            if (ci.getObjectLiteralMap() != null) {
-                method.dup(); //dup script object
-                assert ScriptObject.class.isAssignableFrom(method.peekType().getTypeClass()) : method.peekType().getTypeClass() + " is not a script object";
-                loadConstant(ci.getObjectLiteralMap());
-                method.invoke(ScriptObject.SET_MAP);
+                // stack: s0=object literal being initialized
+                // change map of s0 so that the property we are initilizing when we failed
+                // is now ci.returnValueType
+                if (i == objectLiteralStackDepth) {
+                    method.dup();
+                    assert ci.getObjectLiteralMap() != null;
+                    assert ScriptObject.class.isAssignableFrom(method.peekType().getTypeClass()) : method.peekType().getTypeClass() + " is not a script object";
+                    loadConstant(ci.getObjectLiteralMap());
+                    method.invoke(ScriptObject.SET_MAP);
+                }
             }
 
             // Load RewriteException back; get rid of the stored reference.

nashorn/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java

@@ -352,13 +352,6 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
         return (isProgram ? program : extractFunctionFromScript(program)).setName(null, functionName).setSourceURL(null,  sourceURL);
     }
 
-    private static String getShortDescriptor(final Object value) {
-        if (value == null || !value.getClass().isPrimitive() || value.getClass() != Boolean.class) {
-            return "O";
-        }
-        return value.getClass().getSimpleName();
-    }
-
     private static String stringifyInvalidations(final Map<Integer, Type> ipp) {
         if (ipp == null) {
             return "";
@@ -367,10 +360,11 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
         final Iterator<Map.Entry<Integer, Type>> iter = ipp.entrySet().iterator();
         while (iter.hasNext()) {
             final Map.Entry<Integer, Type> entry = iter.next();
+            final char bct = entry.getValue().getBytecodeStackType();
             sb.append('[').
                     append(entry.getKey()).
                     append("->").
-                    append(getShortDescriptor(entry.getValue())).
+                    append(bct == 'A' ? 'O' : bct).
                     append(']');
             if (iter.hasNext()) {
                 sb.append(' ');

nashorn/test/script/basic/JDK-8041995.js

@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2010, 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-8041995: optimistic object property maps were only updated if the outermost program
+ * point in a property setter failed, not an inner one, which is wrong.
+ *
+ * @test
+ * @run
+ */
+
+function xyzzy() {
+    return 17.4711;
+}
+var obj = { 
+    z: -xyzzy()
+};
+print(obj.z);
+
+function phlug() {
+    var obj = { 
+	4: -Infinity,
+ 	5: Infinity,
+	length: 5 - Math.pow(2, 32)
+    };
+
+    return Array.prototype.lastIndexOf.call(obj, -Infinity) === 4;
+}
+
+var d = new Date;
+print(phlug());
+var d2 = new Date - d;
+print(d2 < 5000); // if this takes more than five seconds we have read the double length as an int
+
+function wrong() {
+    var obj = {
+	length1: 5 - Math.pow(2, 32),
+	length2: 4 - Math.pow(2, 32),
+	length3: 3 - Math.pow(2, 32),
+	length4: 2 - Math.pow(2, 32),
+	length5: 1 - Math.pow(2, 32),
+	length6: Math.pow(2, 32)
+    };
+    for (var i in obj) {
+       print(obj[i]);
+    }
+}
+
+wrong();

nashorn/test/script/basic/JDK-8041995.js.EXPECTED

@@ -0,0 +1,9 @@
+-17.4711
+true
+true
+-4294967291
+-4294967292
+-4294967293
+-4294967294
+-4294967295
+4294967296