# HG changeset patch # User jlaskey # Date 1372246613 10800 # Node ID 682889823712c5daecee26b36d1e01c1b5e58742 # Parent 39e17373d8df884e5db527e76b71a0013e6dabdd 8008458: Strict functions dont share property map Reviewed-by: sundar, hannesw Contributed-by: james.laskey@oracle.com diff -r 39e17373d8df -r 682889823712 src/jdk/nashorn/internal/objects/NativeStrictArguments.java --- a/src/jdk/nashorn/internal/objects/NativeStrictArguments.java Wed Jun 26 16:36:13 2013 +0530 +++ b/src/jdk/nashorn/internal/objects/NativeStrictArguments.java Wed Jun 26 08:36:53 2013 -0300 @@ -57,8 +57,10 @@ 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; } diff -r 39e17373d8df -r 682889823712 src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java --- a/src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java Wed Jun 26 16:36:13 2013 +0530 +++ b/src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java Wed Jun 26 08:36:53 2013 -0300 @@ -155,8 +155,12 @@ 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 @@ 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()); } } } diff -r 39e17373d8df -r 682889823712 src/jdk/nashorn/internal/runtime/FindProperty.java --- a/src/jdk/nashorn/internal/runtime/FindProperty.java Wed Jun 26 16:36:13 2013 +0530 +++ b/src/jdk/nashorn/internal/runtime/FindProperty.java Wed Jun 26 08:36:53 2013 -0300 @@ -89,7 +89,7 @@ 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 @@ * @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 @@ * @return appropriate receiver */ public ScriptObject getSetterReceiver() { - return property != null && property.hasSetterFunction() ? self : prototype; + return property != null && property.hasSetterFunction(prototype) ? self : prototype; } /** diff -r 39e17373d8df -r 682889823712 src/jdk/nashorn/internal/runtime/Property.java --- a/src/jdk/nashorn/internal/runtime/Property.java Wed Jun 26 16:36:13 2013 +0530 +++ b/src/jdk/nashorn/internal/runtime/Property.java Wed Jun 26 08:36:53 2013 -0300 @@ -180,17 +180,19 @@ /** * 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; } diff -r 39e17373d8df -r 682889823712 src/jdk/nashorn/internal/runtime/PropertyMap.java --- a/src/jdk/nashorn/internal/runtime/PropertyMap.java Wed Jun 26 16:36:13 2013 +0530 +++ b/src/jdk/nashorn/internal/runtime/PropertyMap.java Wed Jun 26 08:36:53 2013 -0300 @@ -302,7 +302,7 @@ * * @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 @@ 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. * diff -r 39e17373d8df -r 682889823712 src/jdk/nashorn/internal/runtime/ScriptObject.java --- a/src/jdk/nashorn/internal/runtime/ScriptObject.java Wed Jun 26 16:36:13 2013 +0530 +++ b/src/jdk/nashorn/internal/runtime/ScriptObject.java Wed Jun 26 08:36:53 2013 -0300 @@ -777,30 +777,18 @@ 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 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 @@ 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 @@ * 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; + 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); + + return property; + } + + 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; } - int setterSlot = oldSpillLength++; - - 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); + spill[slot] = value; } - 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; - } - } - - // 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) { diff -r 39e17373d8df -r 682889823712 src/jdk/nashorn/internal/runtime/SetMethodCreator.java --- a/src/jdk/nashorn/internal/runtime/SetMethodCreator.java Wed Jun 26 16:36:13 2013 +0530 +++ b/src/jdk/nashorn/internal/runtime/SetMethodCreator.java Wed Jun 26 08:36:53 2013 -0300 @@ -151,9 +151,10 @@ 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; } diff -r 39e17373d8df -r 682889823712 src/jdk/nashorn/internal/runtime/UserAccessorProperty.java --- a/src/jdk/nashorn/internal/runtime/UserAccessorProperty.java Wed Jun 26 16:36:13 2013 +0530 +++ b/src/jdk/nashorn/internal/runtime/UserAccessorProperty.java Wed Jun 26 08:36:53 2013 -0300 @@ -96,19 +96,19 @@ } /** - * 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 @@ final UserAccessorProperty uc = (UserAccessorProperty) other; return getterSlot == uc.getterSlot && setterSlot == uc.setterSlot; - } + } @Override public int hashCode() { @@ -136,25 +136,17 @@ */ @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