Changeset 196760 in webkit

Timestamp:

Feb 18, 2016 9:19:33 AM (8 years ago)

Author:

Chris Dumez

Message:

Crash on SES selftest page when loading the page while WebInspector is open
​https://bugs.webkit.org/show_bug.cgi?id=154378
<rdar://problem/24713422>

Reviewed by Mark Lam.

Do a partial revert of r196676 so that JSObject::getOwnPropertyDescriptor()
returns early again if it detects that getOwnPropertySlot() returns a
non-own property. This check was removed in r196676 because we assumed that
only JSDOMWindow::getOwnPropertySlot() could return non-own properties.
However, as it turns out, DebuggerScope::getOwnPropertySlot() does so as
well.

Not having the check would lead to crashes when using the debugger because
we would get a slot with the CustomAccessor attribute but getDirect() would
then fail to return the property (because it is not an own property). We
would then cast the value returned by getDirect() to a CustomGetterSetter*
and dereference it.

• runtime/JSObject.cpp:

(JSC::JSObject::getOwnPropertyDescriptor):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/JSObject.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-02-18  Chris Dumez  <cdumez@apple.com>
+
+        Crash on SES selftest page when loading the page while WebInspector is open
+        https://bugs.webkit.org/show_bug.cgi?id=154378
+        <rdar://problem/24713422>
+
+        Reviewed by Mark Lam.
+
+        Do a partial revert of r196676 so that JSObject::getOwnPropertyDescriptor()
+        returns early again if it detects that getOwnPropertySlot() returns a
+        non-own property. This check was removed in r196676 because we assumed that
+        only JSDOMWindow::getOwnPropertySlot() could return non-own properties.
+        However, as it turns out, DebuggerScope::getOwnPropertySlot() does so as
+        well.
+
+        Not having the check would lead to crashes when using the debugger because
+        we would get a slot with the CustomAccessor attribute but getDirect() would
+        then fail to return the property (because it is not an own property). We
+        would then cast the value returned by getDirect() to a CustomGetterSetter*
+        and dereference it.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::getOwnPropertyDescriptor):
+
 2016-02-18  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -2557 +2557 @@
         return false;
 
+    // DebuggerScope::getOwnPropertySlot() (and possibly others) may return attributes from the prototype chain
+    // but getOwnPropertyDescriptor() should only work for 'own' properties so we exit early if we detect that
+    // the property is not an own property.
+    if (slot.slotBase() != this && slot.slotBase()) {
+        auto* proxy = jsDynamicCast<JSProxy*>(this);
+        if (!proxy || proxy->target() != slot.slotBase())
+            return false;
+    }
+
     if (slot.isAccessor())
         descriptor.setAccessorDescriptor(slot.getterSetter(), slot.attributes());