8166902: Nested object literal property maps not reset in optimistic recompilation
authorhannesw
Fri, 30 Sep 2016 19:40:31 +0200
changeset 41238 a3c1a712c67e
parent 41237 0c17b3fdc043
child 41239 e9ab9c841255
8166902: Nested object literal property maps not reset in optimistic recompilation Reviewed-by: lagergren, attila
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/SpillObjectCreator.java
nashorn/test/script/basic/JDK-8166902.js
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java	Thu Sep 29 16:45:12 2016 +0000
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java	Fri Sep 30 19:40:31 2016 +0200
@@ -2486,11 +2486,6 @@
                     }
 
                     @Override
-                    public boolean enterObjectNode(final ObjectNode objectNode) {
-                        return false;
-                    }
-
-                    @Override
                     public boolean enterDefault(final Node node) {
                         if (contains) {
                             return false;
@@ -2562,7 +2557,8 @@
             oc = new FieldObjectCreator<Expression>(this, tuples) {
                 @Override
                 protected void loadValue(final Expression node, final Type type) {
-                    loadExpressionAsType(node, type);
+                    // Use generic type in order to avoid conversion between object types
+                    loadExpressionAsType(node, Type.generic(type));
                 }};
         }
 
@@ -2578,10 +2574,7 @@
         //handler
         if (restOfProperty) {
             final ContinuationInfo ci = getContinuationInfo();
-            // Can be set at most once for a single rest-of method
-            assert ci.getObjectLiteralMap() == null;
-            ci.setObjectLiteralMap(oc.getMap());
-            ci.setObjectLiteralStackDepth(method.getStackSize());
+            ci.setObjectLiteralMap(method.getStackSize(), oc.getMap());
         }
 
         method.dup();
@@ -5309,10 +5302,8 @@
         private Type[] stackTypes;
         // If non-null, this node should perform the requisite type conversion
         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 necessarily top if property is a tree
-        private int objectLiteralStackDepth = -1;
+        // If we are in the middle of an object literal initialization, we need to update the property maps
+        private Map<Integer, PropertyMap> objectLiteralMaps;
         // The line number at the continuation point
         private int lineNumber;
         // The active catch label, in case the continuation point is in a try/catch block
@@ -5364,20 +5355,15 @@
             this.returnValueType = returnValueType;
         }
 
-        int getObjectLiteralStackDepth() {
-            return objectLiteralStackDepth;
-        }
-
-        void setObjectLiteralStackDepth(final int objectLiteralStackDepth) {
-            this.objectLiteralStackDepth = objectLiteralStackDepth;
-        }
-
-        PropertyMap getObjectLiteralMap() {
-            return objectLiteralMap;
-        }
-
-        void setObjectLiteralMap(final PropertyMap objectLiteralMap) {
-            this.objectLiteralMap = objectLiteralMap;
+        void setObjectLiteralMap(final int objectLiteralStackDepth, final PropertyMap objectLiteralMap) {
+            if (objectLiteralMaps == null) {
+                objectLiteralMaps = new HashMap<>();
+            }
+            objectLiteralMaps.put(objectLiteralStackDepth, objectLiteralMap);
+        }
+
+        PropertyMap getObjectLiteralMap(final int stackDepth) {
+            return objectLiteralMaps == null ? null : objectLiteralMaps.get(stackDepth);
         }
 
         @Override
@@ -5467,10 +5453,9 @@
         final int[]   stackStoreSpec = ci.getStackStoreSpec();
         final Type[]  stackTypes     = ci.getStackTypes();
         final boolean isStackEmpty   = stackStoreSpec.length == 0;
-        boolean replacedObjectLiteralMap = false;
+        int replacedObjectLiteralMaps = 0;
         if(!isStackEmpty) {
             // 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.get(slot), slot);
@@ -5478,18 +5463,18 @@
                 // stack: s0=object literal being initialized
                 // change map of s0 so that the property we are initializing when we failed
                 // is now ci.returnValueType
-                if (i == objectLiteralStackDepth) {
+                final PropertyMap map = ci.getObjectLiteralMap(i);
+                if (map != null) {
                     method.dup();
-                    assert ci.getObjectLiteralMap() != null;
                     assert ScriptObject.class.isAssignableFrom(method.peekType().getTypeClass()) : method.peekType().getTypeClass() + " is not a script object";
-                    loadConstant(ci.getObjectLiteralMap());
+                    loadConstant(map);
                     method.invoke(ScriptObject.SET_MAP);
-                    replacedObjectLiteralMap = true;
+                    replacedObjectLiteralMaps++;
                 }
             }
         }
-        // Must have emitted the code for replacing the map of an object literal if we have a set object literal stack depth
-        assert ci.getObjectLiteralStackDepth() == -1 || replacedObjectLiteralMap;
+        // Must have emitted the code for replacing all object literal maps
+        assert ci.objectLiteralMaps == null || ci.objectLiteralMaps.size() == replacedObjectLiteralMaps;
         // Load RewriteException back.
         method.load(rewriteExceptionType, lvarCount);
         // Get rid of the stored reference
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/SpillObjectCreator.java	Thu Sep 29 16:45:12 2016 +0000
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/SpillObjectCreator.java	Fri Sep 30 19:40:31 2016 +0200
@@ -189,7 +189,8 @@
 
     @Override
     protected void loadValue(final Expression expr, final Type type) {
-        codegen.loadExpressionAsType(expr, type);
+        // Use generic type in order to avoid conversion between object types
+        codegen.loadExpressionAsType(expr, Type.generic(type));
     }
 
     @Override
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8166902.js	Fri Sep 30 19:40:31 2016 +0200
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2016, 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-8166902: Nested object literal property maps not reset in optimistic recompilation
+ *
+ * @test
+ * @run
+ */
+
+var o = {
+    a: "A",
+    b: "B"
+};
+
+var m = {
+    x: { z: o.a },
+    y: o.b
+};
+
+Assert.assertEquals(m.x.z, "A");
+Assert.assertEquals(m.y, "B");
+