1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 03:27:45 +00:00

LibJS: Store PrivateElement values in Handle<Value>

This fixes an issue where private element values were not always
protected from GC. I found two instances where this was happening:

- ECMAScriptFunctionObject did not mark m_private_methods
- ClassDefinitionEvaluation had two Vector<PrivateElement> that were
  opaque to the garbage collector, and so if GC occurred while
  constructing a class instance, some or all of its private elements
  could get incorrectly collected.
This commit is contained in:
Andreas Kling 2023-06-02 08:17:28 +02:00
parent 0eddee44f3
commit 5617dd1c83
3 changed files with 14 additions and 19 deletions

View file

@ -1735,13 +1735,13 @@ ThrowCompletionOr<ClassElement::ClassValue> ClassMethod::class_element_evaluatio
switch (kind()) { switch (kind()) {
case Kind::Method: case Kind::Method:
set_function_name(); set_function_name();
return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Method, method_value } }; return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Method, make_handle(method_value) } };
case Kind::Getter: case Kind::Getter:
set_function_name("get"); set_function_name("get");
return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Accessor, Accessor::create(interpreter.vm(), &method_function, nullptr) } }; return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Accessor, make_handle(Value(Accessor::create(interpreter.vm(), &method_function, nullptr))) } };
case Kind::Setter: case Kind::Setter:
set_function_name("set"); set_function_name("set");
return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Accessor, Accessor::create(interpreter.vm(), nullptr, &method_function) } }; return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Accessor, make_handle(Value(Accessor::create(interpreter.vm(), nullptr, &method_function))) } };
default: default:
VERIFY_NOT_REACHED(); VERIFY_NOT_REACHED();
} }
@ -2043,11 +2043,11 @@ ThrowCompletionOr<ECMAScriptFunctionObject*> ClassExpression::class_definition_e
if (existing.key == private_element.key) { if (existing.key == private_element.key) {
VERIFY(existing.kind == PrivateElement::Kind::Accessor); VERIFY(existing.kind == PrivateElement::Kind::Accessor);
VERIFY(private_element.kind == PrivateElement::Kind::Accessor); VERIFY(private_element.kind == PrivateElement::Kind::Accessor);
auto& accessor = private_element.value.as_accessor(); auto& accessor = private_element.value.value().as_accessor();
if (!accessor.getter()) if (!accessor.getter())
existing.value.as_accessor().set_setter(accessor.setter()); existing.value.value().as_accessor().set_setter(accessor.setter());
else else
existing.value.as_accessor().set_getter(accessor.getter()); existing.value.value().as_accessor().set_getter(accessor.getter());
added_to_existing = true; added_to_existing = true;
} }
} }

View file

@ -508,7 +508,7 @@ ThrowCompletionOr<void> Object::private_field_add(PrivateName const& name, Value
m_private_elements = make<Vector<PrivateElement>>(); m_private_elements = make<Vector<PrivateElement>>();
// 4. Append PrivateElement { [[Key]]: P, [[Kind]]: field, [[Value]]: value } to O.[[PrivateElements]]. // 4. Append PrivateElement { [[Key]]: P, [[Kind]]: field, [[Value]]: value } to O.[[PrivateElements]].
m_private_elements->empend(name, PrivateElement::Kind::Field, value); m_private_elements->empend(name, PrivateElement::Kind::Field, make_handle(value));
// 5. Return unused. // 5. Return unused.
return {}; return {};
@ -559,14 +559,14 @@ ThrowCompletionOr<Value> Object::private_get(PrivateName const& name)
// 3. If entry.[[Kind]] is either field or method, then // 3. If entry.[[Kind]] is either field or method, then
if (entry->kind != PrivateElement::Kind::Accessor) { if (entry->kind != PrivateElement::Kind::Accessor) {
// a. Return entry.[[Value]]. // a. Return entry.[[Value]].
return value; return value.value();
} }
// Assert: entry.[[Kind]] is accessor. // Assert: entry.[[Kind]] is accessor.
VERIFY(value.is_accessor()); VERIFY(value.value().is_accessor());
// 6. Let getter be entry.[[Get]]. // 6. Let getter be entry.[[Get]].
auto* getter = value.as_accessor().getter(); auto* getter = value.value().as_accessor().getter();
// 5. If entry.[[Get]] is undefined, throw a TypeError exception. // 5. If entry.[[Get]] is undefined, throw a TypeError exception.
if (!getter) if (!getter)
@ -591,7 +591,7 @@ ThrowCompletionOr<void> Object::private_set(PrivateName const& name, Value value
// 3. If entry.[[Kind]] is field, then // 3. If entry.[[Kind]] is field, then
if (entry->kind == PrivateElement::Kind::Field) { if (entry->kind == PrivateElement::Kind::Field) {
// a. Set entry.[[Value]] to value. // a. Set entry.[[Value]] to value.
entry->value = value; entry->value = make_handle(value);
return {}; return {};
} }
// 4. Else if entry.[[Kind]] is method, then // 4. Else if entry.[[Kind]] is method, then
@ -606,10 +606,10 @@ ThrowCompletionOr<void> Object::private_set(PrivateName const& name, Value value
VERIFY(entry->kind == PrivateElement::Kind::Accessor); VERIFY(entry->kind == PrivateElement::Kind::Accessor);
auto& accessor = entry->value; auto& accessor = entry->value;
VERIFY(accessor.is_accessor()); VERIFY(accessor.value().is_accessor());
// c. Let setter be entry.[[Set]]. // c. Let setter be entry.[[Set]].
auto* setter = accessor.as_accessor().setter(); auto* setter = accessor.value().as_accessor().setter();
// b. If entry.[[Set]] is undefined, throw a TypeError exception. // b. If entry.[[Set]] is undefined, throw a TypeError exception.
if (!setter) if (!setter)
@ -1350,11 +1350,6 @@ void Object::visit_edges(Cell::Visitor& visitor)
m_indexed_properties.for_each_value([&visitor](auto& value) { m_indexed_properties.for_each_value([&visitor](auto& value) {
visitor.visit(value); visitor.visit(value);
}); });
if (m_private_elements) {
for (auto& private_element : *m_private_elements)
visitor.visit(private_element.value);
}
} }
// 7.1.1.1 OrdinaryToPrimitive ( O, hint ), https://tc39.es/ecma262/#sec-ordinarytoprimitive // 7.1.1.1 OrdinaryToPrimitive ( O, hint ), https://tc39.es/ecma262/#sec-ordinarytoprimitive

View file

@ -36,7 +36,7 @@ struct PrivateElement {
PrivateName key; PrivateName key;
Kind kind { Kind::Field }; Kind kind { Kind::Field };
Value value; Handle<Value> value;
}; };
class Object : public Cell { class Object : public Cell {