Attachment 291932 Details for Bug 159398 – Patch

[patch] Patch

bug-159398-20161018094057.patch (text/plain), 31.44 KB, created by Caio Lima on 2016-10-18 01:44:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2016-10-18 01:44:42 PDT

Size: 31.44 KB

patch

obsolete

>Subversion Revision: 207435
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 53b4f4c92f888fa30dce502736dec90043ed6313..39b8553fe491b36f70cb55475e33a3e81a471441 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,75 @@
>+2016-10-18  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [test262] Fixing mapped arguments object property test case
>+        https://bugs.webkit.org/show_bug.cgi?id=159398
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch fixes 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.
>+
>+        To fix the issues, we created an auxilary boolean array named
>+        m_modifiedArguments to store wich arguments[i] descriptor was changed
>+        from its default configuration. This modification was necessary because m_overrides
>+        was responsible to check it, using canAccessIndexQuickly, at the same time
>+        of keeping information about arguments mapping. The problem of this apporach was
>+        that we need 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.
>+        The second issue was solved with the isModifiedArgument(i). It is used
>+        to check if arguments[i] property descriptor is different from defaut
>+        and if it returns true, we fallback all operations (e.g.
>+        deleteProperty, deletePropertyByIndex, put, putByIndex, etc) to Base.
>+        This approach also fixed
>+        arguments-bizarre-behaviour-disable-enumerability test case.
>+
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileCreateDirectArguments):
>+        * ftl/FTLAbstractHeapRepository.h:
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::compileCreateDirectArguments):
>+        * runtime/DirectArguments.cpp:
>+        (JSC::DirectArguments::estimatedSize):
>+        (JSC::DirectArguments::visitChildren):
>+        * runtime/DirectArguments.h:
>+        (JSC::DirectArguments::initModifiedArgumentsIfNecessary):
>+        (JSC::DirectArguments::setModified):
>+        (JSC::DirectArguments::isModifiedArgument):
>+        (JSC::DirectArguments::offsetOfModifiedArguments):
>+        * runtime/GenericArguments.h:
>+        * runtime/GenericArgumentsInlines.h:
>+        (JSC::GenericArguments<Type>::getOwnPropertySlot):
>+        (JSC::GenericArguments<Type>::getOwnPropertySlotByIndex):
>+        (JSC::GenericArguments<Type>::deleteProperty):
>+        (JSC::GenericArguments<Type>::deletePropertyByIndex):
>+        (JSC::GenericArguments<Type>::defineOwnProperty):
>+        (JSC::GenericArguments<Type>::initModifiedArguments):
>+        (JSC::GenericArguments<Type>::initModifiedArgumentsIfNecessary):
>+        (JSC::GenericArguments<Type>::setModified):
>+        (JSC::GenericArguments<Type>::isModifiedArgument):
>+        * runtime/ScopedArguments.cpp:
>+        (JSC::ScopedArguments::visitChildren):
>+        * runtime/ScopedArguments.h:
>+        (JSC::ScopedArguments::initModifiedArgumentsIfNecessary):
>+        (JSC::ScopedArguments::setModified):
>+        (JSC::ScopedArguments::isModifiedArgument):
>+
> 2016-10-17  Filip Pizlo  <fpizlo@apple.com>
> 
>         Air::IRC doesn't need to have a special-case for uncolored Tmps since they all get colored
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>index ae845639d6ccff460d28d1ce4d8c96f25d57aa41..d78dd0ba6e1fb120efbee0d452ab4ba4e44d258a 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>@@ -6647,6 +6647,9 @@ void SpeculativeJIT::compileCreateDirectArguments(Node* node)
>         
>     m_jit.storePtr(
>         TrustedImmPtr(0), JITCompiler::Address(resultGPR, DirectArguments::offsetOfOverrides()));
>+
>+    m_jit.storePtr(
>+        TrustedImmPtr(0), JITCompiler::Address(resultGPR, DirectArguments::offsetOfModifiedArguments()));
>         
>     if (lengthIsKnown) {
>         addSlowPathGenerator(
>diff --git a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>index fe6b1c620444788781616a74cbfec826095381a9..3e5af7797175bbf93e3e9447d85889ea2d8ad28b 100644
>--- a/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>+++ b/Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
>@@ -51,6 +51,7 @@ namespace JSC { namespace FTL {
>     macro(DirectArguments_length, DirectArguments::offsetOfLength()) \
>     macro(DirectArguments_minCapacity, DirectArguments::offsetOfMinCapacity()) \
>     macro(DirectArguments_overrides, DirectArguments::offsetOfOverrides()) \
>+    macro(DirectArguments_modifiedArguments, DirectArguments::offsetOfModifiedArguments()) \
>     macro(GetterSetter_getter, GetterSetter::offsetOfGetter()) \
>     macro(GetterSetter_setter, GetterSetter::offsetOfSetter()) \
>     macro(JSArrayBufferView_length, JSArrayBufferView::offsetOfLength()) \
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index 8f3550dc86325e0acfb5b7946ca82c028403cd1b..9f438d1c3ac00b3a7f67cdc04ca714eac000441d 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -4051,6 +4051,7 @@ 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_modifiedArguments);
>         
>         ValueFromBlock fastResult = m_out.anchor(fastObject);
>         m_out.jump(continuation);
>diff --git a/Source/JavaScriptCore/runtime/DirectArguments.cpp b/Source/JavaScriptCore/runtime/DirectArguments.cpp
>index 7a0d274b13de34f0c01b29ee51c28c9dd9682a64..15d7ab3adef4b67977ad6fc656b92873fe198e45 100644
>--- a/Source/JavaScriptCore/runtime/DirectArguments.cpp
>+++ b/Source/JavaScriptCore/runtime/DirectArguments.cpp
>@@ -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 overridesSize = thisObject->m_overrides ? thisObject->overridesSize() * sizeof(bool) : 0;
>+    size_t modifiedArgumentsSize = thisObject->m_modifiedArguments ? thisObject->m_length * sizeof(bool) : 0;
>+    return Base::estimatedSize(cell) + overridesSize + modifiedArgumentsSize;
> }
> 
> void DirectArguments::visitChildren(JSCell* thisCell, SlotVisitor& visitor)
>@@ -101,6 +102,8 @@ void DirectArguments::visitChildren(JSCell* thisCell, SlotVisitor& visitor)
>     
>     if (thisObject->m_overrides)
>         visitor.markAuxiliary(thisObject->m_overrides.get());
>+    if (thisObject->m_modifiedArguments)
>+        visitor.markAuxiliary(thisObject->m_modifiedArguments.get());
> }
> 
> Structure* DirectArguments::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
>diff --git a/Source/JavaScriptCore/runtime/DirectArguments.h b/Source/JavaScriptCore/runtime/DirectArguments.h
>index f02b0546684b748551b5c9a17a5534b3926d6e55..095dc7c33b1b7743f92efc7da8b380cc88ce2ffd 100644
>--- a/Source/JavaScriptCore/runtime/DirectArguments.h
>+++ b/Source/JavaScriptCore/runtime/DirectArguments.h
>@@ -110,7 +110,22 @@ public:
>     void overrideThings(VM&);
>     void overrideThingsIfNecessary(VM&);
>     void overrideArgument(VM&, unsigned index);
>-    
>+
>+    void initModifiedArgumentsIfNecessary(VM& vm)
>+    {
>+        GenericArguments<DirectArguments>::initModifiedArgumentsIfNecessary(vm, m_length);
>+    }
>+
>+    void setModified(VM& vm, unsigned index)
>+    {
>+        GenericArguments<DirectArguments>::setModified(vm, index, m_length);
>+    }
>+
>+    bool isModifiedArgument(unsigned index)
>+    {
>+        return GenericArguments<DirectArguments>::isModifiedArgument(index, m_length);
>+    }
>+
>     void copyToArguments(ExecState*, VirtualRegister firstElementDest, unsigned offset, unsigned length);
> 
>     DECLARE_INFO;
>@@ -121,6 +136,7 @@ public:
>     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 offsetOfModifiedArguments() { return OBJECT_OFFSETOF(DirectArguments, m_modifiedArguments); }
>     
>     static size_t storageOffset()
>     {
>diff --git a/Source/JavaScriptCore/runtime/GenericArguments.h b/Source/JavaScriptCore/runtime/GenericArguments.h
>index a9d9c1fb1c4a96303e8cb7bbef18a372213c20a5..ec5ce05b5451ff04063a2303c1b0a826846b5f08 100644
>--- a/Source/JavaScriptCore/runtime/GenericArguments.h
>+++ b/Source/JavaScriptCore/runtime/GenericArguments.h
>@@ -52,7 +52,14 @@ protected:
>     static bool deletePropertyByIndex(JSCell*, ExecState*, unsigned propertyName);
>     static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
>     
>+    void initModifiedArguments(VM&, unsigned length);
>+    void initModifiedArgumentsIfNecessary(VM&, unsigned length);
>+    void setModified(VM&, unsigned index, unsigned length);
>+    bool isModifiedArgument(unsigned index, unsigned length);
>+
>     void copyToArguments(ExecState*, VirtualRegister firstElementDest, unsigned offset, unsigned length);
>+    
>+    AuxiliaryBarrier<bool*> m_modifiedArguments;
> };
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/runtime/GenericArgumentsInlines.h b/Source/JavaScriptCore/runtime/GenericArgumentsInlines.h
>index 1e1eed751015d82b3c23d162e1a672360175839a..7697f0791ea6fd834d9134716046b2226b1e4df9 100644
>--- a/Source/JavaScriptCore/runtime/GenericArgumentsInlines.h
>+++ b/Source/JavaScriptCore/runtime/GenericArgumentsInlines.h
>@@ -52,12 +52,17 @@ bool GenericArguments<Type>::getOwnPropertySlot(JSObject* object, ExecState* exe
>     }
>     
>     Optional<uint32_t> index = parseIndex(ident);
>-    if (index && thisObject->canAccessIndexQuickly(index.value())) {
>+    if (index && !thisObject->isModifiedArgument(index.value()) && thisObject->canAccessIndexQuickly(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->canAccessIndexQuickly(index.value()))
>+        slot.setValue(thisObject, slot.attributes(), thisObject->getIndexQuickly(index.value()));
>+    
>+    return result;
> }
> 
> template<typename Type>
>@@ -65,12 +70,17 @@ bool GenericArguments<Type>::getOwnPropertySlotByIndex(JSObject* object, ExecSta
> {
>     Type* thisObject = jsCast<Type*>(object);
>     
>-    if (thisObject->canAccessIndexQuickly(index)) {
>+    if (!thisObject->isModifiedArgument(index) && thisObject->canAccessIndexQuickly(index)) {
>         slot.setValue(thisObject, None, thisObject->getIndexQuickly(index));
>         return true;
>     }
>     
>-    return Base::getOwnPropertySlotByIndex(object, exec, index, slot);
>+    bool result = Base::getOwnPropertySlotByIndex(object, exec, index, slot);
>+    
>+    if (thisObject->canAccessIndexQuickly(index))
>+        slot.setValue(thisObject, slot.attributes(), thisObject->getIndexQuickly(index));
>+    
>+    return result;
> }
> 
> template<typename Type>
>@@ -149,10 +159,11 @@ bool GenericArguments<Type>::deleteProperty(JSCell* cell, ExecState* exec, Prope
>             || ident == vm.propertyNames->callee
>             || ident == vm.propertyNames->iteratorSymbol))
>         thisObject->overrideThings(vm);
>-    
>+
>     Optional<uint32_t> index = parseIndex(ident);
>-    if (index && thisObject->canAccessIndexQuickly(index.value())) {
>+    if (index && !thisObject->isModifiedArgument(index.value()) && thisObject->canAccessIndexQuickly(index.value())) {
>         thisObject->overrideArgument(vm, index.value());
>+        thisObject->setModified(vm, index.value());
>         return true;
>     }
>     
>@@ -164,12 +175,13 @@ bool GenericArguments<Type>::deletePropertyByIndex(JSCell* cell, ExecState* exec
> {
>     Type* thisObject = jsCast<Type*>(cell);
>     VM& vm = exec->vm();
>-    
>-    if (thisObject->canAccessIndexQuickly(index)) {
>+
>+    if (!thisObject->isModifiedArgument(index) && thisObject->canAccessIndexQuickly(index)) {
>         thisObject->overrideArgument(vm, index);
>+        thisObject->setModified(vm, index);
>         return true;
>     }
>-    
>+
>     return Base::deletePropertyByIndex(cell, exec, index);
> }
> 
>@@ -185,37 +197,95 @@ bool GenericArguments<Type>::defineOwnProperty(JSObject* object, ExecState* exec
>         thisObject->overrideThingsIfNecessary(vm);
>     else {
>         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->canAccessIndexQuickly(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->isModifiedArgument(index))
>                     return true;
>+                
>+                if (!thisObject->isModifiedArgument(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->setModified(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 (optionalIndex && thisObject->canAccessIndexQuickly(optionalIndex.value())) {
>+            uint32_t index = optionalIndex.value();
>+            
>+            // Just overrride arguments (i.e stop mapping) 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->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>::initModifiedArguments(VM& vm, unsigned argsLength)
>+{
>+    RELEASE_ASSERT(!m_modifiedArguments);
>+
>+    if (argsLength) {
>+        void* backingStore = vm.heap.tryAllocateAuxiliary(this, WTF::roundUpToMultipleOf<8>(argsLength));
>+        RELEASE_ASSERT(backingStore);
>+        bool* modifiedArguments = static_cast<bool*>(backingStore);
>+        m_modifiedArguments.set(vm, this, modifiedArguments);
>+        for (unsigned i = argsLength; i--;)
>+            modifiedArguments[i] = false;
>+    }
>+}
>+
>+template<typename Type>
>+void GenericArguments<Type>::initModifiedArgumentsIfNecessary(VM& vm, unsigned argsLength)
>+{
>+    if (!m_modifiedArguments)
>+        initModifiedArguments(vm, argsLength);
>+}
>+
>+template<typename Type>
>+void GenericArguments<Type>::setModified(VM& vm, unsigned index, unsigned length)
>+{
>+    initModifiedArgumentsIfNecessary(vm, length);
>+    if (index < length)
>+        m_modifiedArguments.get()[index] = true;
>+}
>+
>+template<typename Type>
>+bool GenericArguments<Type>::isModifiedArgument(unsigned index, unsigned length)
>+{
>+    if (!m_modifiedArguments)
>+        return false;
>+    if (index < length)
>+        return m_modifiedArguments.get()[index];
>+    return false;
>+}
>+
>+template<typename Type>
> void GenericArguments<Type>::copyToArguments(ExecState* exec, VirtualRegister firstElementDest, unsigned offset, unsigned length)
> {
>     VM& vm = exec->vm();
>diff --git a/Source/JavaScriptCore/runtime/PropertyDescriptor.h b/Source/JavaScriptCore/runtime/PropertyDescriptor.h
>index ce463e1d4871832725dc0634c9e79bbdc549134b..4fa9c8e3e1b6ae534da14412510adb3186c7ea68 100644
>--- a/Source/JavaScriptCore/runtime/PropertyDescriptor.h
>+++ b/Source/JavaScriptCore/runtime/PropertyDescriptor.h
>@@ -79,6 +79,7 @@ public:
>     bool getterPresent() const { return !!m_getter; }
>     bool equalTo(ExecState*, const PropertyDescriptor& other) const;
>     bool attributesEqual(const PropertyDescriptor& other) const;
>+
>     unsigned attributesOverridingCurrent(const PropertyDescriptor& current) const;
> 
> private:
>diff --git a/Source/JavaScriptCore/runtime/ScopedArguments.cpp b/Source/JavaScriptCore/runtime/ScopedArguments.cpp
>index a5a2fc75b31bad99dc1af6151b3c7975523d750a..2a7aefa8d3d383eea33b8d9a14eae0b224599856 100644
>--- a/Source/JavaScriptCore/runtime/ScopedArguments.cpp
>+++ b/Source/JavaScriptCore/runtime/ScopedArguments.cpp
>@@ -111,6 +111,9 @@ void ScopedArguments::visitChildren(JSCell* cell, SlotVisitor& visitor)
>         visitor.appendValues(
>             thisObject->overflowStorage(), thisObject->m_totalLength - thisObject->m_table->length());
>     }
>+
>+    if (thisObject->m_modifiedArguments)
>+        visitor.markAuxiliary(thisObject->m_modifiedArguments.get());
> }
> 
> Structure* ScopedArguments::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
>diff --git a/Source/JavaScriptCore/runtime/ScopedArguments.h b/Source/JavaScriptCore/runtime/ScopedArguments.h
>index 8f046247b9236ce9c59ac7de249867dccf8c0cbc..afbc33b8dc5457aa12e7e7057eb6e221d39372f0 100644
>--- a/Source/JavaScriptCore/runtime/ScopedArguments.h
>+++ b/Source/JavaScriptCore/runtime/ScopedArguments.h
>@@ -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 initModifiedArgumentsIfNecessary(VM& vm)
>+    {
>+        GenericArguments<ScopedArguments>::initModifiedArgumentsIfNecessary(vm, m_table->length());
>+    }
>+
>+    void setModified(VM& vm, unsigned index)
>+    {
>+        GenericArguments<ScopedArguments>::setModified(vm, index, m_table->length());
>+    }
>+
>+    bool isModifiedArgument(unsigned index)
>+    {
>+        return GenericArguments<ScopedArguments>::isModifiedArgument(index, m_table->length());
>+    }
>+
>     void copyToArguments(ExecState*, VirtualRegister firstElementDest, unsigned offset, unsigned length);
> 
>     DECLARE_INFO;
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index deca5dd9faa110f90d6d26bdfaf1a843184522c5..964f027bc85ea4f3cf760d93ca04c9da5b8e60fe 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,22 @@
>+2016-10-18  Caio Lima  <ticaiolima@gmail.com>
>+
>+        [test262] Fixing mapped arguments object property test case
>+        https://bugs.webkit.org/show_bug.cgi?id=159398
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/arguments-bizarre-behaviour-disable-enumerability.js: Fixed
>+        bizarre behavior.
>+        * stress/arguments-define-property.js: Added.
>+        (assert):
>+        (testProperties):
>+        * stress/arguments-non-configurable.js: Added.
>+        (assert):
>+        (tryChangeNonConfigurableDescriptor):
>+        (set tryChangeNonConfigurableDescriptor):
>+        (tryChangeWritableOfNonConfigurableDescriptor):
>+        * test262.yaml:
>+
> 2016-10-17  Saam Barati  <sbarati@apple.com>
> 
>         Add more tests for the double->String conversion in ValueAdd constant folding
>diff --git a/JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js b/JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js
>index 947850a2001ab89665e50bf33d875c293956560d..ab5735f3d221e7998adbecd069cd5980b24e11f2 100644
>--- a/JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js
>+++ b/JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js
>@@ -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
>index 0000000000000000000000000000000000000000..58632be8420b6f2b910e1416ffe4fc10f7462b81
>--- /dev/null
>+++ b/JSTests/stress/arguments-define-property.js
>@@ -0,0 +1,35 @@
>+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
>index 0000000000000000000000000000000000000000..c5a055fcc8268d811308b1cbd8238a3c924eb5b1
>--- /dev/null
>+++ b/JSTests/stress/arguments-non-configurable.js
>@@ -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");
>+
>diff --git a/JSTests/test262.yaml b/JSTests/test262.yaml
>index 3b51b60aa9c3330d5ec82244954d568cd6211072..8a023aea349c03649753d5d99100249e22ffa076 100644
>--- a/JSTests/test262.yaml
>+++ b/JSTests/test262.yaml
>@@ -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

Actions: View | Formatted Diff | Diff

Attachments on bug 159398: 282740 | 282742 | 282743 | 282744 | 282745 | 283286 | 283499 | 283504 | 283505 | 283509 | 283742 | 283766 | 284980 | 284981 | 284982 | 284983 | 284984 | 284985 | 289955 | 289969 | 289970 | 289975 | 289979 | 290296 | 290303 | 290305 | 290306 | 290326 | 290344 | 290350 | 290429 | 290583 | 291051 | 291932 | 294029 | 295654 | 297500 | 297526 | 297747