diff options
author | Andreas Kling <kling@serenityos.org> | 2023-06-02 08:17:28 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2023-06-02 10:33:12 +0200 |
commit | 5617dd1c839eb27168f933bf8a93d2707ddba25a (patch) | |
tree | ba34b0aeb561ff362db11bf658db81082243e809 /Userland | |
parent | 0eddee44f3d594ba702bc496ea2591b686830900 (diff) | |
download | serenity-5617dd1c839eb27168f933bf8a93d2707ddba25a.zip |
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.
Diffstat (limited to 'Userland')
-rw-r--r-- | Userland/Libraries/LibJS/AST.cpp | 12 | ||||
-rw-r--r-- | Userland/Libraries/LibJS/Runtime/Object.cpp | 19 | ||||
-rw-r--r-- | Userland/Libraries/LibJS/Runtime/Object.h | 2 |
3 files changed, 14 insertions, 19 deletions
diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 09e8663230..44cb734ce8 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -1735,13 +1735,13 @@ ThrowCompletionOr<ClassElement::ClassValue> ClassMethod::class_element_evaluatio switch (kind()) { case Kind::Method: 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: 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: 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: VERIFY_NOT_REACHED(); } @@ -2043,11 +2043,11 @@ ThrowCompletionOr<ECMAScriptFunctionObject*> ClassExpression::class_definition_e if (existing.key == private_element.key) { VERIFY(existing.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()) - existing.value.as_accessor().set_setter(accessor.setter()); + existing.value.value().as_accessor().set_setter(accessor.setter()); else - existing.value.as_accessor().set_getter(accessor.getter()); + existing.value.value().as_accessor().set_getter(accessor.getter()); added_to_existing = true; } } diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index e495dc1658..ebfe8d84f6 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -508,7 +508,7 @@ ThrowCompletionOr<void> Object::private_field_add(PrivateName const& name, Value m_private_elements = make<Vector<PrivateElement>>(); // 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. return {}; @@ -559,14 +559,14 @@ ThrowCompletionOr<Value> Object::private_get(PrivateName const& name) // 3. If entry.[[Kind]] is either field or method, then if (entry->kind != PrivateElement::Kind::Accessor) { // a. Return entry.[[Value]]. - return value; + return value.value(); } // Assert: entry.[[Kind]] is accessor. - VERIFY(value.is_accessor()); + VERIFY(value.value().is_accessor()); // 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. if (!getter) @@ -591,7 +591,7 @@ ThrowCompletionOr<void> Object::private_set(PrivateName const& name, Value value // 3. If entry.[[Kind]] is field, then if (entry->kind == PrivateElement::Kind::Field) { // a. Set entry.[[Value]] to value. - entry->value = value; + entry->value = make_handle(value); return {}; } // 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); auto& accessor = entry->value; - VERIFY(accessor.is_accessor()); + VERIFY(accessor.value().is_accessor()); // 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. if (!setter) @@ -1350,11 +1350,6 @@ void Object::visit_edges(Cell::Visitor& visitor) m_indexed_properties.for_each_value([&visitor](auto& 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 diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index 2d01081d45..2b56dd9257 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -36,7 +36,7 @@ struct PrivateElement { PrivateName key; Kind kind { Kind::Field }; - Value value; + Handle<Value> value; }; class Object : public Cell { |