Skip to content

Commit

Permalink
Cherry-pick 259548.774@safari-7615-branch (23e9761). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=257164

    [JSC] putByValWithThis shouldn't bypass definePropertyOnReceiverSlow
    https://bugs.webkit.org/show_bug.cgi?id=257164
    <rdar://108759737>

    Reviewed by Yusuke Suzuki.

    The OrdinarySet revamp in https://webkit.org/b/217916 assumed that there are only 2 cases to take the slow path
    for altered receivers: overriden [[Set]] in prototype chain and Reflect.set(). I thought that it's unobservable
    to take the fast path otherwise since overriden methods were already called.

    However, the third case was missed: put_by_val_with_this bytecode op, which is emitted for setting a property
    on `super` base, and with https://webkit.org/b/252602, for ProxyObjectStore IC when the trap is missing.

    Among other minor web compatibility bugs, missing that case caused properties to be put right on ProxyObject's
    structure, where they are unaccessible, skipping calls to "set" and "defineProperty" traps.

    This change relaxes the condition for taking the definePropertyOnReceiverSlow() while ensuring all common
    [[Set]] targets like JSArray or `class X extends Y {}` are just as fast.

    * JSTests/stress/define-property-on-receiver-jsfunction-prototype-no-crash.js: Added.
    * Source/JavaScriptCore/runtime/JSObject.cpp:
    (JSC::canDefinePropertyOnReceiverFast):
    (JSC::JSObject::definePropertyOnReceiver):

    Canonical link: https://commits.webkit.org/259548.774@safari-7615-branch
  • Loading branch information
Alexey Shvayka authored and mcatanzaro committed Jul 28, 2023
1 parent ef65ba5 commit d784299
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
function shouldThrow(func, errorMessage) {
var errorThrown = false;
try {
func();
} catch (error) {
errorThrown = true;
if (String(error) !== errorMessage)
throw new Error(`Bad error: ${error}`);
}
if (!errorThrown)
throw new Error("Didn't throw!");
}

class MyFunction extends Function {
constructor() {
super();

super.prototype = 1;
}
}

function test1() {
const f = new MyFunction();
f.__defineGetter__("prototype", () => {}); // should throw
}

function test2(i) {
const f = new MyFunction();
try { f.__defineGetter__("prototype", () => {}); } catch {}
f.prototype.x = i; // should not crash
}

(function() {
for (var i = 0; i < 1e4; i++)
shouldThrow(test1, "TypeError: Attempting to change configurable attribute of unconfigurable property.");
})();

(function() {
for (var i = 0; i < 1e4; i++)
test2();
})();
4 changes: 2 additions & 2 deletions JSTests/stress/ordinary-set-exceptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ shouldThrow(function () {
value: 42,
}), true);
receiver.cocoa = 'NG';
}, `TypeError: Attempting to change value of a readonly property.`);
}, `TypeError: Attempted to assign to readonly property.`);

// 9.1.9.1 4-d-ii
shouldThrow(function () {
Expand All @@ -85,7 +85,7 @@ shouldThrow(function () {
value: 42,
}), true);
receiver.cocoa = 'NG';
}, `TypeError: Attempting to change value of a readonly property.`);
}, `TypeError: Attempted to assign to readonly property.`);

// 9.1.9.1 7
shouldThrow(function () {
Expand Down
16 changes: 14 additions & 2 deletions Source/JavaScriptCore/runtime/JSObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,18 @@ bool JSObject::putInlineSlow(JSGlobalObject* globalObject, PropertyName property
return putInlineFast(globalObject, propertyName, value, slot);
}

static bool canDefinePropertyOnReceiverFast(VM& vm, JSObject* receiver, PropertyName propertyName)
{
switch (receiver->type()) {
case ArrayType:
return propertyName != vm.propertyNames->length;
case JSFunctionType:
return propertyName != vm.propertyNames->length && propertyName != vm.propertyNames->name && propertyName != vm.propertyNames->prototype;
default:
return false;
}
}

static NEVER_INLINE bool definePropertyOnReceiverSlow(JSGlobalObject* globalObject, PropertyName propertyName, JSValue value, JSObject* receiver, bool shouldThrow)
{
VM& vm = globalObject->vm();
Expand Down Expand Up @@ -903,8 +915,8 @@ bool JSObject::definePropertyOnReceiver(JSGlobalObject* globalObject, PropertyNa
if (receiver->type() == PureForwardingProxyType)
receiver = jsCast<JSProxy*>(receiver)->target();

if (slot.isTaintedByOpaqueObject() || slot.context() == PutPropertySlot::ReflectSet) {
if (receiver->methodTable()->defineOwnProperty != JSObject::defineOwnProperty)
if (slot.isTaintedByOpaqueObject() || receiver->methodTable()->defineOwnProperty != JSObject::defineOwnProperty) {
if (!canDefinePropertyOnReceiverFast(vm, receiver, propertyName))
return definePropertyOnReceiverSlow(globalObject, propertyName, value, receiver, slot.isStrictMode());
}

Expand Down

0 comments on commit d784299

Please sign in to comment.