Changeset 197295 in webkit for trunk/Source/JavaScriptCore/runtime

Timestamp:

Feb 28, 2016, 10:40:35 AM (9 years ago)

Author:

[email protected]

Message:

ProxyObject.GetOwnProperty is partially broken because it doesn't propagate information back to the slot
​https://bugs.webkit.org/show_bug.cgi?id=154768

Reviewed by Ryosuke Niwa.

This fixes a big bug with ProxyObject.GetOwnProperty:
​http://www.ecma-international.org/ecma-262/6.0/index.html#sec-proxy-object-internal-methods-and-internal-slots-getownproperty-p
We weren't correctly propagating the result of this operation to the
out PropertySlot& parameter. This patch fixes that and adds tests.

• runtime/ObjectConstructor.cpp:

(JSC::objectConstructorGetOwnPropertyDescriptor):
I added a missing exception check after object allocation
because I saw that it was missing while reading the code.

• runtime/PropertyDescriptor.cpp:

(JSC::PropertyDescriptor::setUndefined):
(JSC::PropertyDescriptor::slowGetterSetter):
(JSC::PropertyDescriptor::getter):

• runtime/PropertyDescriptor.h:

(JSC::PropertyDescriptor::attributes):
(JSC::PropertyDescriptor::value):

• runtime/ProxyObject.cpp:

(JSC::ProxyObject::performInternalMethodGetOwnProperty):

• tests/es6.yaml:
• tests/stress/proxy-get-own-property.js:

(let.handler.getOwnPropertyDescriptor):
(set get let.handler.return):
(set get let.handler.getOwnPropertyDescriptor):
(set get let):
(set get let.a):
(let.b):
(let.setter):
(let.getter):

Location:

trunk/Source/JavaScriptCore/runtime

Files:

• ObjectConstructor.cpp (modified) (1 diff)
• PropertyDescriptor.cpp (modified) (1 diff)
• PropertyDescriptor.h (modified) (1 diff)
• ProxyObject.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/runtime/ObjectConstructor.cpp

@@ -233 +233 @@
 
     JSObject* description = constructEmptyObject(exec);
+    if (exec->hadException())
+        return jsUndefined();
     if (!descriptor.isAccessorDescriptor()) {
         description->putDirect(exec->vm(), exec->propertyNames().value, descriptor.value() ? descriptor.value() : jsUndefined(), 0);

trunk/Source/JavaScriptCore/runtime/PropertyDescriptor.cpp

@@ -73 +73 @@
 }
 
+GetterSetter* PropertyDescriptor::slowGetterSetter(ExecState* exec)
+{
+    VM& vm = exec->vm();
+    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
+    GetterSetter* getterSetter = GetterSetter::create(vm, globalObject);
+    if (exec->hadException())
+        return nullptr;
+    if (m_getter && !m_getter.isUndefined())
+        getterSetter->setGetter(vm, globalObject, jsCast<JSObject*>(m_getter));
+    if (m_setter && !m_setter.isUndefined())
+        getterSetter->setSetter(vm, globalObject, jsCast<JSObject*>(m_setter));
+
+    return getterSetter;
+}
+
 JSValue PropertyDescriptor::getter() const
 {

trunk/Source/JavaScriptCore/runtime/PropertyDescriptor.h

@@ -60 +60 @@
     unsigned attributes() const { return m_attributes; }
     JSValue value() const { return m_value; }
+    GetterSetter* slowGetterSetter(ExecState*); // Be aware that this will lazily allocate a GetterSetter object. It's much better to use getter() and setter() individually if possible.
     JS_EXPORT_PRIVATE JSValue getter() const;
     JS_EXPORT_PRIVATE JSValue setter() const;

trunk/Source/JavaScriptCore/runtime/ProxyObject.cpp

@@ -139 +139 @@
 {
     VM& vm = exec->vm();
-    slot.setValue(this, None, jsUndefined()); // We do this to protect against any bad actors. Nobody should depend on this value.
     JSValue handlerValue = this->handler();
     if (handlerValue.isNull()) {
@@ -211 +210 @@
         }
     }
+
+    if (trapResultAsDescriptor.isAccessorDescriptor()) {
+        GetterSetter* getterSetter = trapResultAsDescriptor.slowGetterSetter(exec);
+        if (exec->hadException())
+            return false;
+        slot.setGetterSlot(this, trapResultAsDescriptor.attributes(), getterSetter);
+    } else if (trapResultAsDescriptor.isDataDescriptor())
+        slot.setValue(this, trapResultAsDescriptor.attributes(), trapResultAsDescriptor.value());
+    else
+        slot.setValue(this, trapResultAsDescriptor.attributes(), jsUndefined()); // We use undefined because it's the default value in object properties.
 
     return true;