[GR-24687] Corrections of ArraySetLength().

PullRequest: js/1576

Author: iamstolis

graal-js/src/com.oracle.truffle.js.test/js/GR-24687.js

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * Licensed under the Universal Permissive License v 1.0 as shown at http://oss.oracle.com/licenses/upl.
+ */
+
+load('assert.js');
+
+// valueOf() should be invoked twice (from ToUint32() and ToNumber())
+var valueOfCalls = 0;
+Object.defineProperty([0, 1, 2], 'length', {
+    value: {
+        valueOf: function () {
+            valueOfCalls++;
+            return 3;
+        }
+    }
+});
+assertSame(2, valueOfCalls);
+
+// length should remain writable when its re-definition is refused
+var array = [42, 211];
+assertThrows(function () {
+    Object.defineProperty(array, 'length', {
+        writable: false,
+        configurable: true
+    });
+}, TypeError);
+array.length = 5;
+assertSame(5, array.length);
+
+// length should be constant after re-definition with "writable: false"
+var array = [0, 1, 2];
+Object.defineProperty(array, 'length', {
+    value: 3,
+    writable: false
+});
+assertThrows(function () {
+    array.splice(1, 2, 4);
+}, TypeError);
+assertSame(3, array.length);
+assertSame(array[0], 0);
+assertSame(array[1], 4);
+assertSame(array[2], undefined);
+
+true;

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/builtins/JSAbstractArray.java

@@ -637,7 +637,7 @@ public static long toArrayIndexOrRangeError(Number len, Number len32, BranchProf
     @TruffleBoundary
     public boolean defineOwnProperty(DynamicObject thisObj, Object key, PropertyDescriptor descriptor, boolean doThrow) {
         if (key.equals(LENGTH)) {
-            return defineOwnPropertyLength(thisObj, key, descriptor, doThrow);
+            return defineOwnPropertyLength(thisObj, descriptor, doThrow);
         } else if (key instanceof String && JSRuntime.isArrayIndex((String) key)) {
             return defineOwnPropertyIndex(thisObj, (String) key, descriptor, doThrow);
         } else {
@@ -651,17 +651,18 @@ public boolean defineOwnProperty(DynamicObject thisObj, Object key, PropertyDesc
      *
      * @return whether the operation was successful
      */
-    private boolean defineOwnPropertyLength(DynamicObject thisObj, Object key, PropertyDescriptor descriptor, boolean doThrow) {
+    private boolean defineOwnPropertyLength(DynamicObject thisObj, PropertyDescriptor descriptor, boolean doThrow) {
         if (!descriptor.hasValue()) {
-            if (descriptor.hasWritable() && !descriptor.getWritable()) {
+            boolean success = DefinePropertyUtil.ordinaryDefineOwnProperty(thisObj, LENGTH, descriptor, doThrow);
+            if (success && descriptor.hasWritable() && !descriptor.getWritable()) {
                 setLengthNotWritable(thisObj);
             }
-            return DefinePropertyUtil.ordinaryDefineOwnProperty(thisObj, key, descriptor, doThrow);
+            return success;
         }
 
-        Number newLenNum = JSRuntime.toNumber(descriptor.getValue());
-        long newLen = JSRuntime.toUInt32(newLenNum);
-        if (JSRuntime.doubleValue(newLenNum) != newLen) {
+        long newLen = JSRuntime.toUInt32(descriptor.getValue());
+        Number numberLen = JSRuntime.toNumber(descriptor.getValue());
+        if (JSRuntime.doubleValue(numberLen) != newLen) {
             throw Errors.createRangeErrorInvalidArrayLength();
         }
         PropertyDescriptor lenDesc = getOwnProperty(thisObj, LENGTH);
@@ -715,32 +716,26 @@ private boolean deleteElementsAfterShortening(DynamicObject thisObj, PropertyDes
 
     private boolean definePropertyLength(DynamicObject thisObj, PropertyDescriptor descriptor, PropertyDescriptor currentDesc, long len, boolean doThrow) {
         assert JSRuntime.isValidArrayLength(len);
+        assert !currentDesc.getConfigurable();
         boolean currentWritable = currentDesc.getWritable();
         boolean currentEnumerable = currentDesc.getEnumerable();
-        boolean currentConfigurable = currentDesc.getConfigurable();
 
         boolean newWritable = descriptor.getIfHasWritable(currentWritable);
         boolean newEnumerable = descriptor.getIfHasEnumerable(currentEnumerable);
-        boolean newConfigurable = descriptor.getIfHasConfigurable(currentConfigurable);
+        boolean newConfigurable = descriptor.getIfHasConfigurable(false);
 
-        if (currentConfigurable) {
-            if (!currentWritable && !newWritable) {
-                return DefinePropertyUtil.reject(doThrow, LENGTH_PROPERTY_NOT_WRITABLE);
-            }
-        } else {
-            if (descriptor.getConfigurable() || (newEnumerable != currentEnumerable)) {
-                // ES2020 9.1.6.3, 4.a and 4.b
-                return DefinePropertyUtil.reject(doThrow, CANNOT_REDEFINE_PROPERTY_LENGTH);
-            }
-            if (currentWritable == newWritable && currentEnumerable == newEnumerable) {
-                if (!descriptor.hasValue() || len == getLength(thisObj)) {
-                    return true; // nothing changed
-                }
-            }
-            if (!currentWritable) {
-                return DefinePropertyUtil.reject(doThrow, LENGTH_PROPERTY_NOT_WRITABLE);
+        if (newConfigurable || (newEnumerable != currentEnumerable)) {
+            // ES2020 9.1.6.3, 4.a and 4.b
+            return DefinePropertyUtil.reject(doThrow, CANNOT_REDEFINE_PROPERTY_LENGTH);
+        }
+        if (currentWritable == newWritable && currentEnumerable == newEnumerable) {
+            if (!descriptor.hasValue() || len == getLength(thisObj)) {
+                return true; // nothing changed
             }
         }
+        if (!currentWritable) {
+            return DefinePropertyUtil.reject(doThrow, LENGTH_PROPERTY_NOT_WRITABLE);
+        }
 
         try {
             setLength(thisObj, len, doThrow);
@@ -749,6 +744,10 @@ private boolean definePropertyLength(DynamicObject thisObj, PropertyDescriptor d
             JSObjectUtil.changeFlags(thisObj, LENGTH, newAttr);
         }
 
+        if (!newWritable) {
+            setLengthNotWritable(thisObj);
+        }
+
         return true;
     }
 

graal-js/test/testV8.json

@@ -1077,9 +1077,6 @@
     "filePath" : "mjsunit/regress/regress-5638b.js",
     "status" : "FAIL",
     "comment" : "TODO: evaluate (V8 tests update 2017-07-07)"
-  }, {
-    "filePath" : "mjsunit/regress/regress-5669.js",
-    "status" : "FAIL"
   }, {
     "filePath" : "mjsunit/regress/regress-575364.js",
     "status" : "SKIP",
@@ -1580,10 +1577,6 @@
     "filePath" : "mjsunit/regress/regress-crbug-816961.js",
     "status" : "FAIL",
     "comment" : "TODO: evaluate (V8 tests update 2018-10-29)"
-  }, {
-    "filePath" : "mjsunit/regress/regress-crbug-823069.js",
-    "status" : "FAIL",
-    "comment" : "TODO: evaluate (V8 tests update 2018-10-29)"
   }, {
     "filePath" : "mjsunit/regress/regress-crbug-865892.js",
     "status" : "FAIL",
@@ -1733,10 +1726,6 @@
     "filePath" : "mjsunit/regress/regress-wasm-crbug-618602.js",
     "status" : "SKIP",
     "comment" : "WASM (GR-6494)"
-  }, {
-    "filePath" : "mjsunit/regress/regress_967104.js",
-    "status" : "FAIL",
-    "comment" : "TODO: evaluate (V8 tests update 2019-10-15)"
   }, {
     "filePath" : "mjsunit/regress/wasm/loop-stack-check.js",
     "status" : "SKIP",