[test262] Fixing mapped arguments object property test case
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Dec 2016 21:26:22 +0000 (21:26 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Dec 2016 21:26:22 +0000 (21:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=159398

Patch by Caio Lima <ticaiolima@gmail.com> on 2016-12-24
Reviewed by Saam Barati.

JSTests:

* stress/arguments-bizarre-behaviour-disable-enumerability.js:
* stress/arguments-define-property.js: Added.
(assert):
(testProperties):
* stress/arguments-non-configurable.js: Added.
(assert):
(tryChangeNonConfigurableDescriptor):
(set tryChangeNonConfigurableDescriptor):
(tryChangeWritableOfNonConfigurableDescriptor):
* test262.yaml:

Source/JavaScriptCore:

This patch changes GenericArguments' override mechanism to
implement corret behavior on ECMAScript test262 suite test cases of
mapped arguments object with non-configurable and non-writable
property. Also it is ensuring that arguments[i]
cannot be deleted when argument "i" is {configurable: false}.

The previous implementation is against to the specification for 2 reasons:

1. Every argument in arguments object are {writable: true} by default
   (http://www.ecma-international.org/ecma-262/7.0/index.html#sec-createunmappedargumentsobject).
   It means that we have to stop mapping a defined property index
   if the new property descriptor contains writable (i.e writable is
   present) and its value is false (also check
   https://tc39.github.io/ecma262/#sec-arguments-exotic-objects-defineownproperty-p-desc).
   Previous implementation considers {writable: false} if writable is
   not present.

2. When a property is overriden, "delete" operation is always returning true. However
   delete operations should follow the specification.

We created an auxilary boolean array named m_modifiedArgumentsDescriptor
to store which arguments[i] descriptor was changed from its default
property descriptor. This modification was necessary because m_overrides
was responsible to keep this information at the same time
of keeping information about arguments mapping. The problem of this apporach was
that we needed to call overridesArgument(i) as soon as the ith argument's property
descriptor was changed and it stops the argument's mapping as sideffect, producing
wrong behavior.
To keep tracking arguments mapping status, we renamed DirectArguments::m_overrides to
DirectArguments::m_mappedArguments and now we it is responsible to manage if an
argument[i] is mapped or not.
With these 2 structures, now it is possible to an argument[i] have its property
descriptor modified and don't stop the mapping as soon as it happens. One example
of that wrong behavior can be found on arguments-bizarre-behaviour-disable-enumerability
test case, that now is fixed by this new mechanism.

* bytecode/PolymorphicAccess.cpp:
(JSC::AccessCase::generateWithGuard):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnDirectArguments):
(JSC::DFG::SpeculativeJIT::compileGetArrayLength):
(JSC::DFG::SpeculativeJIT::compileCreateDirectArguments):
* ftl/FTLAbstractHeapRepository.h:
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileCreateDirectArguments):
* jit/JITOperations.cpp:
(JSC::canAccessArgumentIndexQuickly):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emitDirectArgumentsGetByVal):
* runtime/DirectArguments.cpp:
(JSC::DirectArguments::estimatedSize):
(JSC::DirectArguments::visitChildren):
(JSC::DirectArguments::overrideThings):
(JSC::DirectArguments::overrideThingsIfNecessary):
(JSC::DirectArguments::unmapArgument):
(JSC::DirectArguments::copyToArguments):
(JSC::DirectArguments::overridesSize):
(JSC::DirectArguments::overrideArgument): Deleted.
* runtime/DirectArguments.h:
(JSC::DirectArguments::length):
(JSC::DirectArguments::isMappedArgument):
(JSC::DirectArguments::isMappedArgumentInDFG):
(JSC::DirectArguments::getIndexQuickly):
(JSC::DirectArguments::setIndexQuickly):
(JSC::DirectArguments::overrodeThings):
(JSC::DirectArguments::initModifiedArgumentsDescriptorIfNecessary):
(JSC::DirectArguments::setModifiedArgumentDescriptor):
(JSC::DirectArguments::isModifiedArgumentDescriptor):
(JSC::DirectArguments::offsetOfMappedArguments):
(JSC::DirectArguments::offsetOfModifiedArgumentsDescriptor):
(JSC::DirectArguments::canAccessIndexQuickly): Deleted.
(JSC::DirectArguments::canAccessArgumentIndexQuicklyInDFG): Deleted.
(JSC::DirectArguments::offsetOfOverrides): Deleted.
* runtime/GenericArguments.h:
* runtime/GenericArgumentsInlines.h:
(JSC::GenericArguments<Type>::visitChildren):
(JSC::GenericArguments<Type>::getOwnPropertySlot):
(JSC::GenericArguments<Type>::getOwnPropertySlotByIndex):
(JSC::GenericArguments<Type>::getOwnPropertyNames):
(JSC::GenericArguments<Type>::put):
(JSC::GenericArguments<Type>::putByIndex):
(JSC::GenericArguments<Type>::deleteProperty):
(JSC::GenericArguments<Type>::deletePropertyByIndex):
(JSC::GenericArguments<Type>::defineOwnProperty):
(JSC::GenericArguments<Type>::initModifiedArgumentsDescriptor):
(JSC::GenericArguments<Type>::initModifiedArgumentsDescriptorIfNecessary):
(JSC::GenericArguments<Type>::setModifiedArgumentDescriptor):
(JSC::GenericArguments<Type>::isModifiedArgumentDescriptor):
(JSC::GenericArguments<Type>::copyToArguments):
* runtime/ScopedArguments.cpp:
(JSC::ScopedArguments::visitChildren):
(JSC::ScopedArguments::unmapArgument):
(JSC::ScopedArguments::overrideArgument): Deleted.
* runtime/ScopedArguments.h:
(JSC::ScopedArguments::isMappedArgument):
(JSC::ScopedArguments::isMappedArgumentInDFG):
(JSC::ScopedArguments::getIndexQuickly):
(JSC::ScopedArguments::setIndexQuickly):
(JSC::ScopedArguments::initModifiedArgumentsDescriptorIfNecessary):
(JSC::ScopedArguments::setModifiedArgumentDescriptor):
(JSC::ScopedArguments::isModifiedArgumentDescriptor):
(JSC::ScopedArguments::canAccessIndexQuickly): Deleted.
(JSC::ScopedArguments::canAccessArgumentIndexQuicklyInDFG): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@210146 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
JSTests/ChangeLog
JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js
JSTests/stress/arguments-define-property.js [new file with mode: 0644]
JSTests/stress/arguments-non-configurable.js [new file with mode: 0644]
JSTests/test262.yaml
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/jit/JITPropertyAccess.cpp
Source/JavaScriptCore/runtime/DirectArguments.cpp
Source/JavaScriptCore/runtime/DirectArguments.h
Source/JavaScriptCore/runtime/GenericArguments.h
Source/JavaScriptCore/runtime/GenericArgumentsInlines.h
Source/JavaScriptCore/runtime/ScopedArguments.cpp
Source/JavaScriptCore/runtime/ScopedArguments.h

index 55cd99b..2d8a87d 100644 (file)
@@ -1,3 +1,39 @@
+2016-12-24  Caio Lima  <ticaiolima@gmail.com>
+
+        [test262] Fixing mapped arguments object property test case
+        https://bugs.webkit.org/show_bug.cgi?id=159398
+
+        Reviewed by Saam Barati.
+
+        * stress/arguments-bizarre-behaviour-disable-enumerability.js:
+        * stress/arguments-define-property.js: Added.
+        (assert):
+        (testProperties):
+        * stress/arguments-non-configurable.js: Added.
+        (assert):
+        (tryChangeNonConfigurableDescriptor):
+        (set tryChangeNonConfigurableDescriptor):
+        (tryChangeWritableOfNonConfigurableDescriptor):
+        * test262.yaml:
+
+016-12-20  Caio Lima  <ticaiolima@gmail.com>
+
+        [test262] Fixing mapped arguments object property test case
+        https://bugs.webkit.org/show_bug.cgi?id=159398
+
+        Reviewed by .
+
+        * stress/arguments-bizarre-behaviour-disable-enumerability.js:
+        * stress/arguments-define-property.js: Added.
+        (assert):
+        (testProperties):
+        * stress/arguments-non-configurable.js: Added.
+        (assert):
+        (tryChangeNonConfigurableDescriptor):
+        (set tryChangeNonConfigurableDescriptor):
+        (tryChangeWritableOfNonConfigurableDescriptor):
+        * test262.yaml:
+
 2016-12-23  Keith Miller  <keith_miller@apple.com>
 
         WebAssembly: trap on bad division.
index 947850a..ab5735f 100644 (file)
@@ -24,7 +24,5 @@ if (array.join(",") != "0")
 if (Object.keys(result[2]).join(",") != "0")
     throw new Error();
 
-// FIXME: This is totally weird!
-// https://bugs.webkit.org/show_bug.cgi?id=141952
-if (Object.getOwnPropertyDescriptor(result[2], 0).enumerable !== true)
+if (Object.getOwnPropertyDescriptor(result[2], 0).enumerable === true)
     throw new Error();
diff --git a/JSTests/stress/arguments-define-property.js b/JSTests/stress/arguments-define-property.js
new file mode 100644 (file)
index 0000000..b8d9ba8
--- /dev/null
@@ -0,0 +1,34 @@
+function assert(a) {
+    if (!a)
+        throw Error("Bad assertion!");
+}
+
+function testProperties(o, initProperty, testProperty, shouldThrow) {
+    Object.defineProperty(arguments, 0, initProperty);
+
+    if (shouldThrow) {
+        try {
+            Object.defineProperty(arguments, 0, testProperty);
+            assert(false);
+        } catch(e) {
+            assert(e instanceof TypeError);
+        }
+    } else {
+        assert(Object.defineProperty(arguments, 0, testProperty));
+    }
+}
+
+testProperties("foo", {configurable: false}, {writable: true}, false);
+testProperties("foo", {configurable: false}, {configurable: true}, true);
+testProperties("foo", {configurable: false, enumareble: true}, {enumerable: false}, true);
+testProperties("foo", {configurable: false, writable: false}, {writable: false}, false);
+testProperties("foo", {configurable: false, writable: false}, {writable: true}, true);
+testProperties("foo", {configurable: false, writable: false, value: 50}, {value: 30}, true);
+testProperties("foo", {configurable: false, writable: false, value: 30}, {value: 30}, false);
+testProperties("foo", {configurable: false, get: () => {return 0}}, {get: () => {return 10}}, true);
+let getterFoo = () => {return 0};
+testProperties("foo", {configurable: false, get: getterFoo}, {get: getterFoo}, false);
+testProperties("foo", {configurable: false, set: (x) => {return 0}}, {get: (x) => {return 10}}, true);
+let setterFoo = (x) => {return 0};
+testProperties("foo", {configurable: false, set: setterFoo}, {set: setterFoo}, false);
+
diff --git a/JSTests/stress/arguments-non-configurable.js b/JSTests/stress/arguments-non-configurable.js
new file mode 100644 (file)
index 0000000..c5a055f
--- /dev/null
@@ -0,0 +1,27 @@
+function assert(a) {
+    if (!a)
+        throw Error("Bad assertion!");
+}
+
+function tryChangeNonConfigurableDescriptor(x) {
+    Object.defineProperty(arguments, 0, {configurable: false});
+    try {
+        Object.defineProperty(arguments, 0, x);
+        assert(false);
+    } catch(e) {
+        assert(e instanceof TypeError);
+    }
+}
+
+tryChangeNonConfigurableDescriptor({get: () => {return 50;} });
+tryChangeNonConfigurableDescriptor({set: (x) => {}});
+tryChangeNonConfigurableDescriptor({writable: true, enumerable: false});
+
+function tryChangeWritableOfNonConfigurableDescriptor(x) {
+    Object.defineProperty(arguments, 0, {configurable: false});
+    Object.defineProperty(arguments, 0, {writable: true});
+    assert(Object.defineProperty(arguments, 0, {writable: false}));
+}
+
+tryChangeWritableOfNonConfigurableDescriptor("foo");
+
index f0eabf8..e344211 100644 (file)
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-1.js
   cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-2.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-3.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-4.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-1.js
   cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-2.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-3.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-4.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-1.js
   cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-2.js
   cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-3.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-4.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-5.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-strict-delete-1.js
   cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-strict-delete-2.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-strict-delete-3.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-strict-delete-4.js
-  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonwritable-nonconfigurable-1.js
   cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
 - path: test262/test/language/arguments-object/mapped/mapped-arguments-nonwritable-nonconfigurable-2.js
index b7d4161..a0bd33c 100644 (file)
@@ -1,3 +1,116 @@
+2016-12-24  Caio Lima  <ticaiolima@gmail.com>
+
+        [test262] Fixing mapped arguments object property test case
+        https://bugs.webkit.org/show_bug.cgi?id=159398
+
+        Reviewed by Saam Barati.
+
+        This patch changes GenericArguments' override mechanism to
+        implement corret behavior on ECMAScript test262 suite test cases of
+        mapped arguments object with non-configurable and non-writable
+        property. Also it is ensuring that arguments[i]
+        cannot be deleted when argument "i" is {configurable: false}.
+        
+        The previous implementation is against to the specification for 2 reasons:
+
+        1. Every argument in arguments object are {writable: true} by default
+           (http://www.ecma-international.org/ecma-262/7.0/index.html#sec-createunmappedargumentsobject).
+           It means that we have to stop mapping a defined property index
+           if the new property descriptor contains writable (i.e writable is
+           present) and its value is false (also check
+           https://tc39.github.io/ecma262/#sec-arguments-exotic-objects-defineownproperty-p-desc).
+           Previous implementation considers {writable: false} if writable is
+           not present.
+
+        2. When a property is overriden, "delete" operation is always returning true. However
+           delete operations should follow the specification.
+
+        We created an auxilary boolean array named m_modifiedArgumentsDescriptor
+        to store which arguments[i] descriptor was changed from its default
+        property descriptor. This modification was necessary because m_overrides
+        was responsible to keep this information at the same time
+        of keeping information about arguments mapping. The problem of this apporach was
+        that we needed to call overridesArgument(i) as soon as the ith argument's property
+        descriptor was changed and it stops the argument's mapping as sideffect, producing
+        wrong behavior.
+        To keep tracking arguments mapping status, we renamed DirectArguments::m_overrides to
+        DirectArguments::m_mappedArguments and now we it is responsible to manage if an
+        argument[i] is mapped or not.
+        With these 2 structures, now it is possible to an argument[i] have its property 
+        descriptor modified and don't stop the mapping as soon as it happens. One example
+        of that wrong behavior can be found on arguments-bizarre-behaviour-disable-enumerability
+        test case, that now is fixed by this new mechanism.
+
+        * bytecode/PolymorphicAccess.cpp:
+        (JSC::AccessCase::generateWithGuard):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnDirectArguments):
+        (JSC::DFG::SpeculativeJIT::compileGetArrayLength):
+        (JSC::DFG::SpeculativeJIT::compileCreateDirectArguments):
+        * ftl/FTLAbstractHeapRepository.h:
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::compileCreateDirectArguments):
+        * jit/JITOperations.cpp:
+        (JSC::canAccessArgumentIndexQuickly):
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emitDirectArgumentsGetByVal):
+        * runtime/DirectArguments.cpp:
+        (JSC::DirectArguments::estimatedSize):
+        (JSC::DirectArguments::visitChildren):
+        (JSC::DirectArguments::overrideThings):
+        (JSC::DirectArguments::overrideThingsIfNecessary):
+        (JSC::DirectArguments::unmapArgument):
+        (JSC::DirectArguments::copyToArguments):
+        (JSC::DirectArguments::overridesSize):
+        (JSC::DirectArguments::overrideArgument): Deleted.
+        * runtime/DirectArguments.h:
+        (JSC::DirectArguments::length):
+        (JSC::DirectArguments::isMappedArgument):
+        (JSC::DirectArguments::isMappedArgumentInDFG):
+        (JSC::DirectArguments::getIndexQuickly):
+        (JSC::DirectArguments::setIndexQuickly):
+        (JSC::DirectArguments::overrodeThings):
+        (JSC::DirectArguments::initModifiedArgumentsDescriptorIfNecessary):
+        (JSC::DirectArguments::setModifiedArgumentDescriptor):
+        (JSC::DirectArguments::isModifiedArgumentDescriptor):
+        (JSC::DirectArguments::offsetOfMappedArguments):
+        (JSC::DirectArguments::offsetOfModifiedArgumentsDescriptor):
+        (JSC::DirectArguments::canAccessIndexQuickly): Deleted.
+        (JSC::DirectArguments::canAccessArgumentIndexQuicklyInDFG): Deleted.
+        (JSC::DirectArguments::offsetOfOverrides): Deleted.
+        * runtime/GenericArguments.h:
+        * runtime/GenericArgumentsInlines.h:
+        (JSC::GenericArguments<Type>::visitChildren):
+        (JSC::GenericArguments<Type>::getOwnPropertySlot):
+        (JSC::GenericArguments<Type>::getOwnPropertySlotByIndex):
+        (JSC::GenericArguments<Type>::getOwnPropertyNames):
+        (JSC::GenericArguments<Type>::put):
+        (JSC::GenericArguments<Type>::putByIndex):
+        (JSC::GenericArguments<Type>::deleteProperty):
+        (JSC::GenericArguments<Type>::deletePropertyByIndex):
+        (JSC::GenericArguments<Type>::defineOwnProperty):
+        (JSC::GenericArguments<Type>::initModifiedArgumentsDescriptor):
+        (JSC::GenericArguments<Type>::initModifiedArgumentsDescriptorIfNecessary):
+        (JSC::GenericArguments<Type>::setModifiedArgumentDescriptor):
+        (JSC::GenericArguments<Type>::isModifiedArgumentDescriptor):
+        (JSC::GenericArguments<Type>::copyToArguments):
+        * runtime/ScopedArguments.cpp:
+        (JSC::ScopedArguments::visitChildren):
+        (JSC::ScopedArguments::unmapArgument):
+        (JSC::ScopedArguments::overrideArgument): Deleted.
+        * runtime/ScopedArguments.h:
+        (JSC::ScopedArguments::isMappedArgument):
+        (JSC::ScopedArguments::isMappedArgumentInDFG):
+        (JSC::ScopedArguments::getIndexQuickly):
+        (JSC::ScopedArguments::setIndexQuickly):
+        (JSC::ScopedArguments::initModifiedArgumentsDescriptorIfNecessary):
+        (JSC::ScopedArguments::setModifiedArgumentDescriptor):
+        (JSC::ScopedArguments::isModifiedArgumentDescriptor):
+        (JSC::ScopedArguments::canAccessIndexQuickly): Deleted.
+        (JSC::ScopedArguments::canAccessArgumentIndexQuicklyInDFG): Deleted.
+
 2016-12-23  Mark Lam  <mark.lam@apple.com>
 
         Using Option::breakOnThrow() shouldn't crash while printing a null CodeBlock.
index 134c3d4..bc62ec1 100644 (file)
@@ -638,7 +638,7 @@ void AccessCase::generateWithGuard(
         fallThrough.append(
             jit.branchTestPtr(
                 CCallHelpers::NonZero,
-                CCallHelpers::Address(baseGPR, DirectArguments::offsetOfOverrides())));
+                CCallHelpers::Address(baseGPR, DirectArguments::offsetOfMappedArguments())));
         jit.load32(
             CCallHelpers::Address(baseGPR, DirectArguments::offsetOfLength()),
             valueRegs.payloadGPR());
index bf87574..82bbe2b 100644 (file)
@@ -6118,7 +6118,7 @@ void SpeculativeJIT::compileGetByValOnDirectArguments(Node* node)
         ExoticObjectMode, JSValueSource(), 0,
         m_jit.branchTestPtr(
             MacroAssembler::NonZero,
-            MacroAssembler::Address(baseReg, DirectArguments::offsetOfOverrides())));
+            MacroAssembler::Address(baseReg, DirectArguments::offsetOfMappedArguments())));
     speculationCheck(
         ExoticObjectMode, JSValueSource(), 0,
         m_jit.branch32(
@@ -6292,7 +6292,7 @@ void SpeculativeJIT::compileGetArrayLength(Node* node)
             ExoticObjectMode, JSValueSource(), 0,
             m_jit.branchTestPtr(
                 MacroAssembler::NonZero,
-                MacroAssembler::Address(baseReg, DirectArguments::offsetOfOverrides())));
+                MacroAssembler::Address(baseReg, DirectArguments::offsetOfMappedArguments())));
         
         m_jit.load32(
             MacroAssembler::Address(baseReg, DirectArguments::offsetOfLength()), resultReg);
@@ -6658,7 +6658,10 @@ void SpeculativeJIT::compileCreateDirectArguments(Node* node)
         JITCompiler::Address(resultGPR, DirectArguments::offsetOfMinCapacity()));
         
     m_jit.storePtr(
-        TrustedImmPtr(0), JITCompiler::Address(resultGPR, DirectArguments::offsetOfOverrides()));
+        TrustedImmPtr(0), JITCompiler::Address(resultGPR, DirectArguments::offsetOfMappedArguments()));
+
+    m_jit.storePtr(
+        TrustedImmPtr(0), JITCompiler::Address(resultGPR, DirectArguments::offsetOfModifiedArgumentsDescriptor()));
     
     if (lengthIsKnown) {
         addSlowPathGenerator(
index 52513dd..7cf9885 100644 (file)
@@ -51,7 +51,8 @@ namespace JSC { namespace FTL {
     macro(DirectArguments_callee, DirectArguments::offsetOfCallee()) \
     macro(DirectArguments_length, DirectArguments::offsetOfLength()) \
     macro(DirectArguments_minCapacity, DirectArguments::offsetOfMinCapacity()) \
-    macro(DirectArguments_overrides, DirectArguments::offsetOfOverrides()) \
+    macro(DirectArguments_mappedArguments, DirectArguments::offsetOfMappedArguments()) \
+    macro(DirectArguments_modifiedArgumentsDescriptor, DirectArguments::offsetOfModifiedArgumentsDescriptor()) \
     macro(GetterSetter_getter, GetterSetter::offsetOfGetter()) \
     macro(GetterSetter_setter, GetterSetter::offsetOfSetter()) \
     macro(JSArrayBufferView_length, JSArrayBufferView::offsetOfLength()) \
index 6043063..70ebf86 100644 (file)
@@ -3140,7 +3140,7 @@ private:
             LValue arguments = lowCell(m_node->child1());
             speculate(
                 ExoticObjectMode, noValue(), nullptr,
-                m_out.notNull(m_out.loadPtr(arguments, m_heaps.DirectArguments_overrides)));
+                m_out.notNull(m_out.loadPtr(arguments, m_heaps.DirectArguments_mappedArguments)));
             setInt32(m_out.load32NonNegative(arguments, m_heaps.DirectArguments_length));
             return;
         }
@@ -3292,7 +3292,7 @@ private:
             
             speculate(
                 ExoticObjectMode, noValue(), nullptr,
-                m_out.notNull(m_out.loadPtr(base, m_heaps.DirectArguments_overrides)));
+                m_out.notNull(m_out.loadPtr(base, m_heaps.DirectArguments_mappedArguments)));
             speculate(
                 ExoticObjectMode, noValue(), nullptr,
                 m_out.aboveOrEqual(
@@ -4088,7 +4088,8 @@ private:
         
         m_out.store32(length.value, fastObject, m_heaps.DirectArguments_length);
         m_out.store32(m_out.constInt32(minCapacity), fastObject, m_heaps.DirectArguments_minCapacity);
-        m_out.storePtr(m_out.intPtrZero, fastObject, m_heaps.DirectArguments_overrides);
+        m_out.storePtr(m_out.intPtrZero, fastObject, m_heaps.DirectArguments_mappedArguments);
+        m_out.storePtr(m_out.intPtrZero, fastObject, m_heaps.DirectArguments_modifiedArgumentsDescriptor);
         
         ValueFromBlock fastResult = m_out.anchor(fastObject);
         m_out.jump(continuation);
index 44735d5..ede03a9 100644 (file)
@@ -1620,13 +1620,13 @@ static bool canAccessArgumentIndexQuickly(JSObject& object, uint32_t index)
     switch (object.structure()->typeInfo().type()) {
     case DirectArgumentsType: {
         DirectArguments* directArguments = jsCast<DirectArguments*>(&object);
-        if (directArguments->canAccessArgumentIndexQuicklyInDFG(index))
+        if (directArguments->isMappedArgumentInDFG(index))
             return true;
         break;
     }
     case ScopedArgumentsType: {
         ScopedArguments* scopedArguments = jsCast<ScopedArguments*>(&object);
-        if (scopedArguments->canAccessArgumentIndexQuicklyInDFG(index))
+        if (scopedArguments->isMappedArgumentInDFG(index))
             return true;
         break;
     }
index 7ea1e20..8dcf49c 100644 (file)
@@ -1458,7 +1458,7 @@ JIT::JumpList JIT::emitDirectArgumentsGetByVal(Instruction*, PatchableJump& badT
     badType = patchableBranch32(NotEqual, scratch, TrustedImm32(DirectArgumentsType));
     
     slowCases.append(branch32(AboveOrEqual, property, Address(base, DirectArguments::offsetOfLength())));
-    slowCases.append(branchTestPtr(NonZero, Address(base, DirectArguments::offsetOfOverrides())));
+    slowCases.append(branchTestPtr(NonZero, Address(base, DirectArguments::offsetOfMappedArguments())));
     
     zeroExtend32ToPtr(property, scratch);
     loadValue(BaseIndex(base, scratch, TimesEight, DirectArguments::storageOffset()), result);
index 881570b..5e31c31 100644 (file)
@@ -86,8 +86,9 @@ DirectArguments* DirectArguments::createByCopying(ExecState* exec)
 size_t DirectArguments::estimatedSize(JSCell* cell)
 {
     DirectArguments* thisObject = jsCast<DirectArguments*>(cell);
-    size_t overridesSize = thisObject->m_overrides ? thisObject->overridesSize() : 0;
-    return Base::estimatedSize(cell) + overridesSize;
+    size_t mappedArgumentsSize = thisObject->m_mappedArguments ? thisObject->mappedArgumentsSize() * sizeof(bool) : 0;
+    size_t modifiedArgumentsSize = thisObject->m_modifiedArgumentsDescriptor ? thisObject->m_length * sizeof(bool) : 0;
+    return Base::estimatedSize(cell) + mappedArgumentsSize + modifiedArgumentsSize;
 }
 
 void DirectArguments::visitChildren(JSCell* thisCell, SlotVisitor& visitor)
@@ -95,12 +96,13 @@ void DirectArguments::visitChildren(JSCell* thisCell, SlotVisitor& visitor)
     DirectArguments* thisObject = static_cast<DirectArguments*>(thisCell);
     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
     Base::visitChildren(thisObject, visitor);
-    
+
     visitor.appendValues(thisObject->storage(), std::max(thisObject->m_length, thisObject->m_minCapacity));
     visitor.append(thisObject->m_callee);
 
-    if (bool* override = thisObject->m_overrides.get())
-        visitor.markAuxiliary(override);
+    if (thisObject->m_mappedArguments)
+        visitor.markAuxiliary(thisObject->m_mappedArguments.get());
+    GenericArguments<DirectArguments>::visitChildren(thisCell, visitor);
 }
 
 Structure* DirectArguments::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
@@ -110,35 +112,35 @@ Structure* DirectArguments::createStructure(VM& vm, JSGlobalObject* globalObject
 
 void DirectArguments::overrideThings(VM& vm)
 {
-    RELEASE_ASSERT(!m_overrides);
+    RELEASE_ASSERT(!m_mappedArguments);
     
     putDirect(vm, vm.propertyNames->length, jsNumber(m_length), DontEnum);
     putDirect(vm, vm.propertyNames->callee, m_callee.get(), DontEnum);
     putDirect(vm, vm.propertyNames->iteratorSymbol, globalObject()->arrayProtoValuesFunction(), DontEnum);
     
-    void* backingStore = vm.heap.tryAllocateAuxiliary(this, overridesSize());
+    void* backingStore = vm.heap.tryAllocateAuxiliary(this, mappedArgumentsSize());
     RELEASE_ASSERT(backingStore);
     bool* overrides = static_cast<bool*>(backingStore);
-    m_overrides.set(vm, this, overrides);
+    m_mappedArguments.set(vm, this, overrides);
     for (unsigned i = m_length; i--;)
         overrides[i] = false;
 }
 
 void DirectArguments::overrideThingsIfNecessary(VM& vm)
 {
-    if (!m_overrides)
+    if (!m_mappedArguments)
         overrideThings(vm);
 }
 
-void DirectArguments::overrideArgument(VM& vm, unsigned index)
+void DirectArguments::unmapArgument(VM& vm, unsigned index)
 {
     overrideThingsIfNecessary(vm);
-    m_overrides.get()[index] = true;
+    m_mappedArguments.get()[index] = true;
 }
 
 void DirectArguments::copyToArguments(ExecState* exec, VirtualRegister firstElementDest, unsigned offset, unsigned length)
 {
-    if (!m_overrides) {
+    if (!m_mappedArguments) {
         unsigned limit = std::min(length + offset, m_length);
         unsigned i;
         VirtualRegister start = firstElementDest - offset;
@@ -152,10 +154,10 @@ void DirectArguments::copyToArguments(ExecState* exec, VirtualRegister firstElem
     GenericArguments::copyToArguments(exec, firstElementDest, offset, length);
 }
 
-unsigned DirectArguments::overridesSize()
+unsigned DirectArguments::mappedArgumentsSize()
 {
     // We always allocate something; in the relatively uncommon case of overriding an empty argument we
-    // still allocate so that m_overrides is non-null. We use that to indicate that the other properties
+    // still allocate so that m_mappedArguments is non-null. We use that to indicate that the other properties
     // (length, etc) are overridden.
     return WTF::roundUpToMultipleOf<8>(m_length ? m_length : 1);
 }
index f02b054..e637a20 100644 (file)
@@ -66,30 +66,30 @@ public:
     
     uint32_t length(ExecState* exec) const
     {
-        if (UNLIKELY(m_overrides))
+        if (UNLIKELY(m_mappedArguments))
             return get(exec, exec->propertyNames().length).toUInt32(exec);
         return m_length;
     }
     
-    bool canAccessIndexQuickly(uint32_t i) const
+    bool isMappedArgument(uint32_t i) const
     {
-        return i < m_length && (!m_overrides || !m_overrides.get()[i]);
+        return i < m_length && (!m_mappedArguments || !m_mappedArguments.get()[i]);
     }
 
-    bool canAccessArgumentIndexQuicklyInDFG(uint32_t i) const
+    bool isMappedArgumentInDFG(uint32_t i) const
     {
         return i < m_length && !overrodeThings();
     }
 
     JSValue getIndexQuickly(uint32_t i) const
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(canAccessIndexQuickly(i));
+        ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
         return const_cast<DirectArguments*>(this)->storage()[i].get();
     }
     
     void setIndexQuickly(VM& vm, uint32_t i, JSValue value)
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(canAccessIndexQuickly(i));
+        ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
         storage()[i].set(vm, this, value);
     }
     
@@ -106,11 +106,26 @@ public:
     }
     
     // Methods intended for use by the GenericArguments mixin.
-    bool overrodeThings() const { return !!m_overrides; }
+    bool overrodeThings() const { return !!m_mappedArguments; }
     void overrideThings(VM&);
     void overrideThingsIfNecessary(VM&);
-    void overrideArgument(VM&, unsigned index);
-    
+    void unmapArgument(VM&, unsigned index);
+
+    void initModifiedArgumentsDescriptorIfNecessary(VM& vm)
+    {
+        GenericArguments<DirectArguments>::initModifiedArgumentsDescriptorIfNecessary(vm, m_length);
+    }
+
+    void setModifiedArgumentDescriptor(VM& vm, unsigned index)
+    {
+        GenericArguments<DirectArguments>::setModifiedArgumentDescriptor(vm, index, m_length);
+    }
+
+    bool isModifiedArgumentDescriptor(unsigned index)
+    {
+        return GenericArguments<DirectArguments>::isModifiedArgumentDescriptor(index, m_length);
+    }
+
     void copyToArguments(ExecState*, VirtualRegister firstElementDest, unsigned offset, unsigned length);
 
     DECLARE_INFO;
@@ -120,7 +135,8 @@ public:
     static ptrdiff_t offsetOfCallee() { return OBJECT_OFFSETOF(DirectArguments, m_callee); }
     static ptrdiff_t offsetOfLength() { return OBJECT_OFFSETOF(DirectArguments, m_length); }
     static ptrdiff_t offsetOfMinCapacity() { return OBJECT_OFFSETOF(DirectArguments, m_minCapacity); }
-    static ptrdiff_t offsetOfOverrides() { return OBJECT_OFFSETOF(DirectArguments, m_overrides); }
+    static ptrdiff_t offsetOfMappedArguments() { return OBJECT_OFFSETOF(DirectArguments, m_mappedArguments); }
+    static ptrdiff_t offsetOfModifiedArgumentsDescriptor() { return OBJECT_OFFSETOF(DirectArguments, m_modifiedArgumentsDescriptor); }
     
     static size_t storageOffset()
     {
@@ -143,12 +159,12 @@ private:
         return bitwise_cast<WriteBarrier<Unknown>*>(bitwise_cast<char*>(this) + storageOffset());
     }
     
-    unsigned overridesSize();
+    unsigned mappedArgumentsSize();
     
     WriteBarrier<JSFunction> m_callee;
     uint32_t m_length; // Always the actual length of captured arguments and never what was stored into the length property.
     uint32_t m_minCapacity; // The max of this and length determines the capacity of this object. It may be the actual capacity, or maybe something smaller. We arrange it this way to be kind to the JITs.
-    AuxiliaryBarrier<bool*> m_overrides; // If non-null, it means that length, callee, and caller are fully materialized properties.
+    AuxiliaryBarrier<bool*> m_mappedArguments; // If non-null, it means that length, callee, and caller are fully materialized properties.
 };
 
 } // namespace JSC
index a9d9c1f..5ba509f 100644 (file)
@@ -43,6 +43,7 @@ protected:
     {
     }
 
+    static void visitChildren(JSCell*, SlotVisitor&);
     static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
     static bool getOwnPropertySlotByIndex(JSObject*, ExecState*, unsigned propertyName, PropertySlot&);
     static void getOwnPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
@@ -52,7 +53,14 @@ protected:
     static bool deletePropertyByIndex(JSCell*, ExecState*, unsigned propertyName);
     static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
     
+    void initModifiedArgumentsDescriptor(VM&, unsigned length);
+    void initModifiedArgumentsDescriptorIfNecessary(VM&, unsigned length);
+    void setModifiedArgumentDescriptor(VM&, unsigned index, unsigned length);
+    bool isModifiedArgumentDescriptor(unsigned index, unsigned length);
+
     void copyToArguments(ExecState*, VirtualRegister firstElementDest, unsigned offset, unsigned length);
+    
+    AuxiliaryBarrier<bool*> m_modifiedArgumentsDescriptor;
 };
 
 } // namespace JSC
index edd6477..db23a81 100644 (file)
 namespace JSC {
 
 template<typename Type>
+void GenericArguments<Type>::visitChildren(JSCell* thisCell, SlotVisitor& visitor)
+{
+    Type* thisObject = static_cast<Type*>(thisCell);
+    ASSERT_GC_OBJECT_INHERITS(thisObject, info());
+    
+    if (thisObject->m_modifiedArgumentsDescriptor)
+        visitor.markAuxiliary(thisObject->m_modifiedArgumentsDescriptor.get());
+}
+
+template<typename Type>
 bool GenericArguments<Type>::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName ident, PropertySlot& slot)
 {
     Type* thisObject = jsCast<Type*>(object);
@@ -52,12 +62,17 @@ bool GenericArguments<Type>::getOwnPropertySlot(JSObject* object, ExecState* exe
     }
     
     std::optional<uint32_t> index = parseIndex(ident);
-    if (index && thisObject->canAccessIndexQuickly(index.value())) {
+    if (index && !thisObject->isModifiedArgumentDescriptor(index.value()) && thisObject->isMappedArgument(index.value())) {
         slot.setValue(thisObject, None, thisObject->getIndexQuickly(index.value()));
         return true;
     }
     
-    return Base::getOwnPropertySlot(thisObject, exec, ident, slot);
+    bool result = Base::getOwnPropertySlot(thisObject, exec, ident, slot);
+    
+    if (index && thisObject->isMappedArgument(index.value()))
+        slot.setValue(thisObject, slot.attributes(), thisObject->getIndexQuickly(index.value()));
+    
+    return result;
 }
 
 template<typename Type>
@@ -65,13 +80,16 @@ bool GenericArguments<Type>::getOwnPropertySlotByIndex(JSObject* object, ExecSta
 {
     Type* thisObject = jsCast<Type*>(object);
     
-    if (thisObject->canAccessIndexQuickly(index)) {
-        JSValue result = thisObject->getIndexQuickly(index);
-        slot.setValue(thisObject, None, result);
+    if (!thisObject->isModifiedArgumentDescriptor(index) && thisObject->isMappedArgument(index)) {
+        slot.setValue(thisObject, None, thisObject->getIndexQuickly(index));
         return true;
     }
     
     bool result = Base::getOwnPropertySlotByIndex(object, exec, index, slot);
+    
+    if (thisObject->isMappedArgument(index))
+        slot.setValue(thisObject, slot.attributes(), thisObject->getIndexQuickly(index));
+    
     return result;
 }
 
@@ -82,7 +100,7 @@ void GenericArguments<Type>::getOwnPropertyNames(JSObject* object, ExecState* ex
 
     if (array.includeStringProperties()) {
         for (unsigned i = 0; i < thisObject->internalLength(); ++i) {
-            if (!thisObject->canAccessIndexQuickly(i))
+            if (!thisObject->isMappedArgument(i))
                 continue;
             array.add(Identifier::from(exec, i));
         }
@@ -118,7 +136,7 @@ bool GenericArguments<Type>::put(JSCell* cell, ExecState* exec, PropertyName ide
         return ordinarySetSlow(exec, thisObject, ident, value, slot.thisValue(), slot.isStrictMode());
     
     std::optional<uint32_t> index = parseIndex(ident);
-    if (index && thisObject->canAccessIndexQuickly(index.value())) {
+    if (index && thisObject->isMappedArgument(index.value())) {
         thisObject->setIndexQuickly(vm, index.value(), value);
         return true;
     }
@@ -131,8 +149,8 @@ bool GenericArguments<Type>::putByIndex(JSCell* cell, ExecState* exec, unsigned
 {
     Type* thisObject = jsCast<Type*>(cell);
     VM& vm = exec->vm();
-    
-    if (thisObject->canAccessIndexQuickly(index)) {
+
+    if (thisObject->isMappedArgument(index)) {
         thisObject->setIndexQuickly(vm, index, value);
         return true;
     }
@@ -153,8 +171,9 @@ bool GenericArguments<Type>::deleteProperty(JSCell* cell, ExecState* exec, Prope
         thisObject->overrideThings(vm);
     
     std::optional<uint32_t> index = parseIndex(ident);
-    if (index && thisObject->canAccessIndexQuickly(index.value())) {
-        thisObject->overrideArgument(vm, index.value());
+    if (index && !thisObject->isModifiedArgumentDescriptor(index.value()) && thisObject->isMappedArgument(index.value())) {
+        thisObject->unmapArgument(vm, index.value());
+        thisObject->setModifiedArgumentDescriptor(vm, index.value());
         return true;
     }
     
@@ -166,12 +185,13 @@ bool GenericArguments<Type>::deletePropertyByIndex(JSCell* cell, ExecState* exec
 {
     Type* thisObject = jsCast<Type*>(cell);
     VM& vm = exec->vm();
-    
-    if (thisObject->canAccessIndexQuickly(index)) {
-        thisObject->overrideArgument(vm, index);
+
+    if (!thisObject->isModifiedArgumentDescriptor(index) && thisObject->isMappedArgument(index)) {
+        thisObject->unmapArgument(vm, index);
+        thisObject->setModifiedArgumentDescriptor(vm, index);
         return true;
     }
-    
+
     return Base::deletePropertyByIndex(cell, exec, index);
 }
 
@@ -187,37 +207,93 @@ bool GenericArguments<Type>::defineOwnProperty(JSObject* object, ExecState* exec
         thisObject->overrideThingsIfNecessary(vm);
     else {
         std::optional<uint32_t> optionalIndex = parseIndex(ident);
-        if (optionalIndex && thisObject->canAccessIndexQuickly(optionalIndex.value())) {
+        if (optionalIndex) {
             uint32_t index = optionalIndex.value();
-            if (!descriptor.isAccessorDescriptor()) {
+            if (!descriptor.isAccessorDescriptor() && thisObject->isMappedArgument(optionalIndex.value())) {
                 // If the property is not deleted and we are using a non-accessor descriptor, then
                 // make sure that the aliased argument sees the value.
                 if (descriptor.value())
                     thisObject->setIndexQuickly(vm, index, descriptor.value());
             
-                // If the property is not deleted and we are using a non-accessor, writable
-                // descriptor, then we are done. The argument continues to be aliased. Note that we
-                // ignore the request to change enumerability. We appear to have always done so, in
-                // cases where the argument was still aliased.
-                // FIXME: https://bugs.webkit.org/show_bug.cgi?id=141952
-                if (descriptor.writable())
+                // If the property is not deleted and we are using a non-accessor, writable,
+                // configurable and enumerable descriptor and isn't modified, then we are done.
+                // The argument continues to be aliased.
+                if (descriptor.writable() && descriptor.configurable() && descriptor.enumerable() && !thisObject->isModifiedArgumentDescriptor(index))
                     return true;
+                
+                if (!thisObject->isModifiedArgumentDescriptor(index)) {
+                    // If it is a new entry, we need to put direct to initialize argument[i] descriptor properly
+                    JSValue value = thisObject->getIndexQuickly(index);
+                    ASSERT(value);
+                    object->putDirectMayBeIndex(exec, ident, value);
+                    
+                    thisObject->setModifiedArgumentDescriptor(vm, index);
+                }
+            }
+            
+            if (thisObject->isMappedArgument(index)) {
+                // Just unmap arguments if its descriptor contains {writable: false}.
+                // Check https://tc39.github.io/ecma262/#sec-createunmappedargumentsobject
+                // and https://tc39.github.io/ecma262/#sec-createmappedargumentsobject to verify that all data
+                // property from arguments object are {writable: true, configurable: true, enumerable: true} by default
+                if ((descriptor.writablePresent() && !descriptor.writable()) || descriptor.isAccessorDescriptor()) {
+                    if (!descriptor.isAccessorDescriptor()) {
+                        JSValue value = thisObject->getIndexQuickly(index);
+                        ASSERT(value);
+                        object->putDirectMayBeIndex(exec, ident, value);
+                    }
+                    thisObject->unmapArgument(vm, index);
+                    thisObject->setModifiedArgumentDescriptor(vm, index);
+                }
             }
-        
-            // If the property is a non-deleted argument, then move it into the base object and
-            // then delete it.
-            JSValue value = thisObject->getIndexQuickly(index);
-            ASSERT(value);
-            object->putDirectMayBeIndex(exec, ident, value);
-            thisObject->overrideArgument(vm, index);
         }
     }
-    
+
     // Now just let the normal object machinery do its thing.
     return Base::defineOwnProperty(object, exec, ident, descriptor, shouldThrow);
 }
 
 template<typename Type>
+void GenericArguments<Type>::initModifiedArgumentsDescriptor(VM& vm, unsigned argsLength)
+{
+    RELEASE_ASSERT(!m_modifiedArgumentsDescriptor);
+
+    if (argsLength) {
+        void* backingStore = vm.heap.tryAllocateAuxiliary(this, WTF::roundUpToMultipleOf<8>(argsLength));
+        RELEASE_ASSERT(backingStore);
+        bool* modifiedArguments = static_cast<bool*>(backingStore);
+        m_modifiedArgumentsDescriptor.set(vm, this, modifiedArguments);
+        for (unsigned i = argsLength; i--;)
+            modifiedArguments[i] = false;
+    }
+}
+
+template<typename Type>
+void GenericArguments<Type>::initModifiedArgumentsDescriptorIfNecessary(VM& vm, unsigned argsLength)
+{
+    if (!m_modifiedArgumentsDescriptor)
+        initModifiedArgumentsDescriptor(vm, argsLength);
+}
+
+template<typename Type>
+void GenericArguments<Type>::setModifiedArgumentDescriptor(VM& vm, unsigned index, unsigned length)
+{
+    initModifiedArgumentsDescriptorIfNecessary(vm, length);
+    if (index < length)
+        m_modifiedArgumentsDescriptor.get()[index] = true;
+}
+
+template<typename Type>
+bool GenericArguments<Type>::isModifiedArgumentDescriptor(unsigned index, unsigned length)
+{
+    if (!m_modifiedArgumentsDescriptor)
+        return false;
+    if (index < length)
+        return m_modifiedArgumentsDescriptor.get()[index];
+    return false;
+}
+
+template<typename Type>
 void GenericArguments<Type>::copyToArguments(ExecState* exec, VirtualRegister firstElementDest, unsigned offset, unsigned length)
 {
     VM& vm = exec->vm();
@@ -225,7 +301,7 @@ void GenericArguments<Type>::copyToArguments(ExecState* exec, VirtualRegister fi
 
     Type* thisObject = static_cast<Type*>(this);
     for (unsigned i = 0; i < length; ++i) {
-        if (thisObject->canAccessIndexQuickly(i + offset))
+        if (thisObject->isMappedArgument(i + offset))
             exec->r(firstElementDest + i) = thisObject->getIndexQuickly(i + offset);
         else {
             exec->r(firstElementDest + i) = get(exec, i + offset);
index f8eb6ec..97e9939 100644 (file)
@@ -111,6 +111,8 @@ void ScopedArguments::visitChildren(JSCell* cell, SlotVisitor& visitor)
         visitor.appendValues(
             thisObject->overflowStorage(), thisObject->m_totalLength - thisObject->m_table->length());
     }
+
+    GenericArguments<ScopedArguments>::visitChildren(cell, visitor);
 }
 
 Structure* ScopedArguments::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
@@ -135,7 +137,7 @@ void ScopedArguments::overrideThingsIfNecessary(VM& vm)
         overrideThings(vm);
 }
 
-void ScopedArguments::overrideArgument(VM& vm, uint32_t i)
+void ScopedArguments::unmapArgument(VM& vm, uint32_t i)
 {
     ASSERT_WITH_SECURITY_IMPLICATION(i < m_totalLength);
     unsigned namedLength = m_table->length();
index 8f04624..da96005 100644 (file)
@@ -70,7 +70,7 @@ public:
         return internalLength();
     }
     
-    bool canAccessIndexQuickly(uint32_t i) const
+    bool isMappedArgument(uint32_t i) const
     {
         if (i >= m_totalLength)
             return false;
@@ -80,14 +80,14 @@ public:
         return !!overflowStorage()[i - namedLength].get();
     }
 
-    bool canAccessArgumentIndexQuicklyInDFG(uint32_t i) const
+    bool isMappedArgumentInDFG(uint32_t i) const
     {
-        return canAccessIndexQuickly(i);
+        return isMappedArgument(i);
     }
     
     JSValue getIndexQuickly(uint32_t i) const
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(canAccessIndexQuickly(i));
+        ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
         unsigned namedLength = m_table->length();
         if (i < namedLength)
             return m_scope->variableAt(m_table->get(i)).get();
@@ -96,7 +96,7 @@ public:
 
     void setIndexQuickly(VM& vm, uint32_t i, JSValue value)
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(canAccessIndexQuickly(i));
+        ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
         unsigned namedLength = m_table->length();
         if (i < namedLength)
             m_scope->variableAt(m_table->get(i)).set(vm, m_scope.get(), value);
@@ -108,12 +108,27 @@ public:
     {
         return m_callee;
     }
-    
+
     bool overrodeThings() const { return m_overrodeThings; }
     void overrideThings(VM&);
     void overrideThingsIfNecessary(VM&);
-    void overrideArgument(VM&, uint32_t index);
+    void unmapArgument(VM&, uint32_t index);
     
+    void initModifiedArgumentsDescriptorIfNecessary(VM& vm)
+    {
+        GenericArguments<ScopedArguments>::initModifiedArgumentsDescriptorIfNecessary(vm, m_table->length());
+    }
+
+    void setModifiedArgumentDescriptor(VM& vm, unsigned index)
+    {
+        GenericArguments<ScopedArguments>::setModifiedArgumentDescriptor(vm, index, m_table->length());
+    }
+
+    bool isModifiedArgumentDescriptor(unsigned index)
+    {
+        return GenericArguments<ScopedArguments>::isModifiedArgumentDescriptor(index, m_table->length());
+    }
+
     void copyToArguments(ExecState*, VirtualRegister firstElementDest, unsigned offset, unsigned length);
 
     DECLARE_INFO;