From c0952e3670e02a2b344cb2f96232ef06221fc8e1 Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Wed, 30 Nov 2022 09:18:27 -0500
Subject: [PATCH] LibJS: Do not allocate in Set's constructor

We are currently allocating in Set's constructor to create the set's
underlying Map. This can cause GC to occur before the member is actually
initialized, thus we will crash in Set::visit_edges trying to visit a
member that does not exist.

Instead, create the Map in Set::initialize, where we can allocate. Also
change Map to be stored as a normal JS heap-allocated object, rather
than as a stack variable.
---
 Userland/Libraries/LibJS/Runtime/Map.h   |  3 ---
 Userland/Libraries/LibJS/Runtime/Set.cpp |  8 ++++++--
 Userland/Libraries/LibJS/Runtime/Set.h   | 19 ++++++++++---------
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/Userland/Libraries/LibJS/Runtime/Map.h b/Userland/Libraries/LibJS/Runtime/Map.h
index eed8cc4490..866a557dd9 100644
--- a/Userland/Libraries/LibJS/Runtime/Map.h
+++ b/Userland/Libraries/LibJS/Runtime/Map.h
@@ -19,9 +19,6 @@ namespace JS {
 class Map : public Object {
     JS_OBJECT(Map, Object);
 
-    // NOTE: This awkwardness is due to Set using a Map internally.
-    friend class Set;
-
 public:
     static Map* create(Realm&);
 
diff --git a/Userland/Libraries/LibJS/Runtime/Set.cpp b/Userland/Libraries/LibJS/Runtime/Set.cpp
index 618d95b6f8..9d6b7e57a3 100644
--- a/Userland/Libraries/LibJS/Runtime/Set.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Set.cpp
@@ -15,14 +15,18 @@ Set* Set::create(Realm& realm)
 
 Set::Set(Object& prototype)
     : Object(prototype)
-    , m_values(*prototype.shape().realm().intrinsics().map_prototype())
 {
 }
 
+void Set::initialize(Realm& realm)
+{
+    m_values = Map::create(realm);
+}
+
 void Set::visit_edges(Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
-    static_cast<Object&>(m_values).visit_edges(visitor);
+    visitor.visit(m_values);
 }
 
 }
diff --git a/Userland/Libraries/LibJS/Runtime/Set.h b/Userland/Libraries/LibJS/Runtime/Set.h
index 5acf6ab5ab..05523aee96 100644
--- a/Userland/Libraries/LibJS/Runtime/Set.h
+++ b/Userland/Libraries/LibJS/Runtime/Set.h
@@ -19,28 +19,29 @@ class Set : public Object {
 public:
     static Set* create(Realm&);
 
+    virtual void initialize(Realm&) override;
     virtual ~Set() override = default;
 
     // NOTE: Unlike what the spec says, we implement Sets using an underlying map,
     //       so all the functions below do not directly implement the operations as
     //       defined by the specification.
 
-    void set_clear() { m_values.map_clear(); }
-    bool set_remove(Value const& value) { return m_values.map_remove(value); }
-    bool set_has(Value const& key) const { return m_values.map_has(key); }
-    void set_add(Value const& key) { m_values.map_set(key, js_undefined()); }
-    size_t set_size() const { return m_values.map_size(); }
+    void set_clear() { m_values->map_clear(); }
+    bool set_remove(Value const& value) { return m_values->map_remove(value); }
+    bool set_has(Value const& key) const { return m_values->map_has(key); }
+    void set_add(Value const& key) { m_values->map_set(key, js_undefined()); }
+    size_t set_size() const { return m_values->map_size(); }
 
-    auto begin() const { return m_values.begin(); }
-    auto begin() { return m_values.begin(); }
-    auto end() const { return m_values.end(); }
+    auto begin() const { return const_cast<Map const&>(*m_values).begin(); }
+    auto begin() { return m_values->begin(); }
+    auto end() const { return m_values->end(); }
 
 private:
     explicit Set(Object& prototype);
 
     virtual void visit_edges(Visitor& visitor) override;
 
-    Map m_values;
+    GCPtr<Map> m_values;
 };
 
 }