- Revision
- 280463
- Author
- tzaga...@apple.com
- Date
- 2021-07-29 19:00:36 -0700 (Thu, 29 Jul 2021)
Log Message
definePropertyOnReceiver should check if receiver canPerformFastPutInline
https://bugs.webkit.org/show_bug.cgi?id=227963
<rdar://80259710>
Reviewed by Alexey Shvayka.
JSTests:
* stress/reflect-set-custom-value.js: Added.
Source/_javascript_Core:
definePropertyOnReceiver has a fast path if the slot is not opaque and the receiver doesn't
have a custom defineOwnProperty implementation, in which case it calls putInlineFast (and
transitively putDirectInternal<PutModePut>). The issue is that putDirectInternal does not
handle customValues correctly: it just overwrites the property without changing the attributes.
To fix that, we should first check if the property might be a custom value, and if that's the case
we now call `definePropertyOnReceiverSlow`, which has been updated to handle custom values correctly.
I also added assertions to putInlineFastReplacingStaticPropertyIfNeeded and putDirectInternal
to make sure we don't accidentally overwrite custom values in the future.
* runtime/JSObject.cpp:
(JSC::definePropertyOnReceiverSlow):
(JSC::JSObject::definePropertyOnReceiver):
(JSC::JSObject::putInlineFastReplacingStaticPropertyIfNeeded):
* runtime/JSObjectInlines.h:
(JSC::JSObject::putDirectInternal):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (280462 => 280463)
--- trunk/JSTests/ChangeLog 2021-07-30 01:51:18 UTC (rev 280462)
+++ trunk/JSTests/ChangeLog 2021-07-30 02:00:36 UTC (rev 280463)
@@ -1,3 +1,13 @@
+2021-07-29 Tadeu Zagallo <tzaga...@apple.com>
+
+ definePropertyOnReceiver should check if receiver canPerformFastPutInline
+ https://bugs.webkit.org/show_bug.cgi?id=227963
+ <rdar://80259710>
+
+ Reviewed by Alexey Shvayka.
+
+ * stress/reflect-set-custom-value.js: Added.
+
2021-07-29 Yusuke Suzuki <ysuz...@apple.com> and Alexey Shvayka <shvaikal...@gmail.com>
[JSC] Legacy RegExp fields should be accessors
Added: trunk/JSTests/stress/reflect-set-custom-value.js (0 => 280463)
--- trunk/JSTests/stress/reflect-set-custom-value.js (rev 0)
+++ trunk/JSTests/stress/reflect-set-custom-value.js 2021-07-30 02:00:36 UTC (rev 280463)
@@ -0,0 +1,3 @@
+const testGetterSetter = $vm.createCustomTestGetterSetter();
+Reflect.set({}, 'customValue', 'foo', testGetterSetter);
+testGetterSetter.customValue = 42;
Modified: trunk/Source/_javascript_Core/ChangeLog (280462 => 280463)
--- trunk/Source/_javascript_Core/ChangeLog 2021-07-30 01:51:18 UTC (rev 280462)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-07-30 02:00:36 UTC (rev 280463)
@@ -1,3 +1,27 @@
+2021-07-29 Tadeu Zagallo <tzaga...@apple.com>
+
+ definePropertyOnReceiver should check if receiver canPerformFastPutInline
+ https://bugs.webkit.org/show_bug.cgi?id=227963
+ <rdar://80259710>
+
+ Reviewed by Alexey Shvayka.
+
+ definePropertyOnReceiver has a fast path if the slot is not opaque and the receiver doesn't
+ have a custom defineOwnProperty implementation, in which case it calls putInlineFast (and
+ transitively putDirectInternal<PutModePut>). The issue is that putDirectInternal does not
+ handle customValues correctly: it just overwrites the property without changing the attributes.
+ To fix that, we should first check if the property might be a custom value, and if that's the case
+ we now call `definePropertyOnReceiverSlow`, which has been updated to handle custom values correctly.
+ I also added assertions to putInlineFastReplacingStaticPropertyIfNeeded and putDirectInternal
+ to make sure we don't accidentally overwrite custom values in the future.
+
+ * runtime/JSObject.cpp:
+ (JSC::definePropertyOnReceiverSlow):
+ (JSC::JSObject::definePropertyOnReceiver):
+ (JSC::JSObject::putInlineFastReplacingStaticPropertyIfNeeded):
+ * runtime/JSObjectInlines.h:
+ (JSC::JSObject::putDirectInternal):
+
2021-07-29 Yusuke Suzuki <ysuz...@apple.com> and Alexey Shvayka <shvaikal...@gmail.com>
[JSC] Legacy RegExp fields should be accessors
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (280462 => 280463)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2021-07-30 01:51:18 UTC (rev 280462)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2021-07-30 02:00:36 UTC (rev 280463)
@@ -874,6 +874,12 @@
if (slot.attributes() & PropertyAttribute::ReadOnlyOrAccessorOrCustomAccessor)
return typeError(globalObject, scope, shouldThrow, ReadonlyPropertyWriteError);
+ if (slot.attributes() & PropertyAttribute::CustomValue) {
+ PutPropertySlot::PutValueFunc customSetter = slot.customSetter();
+ if (customSetter)
+ RELEASE_AND_RETURN(scope, customSetter(receiver->globalObject(vm), JSValue::encode(receiver), JSValue::encode(value), propertyName));
+ }
+
PropertyDescriptor descriptor;
descriptor.setValue(value);
RELEASE_AND_RETURN(scope, receiver->methodTable(vm)->defineOwnProperty(receiver, globalObject, propertyName, descriptor, shouldThrow));
@@ -903,6 +909,12 @@
return definePropertyOnReceiverSlow(globalObject, propertyName, value, receiver, slot.isStrictMode());
}
+ if (receiver->structure(vm)->hasCustomGetterSetterProperties()) {
+ unsigned attributes;
+ if (receiver->getDirectOffset(vm, propertyName, attributes) != invalidOffset && (attributes & PropertyAttribute::CustomValue))
+ return definePropertyOnReceiverSlow(globalObject, propertyName, value, receiver, slot.isStrictMode());
+ }
+
if (UNLIKELY(receiver->hasNonReifiedStaticProperties(vm)))
return receiver->putInlineFastReplacingStaticPropertyIfNeeded(globalObject, propertyName, value, slot);
return receiver->putInlineFast(globalObject, propertyName, value, slot);
@@ -925,7 +937,8 @@
return typeError(globalObject, scope, slot.isStrictMode(), ReadonlyPropertyWriteError);
}
// Avoid PutModePut because it fails for non-extensible structures.
- putDirect(vm, propertyName, value, attributesForStructure(entry->value->attributes()) & ~PropertyAttribute::CustomValue, slot);
+ ASSERT(!(entry->value->attributes() & PropertyAttribute::CustomValue));
+ putDirect(vm, propertyName, value, attributesForStructure(entry->value->attributes()), slot);
return true;
}
}
Modified: trunk/Source/_javascript_Core/runtime/JSObjectInlines.h (280462 => 280463)
--- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2021-07-30 01:51:18 UTC (rev 280462)
+++ trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2021-07-30 02:00:36 UTC (rev 280463)
@@ -282,7 +282,7 @@
VM& vm = getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);
- // FIXME: For a failure due to non-extensible structure, the error message is misleading.
+ // FIXME: For a failure due to non-extensible structure, the error message is misleading
if (!putDirectInternal<PutModePut>(vm, propertyName, value, 0, slot))
return typeError(globalObject, scope, slot.isStrictMode(), ReadonlyPropertyWriteError);
return true;
@@ -345,8 +345,10 @@
// https://bugs.webkit.org/show_bug.cgi?id=214342
if (mode == PutModeDefineOwnProperty && (attributes != currentAttributes || (attributes & PropertyAttribute::AccessorOrCustomAccessorOrValue)))
setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes));
- else
+ else {
+ ASSERT(!(currentAttributes & PropertyAttribute::AccessorOrCustomAccessorOrValue));
slot.setExistingProperty(this, offset);
+ }
return true;
}
@@ -403,8 +405,10 @@
// This allows adaptive watchpoints to observe if the new structure is the one we want.
DeferredStructureTransitionWatchpointFire deferredWatchpointFire(vm, structure);
setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes, &deferredWatchpointFire));
- } else
+ } else {
+ ASSERT(!(currentAttributes & PropertyAttribute::AccessorOrCustomAccessorOrValue));
slot.setExistingProperty(this, offset);
+ }
return true;
}
