瀏覽代碼

LibWeb+LibJS: Don't lazily construct web prototypes in cell constructors

It's not safe to allocate from the GC heap while in the constructor of a
GC heap cell. (Because if this ends up triggering a collection, we may
end up trying to call through an uninitialized vtable).

This was already done safely in the initialize() virtual in much of
LibJS and LibWeb. This patch moves the logic for prototypes, mixins,
and CSSStyleDeclaration as well.

Fixes a long-standing GC crash that was pretty easy to reproduce by
refreshing https://vercel.com/
Andreas Kling 2 年之前
父節點
當前提交
cfe663435e

+ 39 - 1
Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp

@@ -2504,6 +2504,8 @@ static void generate_prototype_or_global_mixin_definitions(IDL::Interface const&
     generator.set("namespaced_name", interface.namespaced_name);
     generator.set("namespaced_name", interface.namespaced_name);
     generator.set("class_name", class_name);
     generator.set("class_name", class_name);
     generator.set("fully_qualified_name", interface.fully_qualified_name);
     generator.set("fully_qualified_name", interface.fully_qualified_name);
+    generator.set("parent_name", interface.parent_name);
+    generator.set("prototype_base_class", interface.prototype_base_class);
 
 
     if (interface.pair_iterator_types.has_value()) {
     if (interface.pair_iterator_types.has_value()) {
         generator.set("iterator_name", DeprecatedString::formatted("{}Iterator", interface.name));
         generator.set("iterator_name", DeprecatedString::formatted("{}Iterator", interface.name));
@@ -2515,6 +2517,7 @@ static void generate_prototype_or_global_mixin_definitions(IDL::Interface const&
 #define define_direct_property (object.define_direct_property)
 #define define_direct_property (object.define_direct_property)
 #define define_native_accessor (object.define_native_accessor)
 #define define_native_accessor (object.define_native_accessor)
 #define define_native_function (object.define_native_function)
 #define define_native_function (object.define_native_function)
+#define set_prototype (object.set_prototype)
 
 
 JS::ThrowCompletionOr<void> @class_name@::initialize(JS::Realm& realm, JS::Object& object)
 JS::ThrowCompletionOr<void> @class_name@::initialize(JS::Realm& realm, JS::Object& object)
 {
 {
@@ -2535,6 +2538,20 @@ JS::ThrowCompletionOr<void> @class_name@::initialize(JS::Realm& realm)
 
 
 )~~~");
 )~~~");
 
 
+    if (interface.prototype_base_class == "ObjectPrototype") {
+        generator.append(R"~~~(
+
+    set_prototype(realm.intrinsics().object_prototype());
+
+)~~~");
+    } else {
+        generator.append(R"~~~(
+
+    set_prototype(&ensure_web_prototype<@prototype_base_class@>(realm, "@parent_name@"));
+
+)~~~");
+    }
+
     if (interface.has_unscopable_member) {
     if (interface.has_unscopable_member) {
         generator.append(R"~~~(
         generator.append(R"~~~(
     auto unscopable_object = JS::Object::create(realm, nullptr);
     auto unscopable_object = JS::Object::create(realm, nullptr);
@@ -3710,7 +3727,7 @@ namespace Web::Bindings {
 )~~~");
 )~~~");
     } else if (!interface.parent_name.is_empty()) {
     } else if (!interface.parent_name.is_empty()) {
         generator.append(R"~~~(
         generator.append(R"~~~(
-    : Object(ConstructWithPrototypeTag::Tag, ensure_web_prototype<@prototype_base_class@>(realm, "@parent_name@"))
+    : Object(realm, nullptr)
 )~~~");
 )~~~");
     } else {
     } else {
         generator.append(R"~~~(
         generator.append(R"~~~(
@@ -3781,7 +3798,9 @@ void generate_iterator_prototype_implementation(IDL::Interface const& interface,
     SourceGenerator generator { builder };
     SourceGenerator generator { builder };
 
 
     generator.set("name", DeprecatedString::formatted("{}Iterator", interface.name));
     generator.set("name", DeprecatedString::formatted("{}Iterator", interface.name));
+    generator.set("parent_name", interface.parent_name);
     generator.set("prototype_class", DeprecatedString::formatted("{}IteratorPrototype", interface.name));
     generator.set("prototype_class", DeprecatedString::formatted("{}IteratorPrototype", interface.name));
+    generator.set("prototype_base_class", interface.prototype_base_class);
     generator.set("fully_qualified_name", DeprecatedString::formatted("{}Iterator", interface.fully_qualified_name));
     generator.set("fully_qualified_name", DeprecatedString::formatted("{}Iterator", interface.fully_qualified_name));
     generator.set("possible_include_path", DeprecatedString::formatted("{}Iterator", interface.name.replace("::"sv, "/"sv, ReplaceMode::All)));
     generator.set("possible_include_path", DeprecatedString::formatted("{}Iterator", interface.name.replace("::"sv, "/"sv, ReplaceMode::All)));
 
 
@@ -3795,6 +3814,7 @@ void generate_iterator_prototype_implementation(IDL::Interface const& interface,
 #include <LibJS/Runtime/TypedArray.h>
 #include <LibJS/Runtime/TypedArray.h>
 #include <LibWeb/Bindings/@prototype_class@.h>
 #include <LibWeb/Bindings/@prototype_class@.h>
 #include <LibWeb/Bindings/ExceptionOrUtils.h>
 #include <LibWeb/Bindings/ExceptionOrUtils.h>
+#include <LibWeb/Bindings/Intrinsics.h>
 
 
 #if __has_include(<LibWeb/@possible_include_path@.h>)
 #if __has_include(<LibWeb/@possible_include_path@.h>)
 #    include <LibWeb/@possible_include_path@.h>
 #    include <LibWeb/@possible_include_path@.h>
@@ -3842,6 +3862,24 @@ JS::ThrowCompletionOr<void> @prototype_class@::initialize(JS::Realm& realm)
     auto& vm = this->vm();
     auto& vm = this->vm();
     MUST_OR_THROW_OOM(Base::initialize(realm));
     MUST_OR_THROW_OOM(Base::initialize(realm));
 
 
+)~~~");
+
+    if (interface.prototype_base_class == "ObjectPrototype") {
+        generator.append(R"~~~(
+
+    set_prototype(realm.intrinsics().object_prototype());
+
+)~~~");
+    } else {
+        generator.append(R"~~~(
+
+    set_prototype(&ensure_web_prototype<@prototype_base_class@>(realm, "@parent_name@"));
+
+)~~~");
+    }
+
+    generator.append(R"~~~(
+
     define_native_function(realm, vm.names.next, next, 0, JS::Attribute::Writable | JS::Attribute::Enumerable | JS::Attribute::Configurable);
     define_native_function(realm, vm.names.next, next, 0, JS::Attribute::Writable | JS::Attribute::Enumerable | JS::Attribute::Configurable);
     define_direct_property(vm.well_known_symbol_to_string_tag(), MUST_OR_THROW_OOM(JS::PrimitiveString::create(vm, "Iterator"sv)), JS::Attribute::Configurable);
     define_direct_property(vm.well_known_symbol_to_string_tag(), MUST_OR_THROW_OOM(JS::PrimitiveString::create(vm, "Iterator"sv)), JS::Attribute::Configurable);
 
 

+ 2 - 2
Userland/Libraries/LibJS/Runtime/Object.h

@@ -187,6 +187,8 @@ public:
     template<typename T>
     template<typename T>
     bool fast_is() const = delete;
     bool fast_is() const = delete;
 
 
+    void set_prototype(Object*);
+
 protected:
 protected:
     enum class GlobalObjectTag { Tag };
     enum class GlobalObjectTag { Tag };
     enum class ConstructWithoutPrototypeTag { Tag };
     enum class ConstructWithoutPrototypeTag { Tag };
@@ -198,8 +200,6 @@ protected:
     Object(ConstructWithPrototypeTag, Object& prototype);
     Object(ConstructWithPrototypeTag, Object& prototype);
     explicit Object(Shape&);
     explicit Object(Shape&);
 
 
-    void set_prototype(Object*);
-
     // [[Extensible]]
     // [[Extensible]]
     bool m_is_extensible { true };
     bool m_is_extensible { true };
 
 

+ 8 - 1
Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp

@@ -16,10 +16,17 @@
 namespace Web::CSS {
 namespace Web::CSS {
 
 
 CSSStyleDeclaration::CSSStyleDeclaration(JS::Realm& realm)
 CSSStyleDeclaration::CSSStyleDeclaration(JS::Realm& realm)
-    : PlatformObject(Bindings::ensure_web_prototype<Bindings::CSSStyleDeclarationPrototype>(realm, "CSSStyleDeclaration"))
+    : PlatformObject(realm)
 {
 {
 }
 }
 
 
+JS::ThrowCompletionOr<void> CSSStyleDeclaration::initialize(JS::Realm& realm)
+{
+    MUST_OR_THROW_OOM(Base::initialize(realm));
+    set_prototype(&Bindings::ensure_web_prototype<Bindings::CSSStyleDeclarationPrototype>(realm, "CSSStyleDeclaration"));
+    return {};
+}
+
 WebIDL::ExceptionOr<JS::NonnullGCPtr<PropertyOwningCSSStyleDeclaration>> PropertyOwningCSSStyleDeclaration::create(JS::Realm& realm, Vector<StyleProperty> properties, HashMap<DeprecatedString, StyleProperty> custom_properties)
 WebIDL::ExceptionOr<JS::NonnullGCPtr<PropertyOwningCSSStyleDeclaration>> PropertyOwningCSSStyleDeclaration::create(JS::Realm& realm, Vector<StyleProperty> properties, HashMap<DeprecatedString, StyleProperty> custom_properties)
 {
 {
     return MUST_OR_THROW_OOM(realm.heap().allocate<PropertyOwningCSSStyleDeclaration>(realm, realm, move(properties), move(custom_properties)));
     return MUST_OR_THROW_OOM(realm.heap().allocate<PropertyOwningCSSStyleDeclaration>(realm, realm, move(properties), move(custom_properties)));

+ 1 - 0
Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.h

@@ -19,6 +19,7 @@ class CSSStyleDeclaration : public Bindings::PlatformObject {
 
 
 public:
 public:
     virtual ~CSSStyleDeclaration() = default;
     virtual ~CSSStyleDeclaration() = default;
+    virtual JS::ThrowCompletionOr<void> initialize(JS::Realm&) override;
 
 
     virtual size_t length() const = 0;
     virtual size_t length() const = 0;
     virtual DeprecatedString item(size_t index) const = 0;
     virtual DeprecatedString item(size_t index) const = 0;