Mercurial > hg > openjdk > aarch64-port > nashorn
changeset 497:03ba1cd734c0
8022731: NativeArguments has wrong implementation of isMapped()
Reviewed-by: lagergren, jlaskey
| author | hannesw |
|---|---|
| date | Mon, 12 Aug 2013 13:31:43 +0200 |
| parents | 0bbaa0ac36ab |
| children | 821b605c7046 |
| files | src/jdk/nashorn/internal/objects/NativeArguments.java test/script/basic/JDK-8022731.js test/script/basic/JDK-8022731.js.EXPECTED |
| diffstat | 3 files changed, 191 insertions(+), 374 deletions(-) [+] |
line wrap: on
line diff
--- a/src/jdk/nashorn/internal/objects/NativeArguments.java Mon Aug 12 16:52:32 2013 +0530 +++ b/src/jdk/nashorn/internal/objects/NativeArguments.java Mon Aug 12 13:31:43 2013 +0200 @@ -76,36 +76,21 @@ private Object length; private Object callee; - private ArrayData namedArgs; - // This is lazily initialized - only when delete is invoked at all + private final int numMapped; + private final int numParams; + + // These are lazily initialized when delete is invoked on a mapped arg or an unmapped argument is set. + private ArrayData unmappedArgs; private BitSet deleted; NativeArguments(final Object[] arguments, final Object callee, final int numParams, final ScriptObject proto, final PropertyMap map) { super(proto, map); setIsArguments(); - setArray(ArrayData.allocate(arguments)); this.length = arguments.length; this.callee = callee; - - /** - * Declared number of parameters may be more or less than the actual passed - * runtime arguments count. We need to truncate or extend with undefined values. - * - * Example: - * - * // less declared params - * (function (x) { print(arguments); })(20, 44); - * - * // more declared params - * (function (x, y) { print(arguments); })(3); - */ - final Object[] newValues = new Object[numParams]; - if (numParams > arguments.length) { - Arrays.fill(newValues, UNDEFINED); - } - System.arraycopy(arguments, 0, newValues, 0, Math.min(newValues.length, arguments.length)); - this.namedArgs = ArrayData.allocate(newValues); + this.numMapped = Math.min(numParams, arguments.length); + this.numParams = numParams; } @Override @@ -118,7 +103,8 @@ */ @Override public Object getArgument(final int key) { - return namedArgs.has(key) ? namedArgs.getObject(key) : UNDEFINED; + assert key >= 0 && key < numParams : "invalid argument index"; + return isMapped(key) ? getArray().getObject(key) : getUnmappedArg(key); } /** @@ -126,353 +112,36 @@ */ @Override public void setArgument(final int key, final Object value) { - if (namedArgs.has(key)) { - namedArgs = namedArgs.set(key, value, false); - } - } - - @Override - public int getInt(final Object key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getInt(index) : super.getInt(key); - } - - @Override - public int getInt(final double key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getInt(index) : super.getInt(key); - } - - @Override - public int getInt(final long key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getInt(index) : super.getInt(key); - } - - @Override - public int getInt(final int key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getInt(index) : super.getInt(key); - } - - @Override - public long getLong(final Object key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getLong(index) : super.getLong(key); - } - - @Override - public long getLong(final double key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getLong(index) : super.getLong(key); - } - - @Override - public long getLong(final long key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getLong(index) : super.getLong(key); - } - - @Override - public long getLong(final int key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getLong(index) : super.getLong(key); - } - - @Override - public double getDouble(final Object key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getDouble(index) : super.getDouble(key); - } - - @Override - public double getDouble(final double key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getDouble(index) : super.getDouble(key); - } - - @Override - public double getDouble(final long key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getDouble(index) : super.getDouble(key); - } - - @Override - public double getDouble(final int key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getDouble(index) : super.getDouble(key); - } - - @Override - public Object get(final Object key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getObject(index) : super.get(key); - } - - @Override - public Object get(final double key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getObject(index) : super.get(key); - } - - @Override - public Object get(final long key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getObject(index) : super.get(key); - } - - @Override - public Object get(final int key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) ? namedArgs.getObject(index) : super.get(key); - } - - @Override - public void set(final Object key, final int value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); + assert key >= 0 && key < numParams : "invalid argument index"; + if (isMapped(key)) { + setArray(getArray().set(key, value, false)); } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final Object key, final long value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final Object key, final double value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final Object key, final Object value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final double key, final int value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); + setUnmappedArg(key, value); } } @Override - public void set(final double key, final long value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final double key, final double value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final double key, final Object value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final long key, final int value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final long key, final long value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final long key, final double value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final long key, final Object value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final int key, final int value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final int key, final long value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final int key, final double value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public void set(final int key, final Object value, final boolean strict) { - final int index = ArrayIndex.getArrayIndex(key); - if (isMapped(index)) { - namedArgs = namedArgs.set(index, value, strict); - } else { - super.set(key, value, strict); - } - } - - @Override - public boolean has(final Object key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) || super.has(key); - } - - @Override - public boolean has(final double key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) || super.has(key); - } - - @Override - public boolean has(final long key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) || super.has(key); - } - - @Override - public boolean has(final int key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) || super.has(key); - } - - @Override - public boolean hasOwnProperty(final Object key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) || super.hasOwnProperty(key); - } - - @Override - public boolean hasOwnProperty(final int key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) || super.hasOwnProperty(key); - } - - @Override - public boolean hasOwnProperty(final long key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) || super.hasOwnProperty(key); - } - - @Override - public boolean hasOwnProperty(final double key) { - final int index = ArrayIndex.getArrayIndex(key); - return isMapped(index) || super.hasOwnProperty(key); - } - - @Override public boolean delete(final int key, final boolean strict) { final int index = ArrayIndex.getArrayIndex(key); - final boolean success = super.delete(key, strict); - if (success && namedArgs.has(index)) { - setDeleted(index); - } - return success; + return isMapped(index) ? deleteMapped(index, strict) : super.delete(key, strict); } @Override public boolean delete(final long key, final boolean strict) { final int index = ArrayIndex.getArrayIndex(key); - final boolean success = super.delete(key, strict); - if (success && namedArgs.has(index)) { - setDeleted(index); - } - return success; + return isMapped(index) ? deleteMapped(index, strict) : super.delete(key, strict); } @Override public boolean delete(final double key, final boolean strict) { final int index = ArrayIndex.getArrayIndex(key); - final boolean success = super.delete(key, strict); - if (success && namedArgs.has(index)) { - setDeleted(index); - } - return success; + return isMapped(index) ? deleteMapped(index, strict) : super.delete(key, strict); } @Override public boolean delete(final Object key, final boolean strict) { final int index = ArrayIndex.getArrayIndex(key); - final boolean success = super.delete(key, strict); - if (success && namedArgs.has(index)) { - setDeleted(index); - } - return success; + return isMapped(index) ? deleteMapped(index, strict) : super.delete(key, strict); } /** @@ -483,29 +152,27 @@ public boolean defineOwnProperty(final String key, final Object propertyDesc, final boolean reject) { final int index = ArrayIndex.getArrayIndex(key); if (index >= 0) { - final boolean allowed = super.defineOwnProperty(key, propertyDesc, false); - if (!allowed) { + final boolean isMapped = isMapped(index); + final Object oldValue = isMapped ? getArray().getObject(index) : null; + + if (!super.defineOwnProperty(key, propertyDesc, false)) { if (reject) { throw typeError("cant.redefine.property", key, ScriptRuntime.safeToString(this)); } return false; } - if (isMapped(index)) { + if (isMapped) { // When mapped argument is redefined, if new descriptor is accessor property // or data-non-writable property, we have to "unmap" (unlink). final PropertyDescriptor desc = toPropertyDescriptor(Global.instance(), propertyDesc); if (desc.type() == PropertyDescriptor.ACCESSOR) { - setDeleted(index); - } else { - // set "value" from new descriptor to named args - if (desc.has(PropertyDescriptor.VALUE)) { - namedArgs = namedArgs.set(index, desc.getValue(), false); - } - - if (desc.has(PropertyDescriptor.WRITABLE) && !desc.isWritable()) { - setDeleted(index); - } + setDeleted(index, oldValue); + } else if (desc.has(PropertyDescriptor.WRITABLE) && !desc.isWritable()) { + // delete and set value from new descriptor if it has one, otherwise use old value + setDeleted(index, desc.has(PropertyDescriptor.VALUE) ? desc.getValue() : oldValue); + } else if (desc.has(PropertyDescriptor.VALUE)) { + setArray(getArray().set(index, desc.getValue(), false)); } } @@ -519,31 +186,72 @@ // We track deletions using a bit set (delete arguments[index]) private boolean isDeleted(final int index) { - return (deleted != null) ? deleted.get(index) : false; + return deleted != null && deleted.get(index); + } + + private void setDeleted(final int index, final Object unmappedValue) { + if (deleted == null) { + deleted = new BitSet(numMapped); + } + deleted.set(index, true); + setUnmappedArg(index, unmappedValue); + } + + private boolean deleteMapped(final int index, final boolean strict) { + final Object value = getArray().getObject(index); + final boolean success = super.delete(index, strict); + if (success) { + setDeleted(index, value); + } + return success; + } + + private Object getUnmappedArg(final int key) { + assert key >= 0 && key < numParams; + return unmappedArgs == null ? UNDEFINED : unmappedArgs.getObject(key); } - private void setDeleted(final int index) { - if (deleted == null) { - deleted = new BitSet((int)namedArgs.length()); + private void setUnmappedArg(final int key, final Object value) { + assert key >= 0 && key < numParams; + if (unmappedArgs == null) { + /* + * Declared number of parameters may be more or less than the actual passed + * runtime arguments count. We need to truncate or extend with undefined values. + * + * Example: + * + * // less declared params + * (function (x) { print(arguments); })(20, 44); + * + * // more declared params + * (function (x, y) { print(arguments); })(3); + */ + final Object[] newValues = new Object[numParams]; + System.arraycopy(getArray().asObjectArray(), 0, newValues, 0, numMapped); + if (numMapped < numParams) { + Arrays.fill(newValues, numMapped, numParams, UNDEFINED); + } + this.unmappedArgs = ArrayData.allocate(newValues); } - deleted.set(index, true); + // Set value of argument + unmappedArgs = unmappedArgs.set(key, value, false); } /** * Are arguments[index] and corresponding named parameter linked? * - * In non-strict mode, arguments[index] and corresponding named param - * are "linked" or "mapped". Modifications are tacked b/w each other - till - * (delete arguments[index]) is used. Once deleted, the corresponding arg - * is no longer 'mapped'. Please note that delete can happen only through - * the arguments array - named param can not be deleted. (delete is one-way). + * In non-strict mode, arguments[index] and corresponding named param are "linked" or "mapped" + * if the argument is provided by the caller. Modifications are tacked b/w each other - until + * (delete arguments[index]) is used. Once deleted, the corresponding arg is no longer 'mapped'. + * Please note that delete can happen only through the arguments array - named param can not + * be deleted. (delete is one-way). */ private boolean isMapped(final int index) { - // in named args and not marked as "deleted" - return namedArgs.has(index) && !isDeleted(index); + // in mapped named args and not marked as "deleted" + return index >= 0 && index < numMapped && !isDeleted(index); } - /** + /** * Factory to create correct Arguments object based on strict mode. * * @param arguments the actual arguments array passed
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/script/basic/JDK-8022731.js Mon Aug 12 13:31:43 2013 +0200 @@ -0,0 +1,93 @@ +/* + * Copyright (c) 2010, 2013, 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-8022731: NativeArguments has wrong implementation of isMapped() + * + * @test + * @run + */ + +Object.defineProperty(Object.prototype, "0", {value: "proto"}); + +function test0(a, b) { + Object.defineProperty(arguments, "1", {get: function() { return "get" }}); + return arguments[0]; +} + +function test1(a, b) { + Object.defineProperty(arguments, "0", {get: function() { return "get" }}); + return a; +} + +function test2(a, b) { + Object.defineProperty(arguments, "0", {value: "value"}); + delete arguments[0]; + return a; +} + +function test3(a, b) { + arguments[1] = "arg1"; + return b; +} + +function test4(a, b) { + b = "b"; + return arguments[1]; +} + +function test5(a, b) { + Object.defineProperty(arguments, "0", {value: "value"}); + arguments[0] = "new"; + return a; +} + +function test6(a, b) { + Object.defineProperty(arguments, "0", {value: "value"}); + arguments[0] = "new"; + delete arguments[0]; + return a; +} + +function test7(a, b) { + Object.defineProperty(arguments, "0", {value: "value", writable: false}); + arguments[0] = "new"; + return a; +} + +print(test0()); +print(test0("p1", "p2")); +print(test1()); +print(test1("p1")); +print(test2()); +print(test2("p1")); +print(test3()); +print(test3(1, 2)); +print(test4()); +print(test4("p1", "p2")); +print(test5()); +print(test5("p1")); +print(test6()); +print(test6("p1")); +print(test7()); +print(test7("p1"));