8008458: Strict functions dont share property map

Reviewed-by: sundar, hannesw
Contributed-by: james.laskey@oracle.com

nashorn/src/jdk/nashorn/internal/objects/NativeStrictArguments.java

@@ -57,8 +57,10 @@ public final class NativeStrictArguments extends ScriptObject {
         PropertyMap map = PropertyMap.newMap(NativeStrictArguments.class);
         map = Lookup.newProperty(map, "length", Property.NOT_ENUMERABLE, G$LENGTH, S$LENGTH);
         // In strict mode, the caller and callee properties should throw TypeError
-        map = ScriptFunctionImpl.newThrowerProperty(map, "caller");
-        map = ScriptFunctionImpl.newThrowerProperty(map, "callee");
+        // Need to add properties directly to map since slots are assigned speculatively by newUserAccessors.
+        final int flags = Property.NOT_ENUMERABLE | Property.NOT_CONFIGURABLE;
+        map = map.addProperty(map.newUserAccessors("caller", flags));
+        map = map.addProperty(map.newUserAccessors("callee", flags));
         nasgenmap$ = map;
     }
 

nashorn/src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java

@@ -155,8 +155,12 @@ public class ScriptFunctionImpl extends ScriptFunction {
                 Lookup.TYPE_ERROR_THROWER_GETTER, Lookup.TYPE_ERROR_THROWER_SETTER);
     }
 
-    private static PropertyMap createStrictModeMap(final PropertyMap functionMap) {
-        return newThrowerProperty(newThrowerProperty(functionMap, "arguments"), "caller");
+    private static PropertyMap createStrictModeMap(PropertyMap map) {
+        final int flags = Property.NOT_ENUMERABLE | Property.NOT_CONFIGURABLE;
+        // Need to add properties directly to map since slots are assigned speculatively by newUserAccessors.
+        map = map.addProperty(map.newUserAccessors("arguments", flags));
+        map = map.addProperty(map.newUserAccessors("caller", flags));
+        return map;
     }
 
     // Choose the map based on strict mode!
@@ -260,12 +264,15 @@ public class ScriptFunctionImpl extends ScriptFunction {
         this.setProto(Global.instance().getFunctionPrototype());
         this.prototype = LAZY_PROTOTYPE;
 
-        if (isStrict()) {
-            final ScriptFunction func = getTypeErrorThrower();
-            // We have to fill user accessor functions late as these are stored
-            // in this object rather than in the PropertyMap of this object.
-            setUserAccessors("arguments", func, func);
-            setUserAccessors("caller", func, func);
+        // We have to fill user accessor functions late as these are stored
+        // in this object rather than in the PropertyMap of this object.
+
+        if (findProperty("arguments", true) != null) {
+            setUserAccessors("arguments", getTypeErrorThrower(), getTypeErrorThrower());
+        }
+
+        if (findProperty("caller", true) != null) {
+            setUserAccessors("caller", getTypeErrorThrower(), getTypeErrorThrower());
         }
     }
 }

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

@@ -89,7 +89,7 @@ public final class FindProperty {
         MethodHandle setter = property.getSetter(type, getOwner().getMap());
         if (property instanceof UserAccessorProperty) {
             final UserAccessorProperty uc = (UserAccessorProperty) property;
-            setter = MH.insertArguments(setter, 0, (isInherited() ? getOwner() : null),
+            setter = MH.insertArguments(setter, 0, isInherited() ? getOwner() : null,
                     uc.getSetterSlot(), strict? property.getKey() : null);
         }
 
@@ -109,7 +109,7 @@ public final class FindProperty {
      * @return appropriate receiver
      */
     public ScriptObject getGetterReceiver() {
-        return property != null && property.hasGetterFunction() ? self : prototype;
+        return property != null && property.hasGetterFunction(prototype) ? self : prototype;
     }
 
    /**
@@ -117,7 +117,7 @@ public final class FindProperty {
      * @return appropriate receiver
      */
     public ScriptObject getSetterReceiver() {
-        return property != null && property.hasSetterFunction() ? self : prototype;
+        return property != null && property.hasSetterFunction(prototype) ? self : prototype;
     }
 
     /**

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

@@ -180,17 +180,19 @@ public abstract class Property {
 
     /**
      * Check whether this property has a user defined getter function. See {@link UserAccessorProperty}
+     * @param obj object containing getter
      * @return true if getter function exists, false is default
      */
-    public boolean hasGetterFunction() {
+    public boolean hasGetterFunction(final ScriptObject obj) {
         return false;
     }
 
     /**
      * Check whether this property has a user defined setter function. See {@link UserAccessorProperty}
+     * @param obj object containing setter
      * @return true if getter function exists, false is default
      */
-    public boolean hasSetterFunction() {
+    public boolean hasSetterFunction(final ScriptObject obj) {
         return false;
     }
 

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

@@ -302,7 +302,7 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
      *
      * @return New {@link PropertyMap} with {@link Property} added.
      */
-    PropertyMap addProperty(final Property property) {
+    public PropertyMap addProperty(final Property property) {
         PropertyMap newMap = checkHistory(property);
 
         if (newMap == null) {
@@ -383,6 +383,21 @@ public final class PropertyMap implements Iterable<Object>, PropertyListener {
         return newMap;
     }
 
+    /*
+     * Make a new UserAccessorProperty property. getter and setter functions are stored in
+     * this ScriptObject and slot values are used in property object. Note that slots
+     * are assigned speculatively and should be added to map before adding other
+     * properties.
+     */
+    public UserAccessorProperty newUserAccessors(final String key, final int propertyFlags) {
+        int oldSpillLength = spillLength;
+
+        final int getterSlot = oldSpillLength++;
+        final int setterSlot = oldSpillLength++;
+
+        return new UserAccessorProperty(key, propertyFlags, getterSlot, setterSlot);
+    }
+
     /**
      * Find a property in the map.
      *

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

@@ -777,30 +777,18 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
     public final Property modifyOwnProperty(final Property oldProperty, final int propertyFlags, final ScriptFunction getter, final ScriptFunction setter) {
         Property newProperty;
         if (oldProperty instanceof UserAccessorProperty) {
-            // re-use the slots of the old user accessor property.
             final UserAccessorProperty uc = (UserAccessorProperty) oldProperty;
-
-            int getterSlot = uc.getGetterSlot();
-            // clear the old getter and set the new getter
+            final int getterSlot = uc.getGetterSlot();
+            final int setterSlot = uc.getSetterSlot();
             setSpill(getterSlot, getter);
-            // if getter function is null, flag the slot to be negative (less by 1)
-            if (getter == null) {
-                getterSlot = -getterSlot - 1;
-            }
-
-            int setterSlot = uc.getSetterSlot();
-            // clear the old setter and set the new setter
             setSpill(setterSlot, setter);
-            // if setter function is null, flag the slot to be negative (less by 1)
-            if (setter == null) {
-                setterSlot = -setterSlot - 1;
-            }
 
-            newProperty = new UserAccessorProperty(oldProperty.getKey(), propertyFlags, getterSlot, setterSlot);
             // if just flipping getter and setter with new functions, no need to change property or map
-            if (oldProperty.equals(newProperty)) {
+            if (uc.flags == propertyFlags) {
                 return oldProperty;
             }
+
+            newProperty = new UserAccessorProperty(oldProperty.getKey(), propertyFlags, getterSlot, setterSlot);
         } else {
             // erase old property value and create new user accessor property
             erasePropertyValue(oldProperty);
@@ -862,12 +850,12 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
      */
     public final void setUserAccessors(final String key, final ScriptFunction getter, final ScriptFunction setter) {
         final Property oldProperty = getMap().findProperty(key);
-        if (oldProperty != null) {
-            final UserAccessorProperty newProperty = newUserAccessors(oldProperty.getKey(), oldProperty.getFlags(), getter, setter);
-            modifyOwnProperty(oldProperty, newProperty);
+        if (oldProperty instanceof UserAccessorProperty) {
+            final UserAccessorProperty ua = (UserAccessorProperty)oldProperty;
+            setSpill(ua.getGetterSlot(), getter);
+            setSpill(ua.getSetterSlot(), setter);
         } else {
-            final UserAccessorProperty newProperty = newUserAccessors(key, 0, getter, setter);
-            addOwnProperty(newProperty);
+            addOwnProperty(newUserAccessors(key, oldProperty != null ? oldProperty.getFlags() : 0, getter, setter));
         }
     }
 
@@ -1712,7 +1700,7 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
 
             final ScriptObject prototype = find.getOwner();
 
-            if (!property.hasGetterFunction()) {
+            if (!property.hasGetterFunction(prototype)) {
                 methodHandle = bindTo(methodHandle, prototype);
             }
             return new GuardedInvocation(methodHandle, getMap().getProtoGetSwitchPoint(proto, name), guard);
@@ -3144,49 +3132,30 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
      * Make a new UserAccessorProperty property. getter and setter functions are stored in
      * this ScriptObject and slot values are used in property object.
      */
-    private UserAccessorProperty newUserAccessors(final String key, final int propertyFlags, final ScriptFunction getter, final ScriptFunction setter) {
-        int oldSpillLength = getMap().getSpillLength();
-
-        int getterSlot = oldSpillLength++;
-        setSpill(getterSlot, getter);
-        // if getter function is null, flag the slot to be negative (less by 1)
-        if (getter == null) {
-            getterSlot = -getterSlot - 1;
-        }
-
-        int setterSlot = oldSpillLength++;
+    protected final UserAccessorProperty newUserAccessors(final String key, final int propertyFlags, final ScriptFunction getter, final ScriptFunction setter) {
+        final UserAccessorProperty property = getMap().newUserAccessors(key, propertyFlags);
+        setSpill(property.getGetterSlot(), getter);
+        setSpill(property.getSetterSlot(), setter);
 
-        setSpill(setterSlot, setter);
-        // if setter function is null, flag the slot to be negative (less by 1)
-        if (setter == null) {
-            setterSlot = -setterSlot - 1;
-        }
-
-        return new UserAccessorProperty(key, propertyFlags, getterSlot, setterSlot);
+        return property;
     }
 
-    private void setSpill(final int slot, final Object value) {
-        if (slot >= 0) {
-            final int index = slot;
-            if (spill == null) {
-                // create new spill.
-                spill = new Object[Math.max(index + 1, SPILL_RATE)];
-            } else if (index >= spill.length) {
-                // grow spill as needed
-                final Object[] newSpill = new Object[index + 1];
-                System.arraycopy(spill, 0, newSpill, 0, spill.length);
-                spill = newSpill;
-            }
-
-            spill[index] = value;
+    protected final void setSpill(final int slot, final Object value) {
+        if (spill == null) {
+            // create new spill.
+            spill = new Object[Math.max(slot + 1, SPILL_RATE)];
+        } else if (slot >= spill.length) {
+            // grow spill as needed
+            final Object[] newSpill = new Object[slot + 1];
+            System.arraycopy(spill, 0, newSpill, 0, spill.length);
+            spill = newSpill;
         }
+
+        spill[slot] = value;
     }
 
-    // user accessors are either stored in spill array slots
-    // get the accessor value using slot number. Note that slot is spill array index.
-    Object getSpill(final int slot) {
-        final int index = slot;
-        return (index < 0 || (index >= spill.length)) ? null : spill[index];
+    protected Object getSpill(final int slot) {
+        return spill != null && slot < spill.length ? spill[slot] : null;
     }
 
     private static MethodHandle findOwnMH(final String name, final Class<?> rtype, final Class<?>... types) {

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

@@ -151,9 +151,10 @@ final class SetMethodCreator {
         assert methodHandle != null;
         assert property     != null;
 
+        final ScriptObject prototype = find.getOwner();
         final MethodHandle boundHandle;
-        if (!property.hasSetterFunction() && find.isInherited()) {
-            boundHandle = ScriptObject.bindTo(methodHandle, find.getOwner());
+        if (!property.hasSetterFunction(prototype) && find.isInherited()) {
+            boundHandle = ScriptObject.bindTo(methodHandle, prototype);
         } else {
             boundHandle = methodHandle;
         }

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

@@ -96,19 +96,19 @@ public final class UserAccessorProperty extends Property {
     }
 
     /**
-     * Return getter slot for this UserAccessorProperty. Slots start with first embed field.
+     * Return getter spill slot for this UserAccessorProperty.
      * @return getter slot
      */
     public int getGetterSlot() {
-        return getterSlot < 0 ? -getterSlot - 1 : getterSlot;
+        return getterSlot;
     }
 
     /**
-     * Return setter slot for this UserAccessorProperty. Slots start with first embed field.
+     * Return setter spill slot for this UserAccessorProperty.
      * @return setter slot
      */
     public int getSetterSlot() {
-        return setterSlot < 0 ? -setterSlot - 1 : setterSlot;
+        return setterSlot;
     }
 
     @Override
@@ -124,7 +124,7 @@ public final class UserAccessorProperty extends Property {
 
         final UserAccessorProperty uc = (UserAccessorProperty) other;
         return getterSlot == uc.getterSlot && setterSlot == uc.setterSlot;
-     }
+    }
 
     @Override
     public int hashCode() {
@@ -136,25 +136,17 @@ public final class UserAccessorProperty extends Property {
      */
     @Override
     public int getSpillCount() {
-        // calculate how many spill array slots used by this propery.
-        int count = 0;
-        if (getGetterSlot() >= 0) {
-            count++;
-        }
-        if (getSetterSlot() >= 0) {
-            count++;
-        }
-        return count;
+        return 2;
     }
 
     @Override
-    public boolean hasGetterFunction() {
-        return getterSlot > -1;
+    public boolean hasGetterFunction(final ScriptObject obj) {
+        return obj.getSpill(getterSlot) != null;
     }
 
     @Override
-    public boolean hasSetterFunction() {
-        return setterSlot > -1;
+    public boolean hasSetterFunction(final ScriptObject obj) {
+        return obj.getSpill(setterSlot) != null;
     }
 
     @Override