Make assertion in JSObject::putOwnDataProperty more precise
authortzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 1 Oct 2019 04:06:25 +0000 (04:06 +0000)
committertzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 1 Oct 2019 04:06:25 +0000 (04:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202379
<rdar://problem/49515980>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/object-assign-target-proto-setter.js: Added.
(get Object):

Source/JavaScriptCore:

Currently, we assert that the structure has no accessors/custom accessors, but that assertion is
too conservative. All we need to prove is that the property being inserted either does not exist
in the target object or is neither an accessor nor read-only.

* runtime/JSObject.h:
(JSC::JSObject::putOwnDataProperty): Deleted.
(JSC::JSObject::putOwnDataPropertyMayBeIndex): Deleted.
* runtime/JSObjectInlines.h:
(JSC::JSObject::validatePutOwnDataProperty):
(JSC::JSObject::putOwnDataProperty):
(JSC::JSObject::putOwnDataPropertyMayBeIndex):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250543 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/object-assign-target-proto-setter.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/JSObjectInlines.h

index bf8e6ff..5a1e684 100644 (file)
@@ -1,3 +1,14 @@
+2019-09-30  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Make assertion in JSObject::putOwnDataProperty more precise
+        https://bugs.webkit.org/show_bug.cgi?id=202379
+        <rdar://problem/49515980>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/object-assign-target-proto-setter.js: Added.
+        (get Object):
+
 2019-09-30  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] HeapSnapshotBuilder m_rootData should be protected with a lock too
diff --git a/JSTests/stress/object-assign-target-proto-setter.js b/JSTests/stress/object-assign-target-proto-setter.js
new file mode 100644 (file)
index 0000000..7cff2ca
--- /dev/null
@@ -0,0 +1,3 @@
+const x = {};
+Object.defineProperty(x, '__proto__', {get: ()=>{}});
+Object.assign(x, { get: ()=> {} });
index b6fb242..fe39922 100644 (file)
@@ -1,3 +1,23 @@
+2019-09-30  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Make assertion in JSObject::putOwnDataProperty more precise
+        https://bugs.webkit.org/show_bug.cgi?id=202379
+        <rdar://problem/49515980>
+
+        Reviewed by Yusuke Suzuki.
+
+        Currently, we assert that the structure has no accessors/custom accessors, but that assertion is
+        too conservative. All we need to prove is that the property being inserted either does not exist
+        in the target object or is neither an accessor nor read-only.
+
+        * runtime/JSObject.h:
+        (JSC::JSObject::putOwnDataProperty): Deleted.
+        (JSC::JSObject::putOwnDataPropertyMayBeIndex): Deleted.
+        * runtime/JSObjectInlines.h:
+        (JSC::JSObject::validatePutOwnDataProperty):
+        (JSC::JSObject::putOwnDataProperty):
+        (JSC::JSObject::putOwnDataPropertyMayBeIndex):
+
 2019-09-30  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] HeapSnapshotBuilder m_rootData should be protected with a lock too
index d84f3f6..776c1fb 100644 (file)
@@ -727,6 +727,9 @@ public:
     // This is used by JSLexicalEnvironment.
     bool putOwnDataProperty(VM&, PropertyName, JSValue, PutPropertySlot&);
     bool putOwnDataPropertyMayBeIndex(ExecState*, PropertyName, JSValue, PutPropertySlot&);
+private:
+    void validatePutOwnDataProperty(VM&, PropertyName, JSValue);
+public:
 
     // Fast access to known property offsets.
     ALWAYS_INLINE JSValue getDirect(PropertyOffset offset) const { return locationForOffset(offset)->get(); }
@@ -1490,30 +1493,6 @@ inline JSValue JSObject::get(ExecState* exec, unsigned propertyName) const
     return jsUndefined();
 }
 
-inline bool JSObject::putOwnDataProperty(VM& vm, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
-{
-    ASSERT(value);
-    ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(this));
-    ASSERT(!structure(vm)->hasGetterSetterProperties());
-    ASSERT(!structure(vm)->hasCustomGetterSetterProperties());
-
-    return putDirectInternal<PutModePut>(vm, propertyName, value, 0, slot);
-}
-
-inline bool JSObject::putOwnDataPropertyMayBeIndex(ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
-{
-    VM& vm = exec->vm();
-    ASSERT(value);
-    ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(this));
-    ASSERT(!structure(vm)->hasGetterSetterProperties());
-    ASSERT(!structure(vm)->hasCustomGetterSetterProperties());
-
-    if (Optional<uint32_t> index = parseIndex(propertyName))
-        return putDirectIndex(exec, index.value(), value, 0, PutDirectIndexLikePutDirect);
-
-    return putDirectInternal<PutModePut>(vm, propertyName, value, 0, slot);
-}
-
 inline bool JSObject::putDirect(VM& vm, PropertyName propertyName, JSValue value, unsigned attributes)
 {
     ASSERT(!value.isGetterSetter() && !(attributes & PropertyAttribute::Accessor));
index 28183d4..79dd36e 100644 (file)
@@ -462,5 +462,40 @@ inline void JSObject::setIndexQuicklyForTypedArray(unsigned i, JSValue value)
     }
 }
     
+inline void JSObject::validatePutOwnDataProperty(VM& vm, PropertyName propertyName, JSValue value)
+{
+#if !ASSERT_DISABLED
+    ASSERT(value);
+    ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(this));
+    unsigned attributes;
+    PropertyOffset offset = structure(vm)->get(vm, propertyName, attributes);
+    if (isValidOffset(offset))
+        ASSERT(!(attributes & (PropertyAttribute::Accessor | PropertyAttribute::CustomAccessor | PropertyAttribute::ReadOnly)));
+    else if (TypeInfo::hasStaticPropertyTable(inlineTypeFlags())) {
+        if (auto entry = findPropertyHashEntry(vm, propertyName))
+            ASSERT(!(entry->value->attributes() & (PropertyAttribute::Accessor | PropertyAttribute::CustomAccessor | PropertyAttribute::ReadOnly)));
+    }
+#else
+    UNUSED_PARAM(vm);
+    UNUSED_PARAM(propertyName);
+    UNUSED_PARAM(value);
+#endif
+}
+
+inline bool JSObject::putOwnDataProperty(VM& vm, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
+{
+    validatePutOwnDataProperty(vm, propertyName, value);
+    return putDirectInternal<PutModePut>(vm, propertyName, value, 0, slot);
+}
+
+inline bool JSObject::putOwnDataPropertyMayBeIndex(ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
+{
+    VM& vm = exec->vm();
+    validatePutOwnDataProperty(vm, propertyName, value);
+    if (Optional<uint32_t> index = parseIndex(propertyName))
+        return putDirectIndex(exec, index.value(), value, 0, PutDirectIndexLikePutDirect);
+
+    return putDirectInternal<PutModePut>(vm, propertyName, value, 0, slot);
+}
 
 } // namespace JSC