Title: [210146] trunk
Revision
210146
Author
[email protected]
Date
2016-12-24 13:26:22 -0800 (Sat, 24 Dec 2016)

Log Message

[test262] Fixing mapped arguments object property test case
https://bugs.webkit.org/show_bug.cgi?id=159398

Patch by Caio Lima <[email protected]> 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/_javascript_Core:

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.

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (210145 => 210146)


--- trunk/JSTests/ChangeLog	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/JSTests/ChangeLog	2016-12-24 21:26:22 UTC (rev 210146)
@@ -1,3 +1,39 @@
+2016-12-24  Caio Lima  <[email protected]>
+
+        [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  <[email protected]>
+
+        [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  <[email protected]>
 
         WebAssembly: trap on bad division.

Modified: trunk/JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js (210145 => 210146)


--- trunk/JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js	2016-12-24 21:26:22 UTC (rev 210146)
@@ -24,7 +24,5 @@
 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();

Added: trunk/JSTests/stress/arguments-define-property.js (0 => 210146)


--- trunk/JSTests/stress/arguments-define-property.js	                        (rev 0)
+++ trunk/JSTests/stress/arguments-define-property.js	2016-12-24 21:26:22 UTC (rev 210146)
@@ -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);
+

Added: trunk/JSTests/stress/arguments-non-configurable.js (0 => 210146)


--- trunk/JSTests/stress/arguments-non-configurable.js	                        (rev 0)
+++ trunk/JSTests/stress/arguments-non-configurable.js	2016-12-24 21:26:22 UTC (rev 210146)
@@ -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");
+

Modified: trunk/JSTests/test262.yaml (210145 => 210146)


--- trunk/JSTests/test262.yaml	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/JSTests/test262.yaml	2016-12-24 21:26:22 UTC (rev 210146)
@@ -51364,37 +51364,37 @@
 - 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

Modified: trunk/Source/_javascript_Core/ChangeLog (210145 => 210146)


--- trunk/Source/_javascript_Core/ChangeLog	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-12-24 21:26:22 UTC (rev 210146)
@@ -1,3 +1,116 @@
+2016-12-24  Caio Lima  <[email protected]>
+
+        [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  <[email protected]>
 
         Using Option::breakOnThrow() shouldn't crash while printing a null CodeBlock.

Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp (210145 => 210146)


--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp	2016-12-24 21:26:22 UTC (rev 210146)
@@ -638,7 +638,7 @@
         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());

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (210145 => 210146)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2016-12-24 21:26:22 UTC (rev 210146)
@@ -6118,7 +6118,7 @@
         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 @@
             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 @@
         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(

Modified: trunk/Source/_javascript_Core/ftl/FTLAbstractHeapRepository.h (210145 => 210146)


--- trunk/Source/_javascript_Core/ftl/FTLAbstractHeapRepository.h	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/ftl/FTLAbstractHeapRepository.h	2016-12-24 21:26:22 UTC (rev 210146)
@@ -51,7 +51,8 @@
     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()) \

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (210145 => 210146)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2016-12-24 21:26:22 UTC (rev 210146)
@@ -3140,7 +3140,7 @@
             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 @@
             
             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 @@
         
         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);

Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (210145 => 210146)


--- trunk/Source/_javascript_Core/jit/JITOperations.cpp	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp	2016-12-24 21:26:22 UTC (rev 210146)
@@ -1620,13 +1620,13 @@
     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;
     }

Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (210145 => 210146)


--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2016-12-24 21:26:22 UTC (rev 210146)
@@ -1458,7 +1458,7 @@
     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);

Modified: trunk/Source/_javascript_Core/runtime/DirectArguments.cpp (210145 => 210146)


--- trunk/Source/_javascript_Core/runtime/DirectArguments.cpp	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/runtime/DirectArguments.cpp	2016-12-24 21:26:22 UTC (rev 210146)
@@ -86,8 +86,9 @@
 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 @@
     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,16 +112,16 @@
 
 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;
 }
@@ -126,19 +128,19 @@
 
 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 @@
     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);
 }

Modified: trunk/Source/_javascript_Core/runtime/DirectArguments.h (210145 => 210146)


--- trunk/Source/_javascript_Core/runtime/DirectArguments.h	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/runtime/DirectArguments.h	2016-12-24 21:26:22 UTC (rev 210146)
@@ -66,17 +66,17 @@
     
     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();
     }
@@ -83,13 +83,13 @@
 
     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 @@
     }
     
     // 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 @@
     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 @@
         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

Modified: trunk/Source/_javascript_Core/runtime/GenericArguments.h (210145 => 210146)


--- trunk/Source/_javascript_Core/runtime/GenericArguments.h	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/runtime/GenericArguments.h	2016-12-24 21:26:22 UTC (rev 210146)
@@ -43,6 +43,7 @@
     {
     }
 
+    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 @@
     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

Modified: trunk/Source/_javascript_Core/runtime/GenericArgumentsInlines.h (210145 => 210146)


--- trunk/Source/_javascript_Core/runtime/GenericArgumentsInlines.h	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/runtime/GenericArgumentsInlines.h	2016-12-24 21:26:22 UTC (rev 210146)
@@ -31,6 +31,16 @@
 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 @@
     }
     
     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 @@
 {
     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 @@
 
     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 @@
         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 @@
 {
     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 @@
         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 @@
 {
     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 @@
         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 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);
+            
+            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);
+                }
+            }
         }
     }
-    
+
     // 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 @@
 
     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);

Modified: trunk/Source/_javascript_Core/runtime/ScopedArguments.cpp (210145 => 210146)


--- trunk/Source/_javascript_Core/runtime/ScopedArguments.cpp	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/runtime/ScopedArguments.cpp	2016-12-24 21:26:22 UTC (rev 210146)
@@ -111,6 +111,8 @@
         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 @@
         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();

Modified: trunk/Source/_javascript_Core/runtime/ScopedArguments.h (210145 => 210146)


--- trunk/Source/_javascript_Core/runtime/ScopedArguments.h	2016-12-24 18:00:00 UTC (rev 210145)
+++ trunk/Source/_javascript_Core/runtime/ScopedArguments.h	2016-12-24 21:26:22 UTC (rev 210146)
@@ -70,7 +70,7 @@
         return internalLength();
     }
     
-    bool canAccessIndexQuickly(uint32_t i) const
+    bool isMappedArgument(uint32_t i) const
     {
         if (i >= m_totalLength)
             return false;
@@ -80,14 +80,14 @@
         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 @@
 
     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 @@
     {
         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;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to