Changeset 196723 in webkit

Timestamp:

Feb 17, 2016 2:59:59 PM (8 years ago)

Author:

Chris Dumez

Message:

SES selftest page crashes on nightly r196694
​https://bugs.webkit.org/show_bug.cgi?id=154350
<rdar://problem/24704334>

Reviewed by Mark Lam.

Source/JavaScriptCore:

SES selftest page crashes after r196001 / r196145 when calling
Object.getOwnPropertyDescriptor(window, "length") after the window
has been reified and "length" has been shadowed by a value property.

It was crashing in JSObject::getOwnPropertyDescriptor() because
we are getting a slot that has attribute "CustomAccessor" but
the property is not a CustomGetterSetter. In this case, since
window.length is [Replaceable] and has been set to a numeric value,
it makes that the property is not a CustomGetterSetter. However,
the "CustomAccessor" attribute should have been dropped from the
slot when window.length was shadowed. Therefore, this code path
should not be exercised at all when calling
getOwnPropertyDescriptor().

The issue was that putDirectInternal() was updating the slot
attributes only if the "Accessor" flag has changed, but not
the "customAccessor" flag. This patch fixes the issue.

• runtime/JSObject.h:

(JSC::JSObject::putDirectInternal):

LayoutTests:

Add test coverage for the crash which happens when shadowing window.length
with a value after the window property and then calling
Object.getOwnPropertyDescriptor(window, "length").

• js/window-length-getOwnPropertyDescriptor-crash-expected.txt: Added.
• js/window-length-getOwnPropertyDescriptor-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/window-length-getOwnPropertyDescriptor-crash-expected.txt (added)
• LayoutTests/js/window-length-getOwnPropertyDescriptor-crash.html (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSObject.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-02-17  Chris Dumez  <cdumez@apple.com>
+
+        SES selftest page crashes on nightly r196694
+        https://bugs.webkit.org/show_bug.cgi?id=154350
+        <rdar://problem/24704334>
+
+        Reviewed by Mark Lam.
+
+        Add test coverage for the crash which happens when shadowing window.length
+        with a value after the window property and then calling
+        Object.getOwnPropertyDescriptor(window, "length").
+
+        * js/window-length-getOwnPropertyDescriptor-crash-expected.txt: Added.
+        * js/window-length-getOwnPropertyDescriptor-crash.html: Added.
+
 2016-02-17  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-02-17  Chris Dumez  <cdumez@apple.com>
+
+        SES selftest page crashes on nightly r196694
+        https://bugs.webkit.org/show_bug.cgi?id=154350
+        <rdar://problem/24704334>
+
+        Reviewed by Mark Lam.
+
+        SES selftest page crashes after r196001 / r196145 when calling
+        Object.getOwnPropertyDescriptor(window, "length") after the window
+        has been reified and "length" has been shadowed by a value property.
+
+        It was crashing in JSObject::getOwnPropertyDescriptor() because
+        we are getting a slot that has attribute "CustomAccessor" but
+        the property is not a CustomGetterSetter. In this case, since
+        window.length is [Replaceable] and has been set to a numeric value,
+        it makes that the property is not a CustomGetterSetter. However,
+        the "CustomAccessor" attribute should have been dropped from the
+        slot when window.length was shadowed. Therefore, this code path
+        should not be exercised at all when calling
+        getOwnPropertyDescriptor().
+
+        The issue was that putDirectInternal() was updating the slot
+        attributes only if the "Accessor" flag has changed, but not
+        the "customAccessor" flag. This patch fixes the issue.
+
+        * runtime/JSObject.h:
+        (JSC::JSObject::putDirectInternal):
+
 2016-02-17  Saam barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSObject.h

@@ -1229 +1229 @@
             slot.setExistingProperty(this, offset);
 
-            if ((attributes & Accessor) != (currentAttributes & Accessor)) {
+            if ((attributes & Accessor) != (currentAttributes & Accessor) || (attributes & CustomAccessor) != (currentAttributes & CustomAccessor)) {
                 ASSERT(!(attributes & ReadOnly));
                 setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes));