[MERGE #4790 @irinayat-MS] Array.copyWithin should throw when invoked…

… on TypedArray with missing slots Merge pull request #4790 from irinayat-MS:Throw_copyWithin Fixes #4356 and a regression from #4118
chakra-core · Mar 9, 2018 · e972090 · e972090
2 parents 3e8e3ff + f219294
commit e972090
Show file tree

Hide file tree

Showing 13 changed files with 81 additions and 54 deletions.
diff --git a/lib/Runtime/Language/JavascriptOperators.cpp b/lib/Runtime/Language/JavascriptOperators.cpp
@@ -4818,11 +4818,7 @@ namespace Js
 #endif
         }
 
-        if (result == FALSE && (propertyOperationFlags & PropertyOperation_StrictMode))
-        {
-            JavascriptError::ThrowCantDelete(propertyOperationFlags, scriptContext, GetPropertyDisplayNameForError(index, scriptContext)->GetString());
-        }
-
+        Assert(result || !(propertyOperationFlags & (PropertyOperation_StrictMode | PropertyOperation_ThrowOnDeleteIfNotConfig)));
         return scriptContext->GetLibrary()->CreateBoolean(result);
     }
 

diff --git a/lib/Runtime/Library/JavascriptError.cpp b/lib/Runtime/Library/JavascriptError.cpp
@@ -694,7 +694,7 @@ namespace Js
         return false;
     }
 
-    bool JavascriptError::ThrowCantDelete(PropertyOperationFlags flags, ScriptContext* scriptContext, PCWSTR varName)
+    bool JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(PropertyOperationFlags flags, ScriptContext* scriptContext, PCWSTR varName)
     {
         bool isNonConfigThrow = (flags & PropertyOperation_ThrowOnDeleteIfNotConfig) == PropertyOperation_ThrowOnDeleteIfNotConfig;
 

diff --git a/lib/Runtime/Library/JavascriptError.h b/lib/Runtime/Library/JavascriptError.h
@@ -135,7 +135,7 @@ namespace Js
         static bool ThrowCantAssignIfStrictMode(PropertyOperationFlags flags, ScriptContext* scriptContext);
         static bool ThrowCantExtendIfStrictMode(PropertyOperationFlags flags, ScriptContext* scriptContext);
         static bool ThrowCantDeleteIfStrictMode(PropertyOperationFlags flags, ScriptContext* scriptContext, PCWSTR varName);
-        static bool ThrowCantDelete(PropertyOperationFlags flags, ScriptContext* scriptContext, PCWSTR varName);
+        static bool ThrowCantDeleteIfStrictModeOrNonconfigurable(PropertyOperationFlags flags, ScriptContext* scriptContext, PCWSTR varName);
         static bool ThrowIfStrictModeUndefinedSetter(PropertyOperationFlags flags, Var setterValue, ScriptContext* scriptContext);
         static bool ThrowIfNotExtensibleUndefinedSetter(PropertyOperationFlags flags, Var setterValue, ScriptContext* scriptContext);
 

diff --git a/lib/Runtime/Library/TypedArray.cpp b/lib/Runtime/Library/TypedArray.cpp
@@ -851,6 +851,13 @@ namespace Js
         return PropertyQueryFlags::Property_NotFound_NoProto;
     }
 
+    BOOL TypedArrayBase::DeleteItem(uint32 index, Js::PropertyOperationFlags flags)
+    {
+        JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(
+            flags, GetScriptContext(), GetScriptContext()->GetIntegerString(index)->GetString());
+        return FALSE;
+    }
+
     PropertyQueryFlags TypedArrayBase::GetItemQuery(Var originalInstance, uint32 index, Var* value, ScriptContext* requestContext)
     {
         *value = DirectGetItem(index);

diff --git a/lib/Runtime/Library/TypedArray.h b/lib/Runtime/Library/TypedArray.h
@@ -108,7 +108,7 @@ namespace Js
         virtual PropertyQueryFlags GetPropertyQuery(Js::Var originalInstance, Js::JavascriptString* propertyNameString, Js::Var* value, Js::PropertyValueInfo* info, Js::ScriptContext* requestContext) override;
         virtual PropertyQueryFlags GetPropertyReferenceQuery(Js::Var originalInstance, Js::PropertyId propertyId, Js::Var* value, Js::PropertyValueInfo* info, Js::ScriptContext* requestContext) override;
         virtual PropertyQueryFlags HasItemQuery(uint32 index) override;
-        virtual BOOL DeleteItem(uint32 index, Js::PropertyOperationFlags flags) override { return false; }
+        virtual BOOL DeleteItem(uint32 index, Js::PropertyOperationFlags flags) override;
         virtual PropertyQueryFlags GetItemQuery(Js::Var originalInstance, uint32 index, Js::Var* value, Js::ScriptContext * requestContext) override;
         virtual BOOL SetItem(uint32 index, Js::Var value, Js::PropertyOperationFlags flags = PropertyOperation_None) override;
         virtual BOOL SetProperty(Js::PropertyId propertyId, Js::Var value, Js::PropertyOperationFlags flags, Js::PropertyValueInfo* info) override;

diff --git a/lib/Runtime/Types/DictionaryTypeHandler.cpp b/lib/Runtime/Types/DictionaryTypeHandler.cpp
@@ -912,7 +912,8 @@ namespace Js
             else if (!(descriptor->Attributes & PropertyConfigurable))
             {
                 // Let/const properties do not have attributes and they cannot be deleted
-                JavascriptError::ThrowCantDelete(propertyOperationFlags, scriptContext, propertyNameString->GetString());
+                JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(
+                    propertyOperationFlags, scriptContext, propertyNameString->GetString());
 
                 return false;
             }
@@ -1007,7 +1008,8 @@ namespace Js
                 (allowLetConstGlobal && (descriptor->Attributes & PropertyLetConstGlobal)))
             {
                 // Let/const properties do not have attributes and they cannot be deleted
-                JavascriptError::ThrowCantDelete(propertyOperationFlags, scriptContext, scriptContext->GetPropertyName(propertyId)->GetBuffer());
+                JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(
+                    propertyOperationFlags, scriptContext, scriptContext->GetPropertyName(propertyId)->GetBuffer());
 
                 return false;
             }

diff --git a/lib/Runtime/Types/ES5ArrayTypeHandler.cpp b/lib/Runtime/Types/ES5ArrayTypeHandler.cpp
@@ -640,7 +640,8 @@ namespace Js
             }
             else if (!(descriptor->Attributes & PropertyConfigurable))
             {
-                JavascriptError::ThrowCantDelete(propertyOperationFlags, instance->GetScriptContext(), TaggedInt::ToString(index, instance->GetScriptContext())->GetString());
+                JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(
+                    propertyOperationFlags, instance->GetScriptContext(), TaggedInt::ToString(index, instance->GetScriptContext())->GetString());
 
                 return false;
             }

diff --git a/lib/Runtime/Types/PathTypeHandler.cpp b/lib/Runtime/Types/PathTypeHandler.cpp
@@ -879,7 +879,8 @@ namespace Js
         }
         if (!(attr & ObjectSlotAttr_Configurable))
         {
-            JavascriptError::ThrowCantDelete(PropertyOperation_None, scriptContext, scriptContext->GetPropertyName(propertyId)->GetBuffer());
+            JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(
+                flags, scriptContext, scriptContext->GetPropertyName(propertyId)->GetBuffer());
             return FALSE;
         }
 

diff --git a/lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp b/lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp
@@ -1576,7 +1576,8 @@ namespace Js
                 }
                 else if (!(descriptor->Attributes & PropertyConfigurable))
                 {
-                    JavascriptError::ThrowCantDelete(propertyOperationFlags, scriptContext, propertyNameString->GetString()); // or propertyName->GetBuffer
+                    JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(
+                        propertyOperationFlags, scriptContext, propertyNameString->GetString()); // or propertyName->GetBuffer
 
                     return false;
                 }
@@ -1708,7 +1709,8 @@ namespace Js
                 else if (!(descriptor->Attributes & PropertyConfigurable) ||
                     (allowLetConstGlobal && (descriptor->Attributes & PropertyLetConstGlobal)))
                 {
-                    JavascriptError::ThrowCantDelete(propertyOperationFlags, scriptContext, propertyRecord->GetBuffer());
+                    JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(
+                        propertyOperationFlags, scriptContext, propertyRecord->GetBuffer());
 
                     return false;
                 }

diff --git a/lib/Runtime/Types/SimpleTypeHandler.cpp b/lib/Runtime/Types/SimpleTypeHandler.cpp
@@ -556,7 +556,8 @@ namespace Js
             }
             if (!(descriptors[index].Attributes & PropertyConfigurable))
             {
-                JavascriptError::ThrowCantDelete(propertyOperationFlags, scriptContext, scriptContext->GetPropertyName(propertyId)->GetBuffer());
+                JavascriptError::ThrowCantDeleteIfStrictModeOrNonconfigurable(
+                    propertyOperationFlags, scriptContext, scriptContext->GetPropertyName(propertyId)->GetBuffer());
 
                 return false;
             }

diff --git a/test/Bugs/deletenonconfig.js b/test/Bugs/deletenonconfig.js
@@ -7,45 +7,57 @@ WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
 
 function makeProperty(obj, prop) {
     Object.defineProperty(obj, prop, {
-      configurable: false,
-      writable: true,
-      value:'basic'
+        configurable: false,
+        writable: true,
+        value: 'basic'
     });
 }
+
 var tests = [
-  {
-    name: "Delete non-configurable property on Array.prototype.copyWithin",
-    body: function () {
-        var obj = {length: 4};
-        makeProperty(obj, '3');
-        assert.throws(() => Array.prototype.copyWithin.call(obj, 3, 0), TypeError, "copyWithin is trying to delete the non-configurable property","Cannot delete non-configurable property '3'");
-    }
-  },
-  {
-    name: "Delete non-configurable property on Array.prototype.pop",
-    body: function () {
-        var obj = {length: 4};
-        makeProperty(obj, '3');
-        assert.throws(() => Array.prototype.pop.call(obj), TypeError, "pop is trying to delete the non-configurable property", "Cannot delete non-configurable property '3'");
-    }
-  },
-  {
-    name: "Delete non-configurable property on Array.prototype.shift",
-    body: function () {
-        var obj = {length: 4};
-        makeProperty(obj, '3');
-        assert.throws(() => Array.prototype.shift.call(obj), TypeError, "shift is trying to delete the non-configurable property", "Cannot delete non-configurable property '3'");
-    }
-  },
-  {
-    name: "Delete non-configurable property on Array.prototype.reverse",
-    body: function () {
-        var obj = {length: 4};
-        makeProperty(obj, '3');
-        assert.throws(() => Array.prototype.reverse.call(obj), TypeError, "reverse is trying to delete the non-configurable property", "Cannot delete non-configurable property '3'");
-    }
-  },
-
+    {
+        name: "Delete non-configurable property on Array.prototype.copyWithin",
+        body: function () {
+            var obj = { length: 4 };
+            makeProperty(obj, '3');
+            assert.throws(() => Array.prototype.copyWithin.call(obj, 3, 0), TypeError, "copyWithin is trying to delete the non-configurable property", "Cannot delete non-configurable property '3'");
+        }
+    },
+    {
+        name: "Delete non-configurable indexed property of TypedArray on Array.prototype.copyWithin",
+        body: function () {
+            var ta = Int8Array.of(0, 1, 2);
+            // Array's implementation of copyWithin uses 'length' property to determine the source items
+            // to copy and it would attempt to delete the target items if the source items are missing,
+            // which is not supported by TypedArrays.
+            Object.defineProperty(ta, "length", { value: 4 });
+            assert.throws(() => Array.prototype.copyWithin.call(ta, 1, 2), TypeError, "copyWithin is trying to delete the non-configurable indexed property", "Cannot delete non-configurable property '2'");
+        }
+    },
+    {
+        name: "Delete non-configurable property on Array.prototype.pop",
+        body: function () {
+            var obj = { length: 4 };
+            makeProperty(obj, '3');
+            assert.throws(() => Array.prototype.pop.call(obj), TypeError, "pop is trying to delete the non-configurable property", "Cannot delete non-configurable property '3'");
+        }
+    },
+    {
+        name: "Delete non-configurable property on Array.prototype.shift",
+        body: function () {
+            var obj = { length: 4 };
+            makeProperty(obj, '3');
+            assert.throws(() => Array.prototype.shift.call(obj), TypeError, "shift is trying to delete the non-configurable property", "Cannot delete non-configurable property '3'");
+        }
+    },
+    {
+        name: "Delete non-configurable property on Array.prototype.reverse",
+        body: function () {
+            var obj = { length: 4 };
+            makeProperty(obj, '3');
+            assert.throws(() => Array.prototype.reverse.call(obj), TypeError, "reverse is trying to delete the non-configurable property", "Cannot delete non-configurable property '3'");
+        }
+    },
+
 ];
 
 testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
diff --git a/test/es6/ES6TypedArrayExtensions.js b/test/es6/ES6TypedArrayExtensions.js
@@ -1314,12 +1314,12 @@ var tests = [
             // Call Array.prototype.reverse passing a TypedArray that lies about length. We should only reverse the part of it less than the indicated length.
             u = getTypedArray();
             Object.defineProperty(u, 'length', { value: 5 });
-            assert.areEqual([4,3,2,1,0,5,6,7,8,9], Array.prototype.reverse.call(u), "Calling %TypedArrayPrototype%.reverse with a TypedArray that lies about length");
+            assert.areEqual([4,3,2,1,0,5,6,7,8,9], Array.prototype.reverse.call(u), "Calling %ArrayPrototype%.reverse with a TypedArray that lies about length");
 
             // Call Array.prototype.reverse passing a TypedArray that lies about length. TypedArrays do not support delete so we will throw if indicated length is longer than actual.
             u = getTypedArray();
             Object.defineProperty(u, 'length', { value: 20 });
-            assert.throws(function() { Array.prototype.reverse.call(u); }, TypeError, "Calling %TypedArrayPrototype%.reverse with a TypedArray that says it has longer length than actual throws", "Object doesn't support this action");
+            assert.throws(function () { Array.prototype.reverse.call(u); }, TypeError, "Calling %ArrayPrototype%.reverse with a TypedArray that says it has longer length than actual throws", "Cannot delete non-configurable property '0'");
 
             assert.throws(function() { reverse.call(); }, TypeError, "Calling %TypedArrayPrototype%.reverse with no this throws TypeError", "'this' is not a typed array object");
             assert.throws(function() { reverse.call(undefined); }, TypeError, "Calling %TypedArrayPrototype%.reverse with undefined this throws TypeError", "'this' is not a typed array object");

diff --git a/test/strict/rlexe.xml b/test/strict/rlexe.xml
@@ -441,7 +441,12 @@
       <files>13.delete.js</files>
       <baseline>13.delete.baseline</baseline>
       <compile-flags>-ES6RegExPrototypeProperties-</compile-flags>
-      <!-- TODO: Delete depends on flag on scriptContext. Add a new opcode for delete 13.delete_sm.js -->
+    </default>
+  </test>
+  <test>
+    <default>
+      <files>13.delete_sm.js</files>
+      <baseline>13.delete_sm.baseline</baseline>
     </default>
   </test>
   <test>