From bf9912cc59327904ac7023f0aac31dbccc84d8de Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Tue, 17 Mar 2020 11:00:09 +0100
Subject: [PATCH] LibJS: Protect function call "this" and arguments from GC

This patch adds a CallFrame stack to Interpreter, which keeps track of
the "this" value and all argument values passed in function calls.

Interpreter::gather_roots() scans the call stack, making sure that all
argument values get marked. :^)
---
 Libraries/LibJS/AST.cpp         | 16 +++++-----------
 Libraries/LibJS/Interpreter.cpp | 10 +++++++---
 Libraries/LibJS/Interpreter.h   | 15 ++++++++++-----
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp
index fb7993aea7..d1e9d53828 100644
--- a/Libraries/LibJS/AST.cpp
+++ b/Libraries/LibJS/AST.cpp
@@ -59,21 +59,15 @@ Value CallExpression::execute(Interpreter& interpreter) const
     ASSERT(callee.as_object()->is_function());
     auto* function = static_cast<Function*>(callee.as_object());
 
-    Vector<Value> argument_values;
+    auto& call_frame = interpreter.push_call_frame();
     for (size_t i = 0; i < m_arguments.size(); ++i)
-        argument_values.append(m_arguments[i].execute(interpreter));
+        call_frame.arguments.append(m_arguments[i].execute(interpreter));
 
-    Value this_value = js_undefined();
     if (m_callee->is_member_expression())
-        this_value = static_cast<const MemberExpression&>(*m_callee).object().execute(interpreter).to_object(interpreter.heap());
+        call_frame.this_value = static_cast<const MemberExpression&>(*m_callee).object().execute(interpreter).to_object(interpreter.heap());
 
-    if (!this_value.is_undefined())
-        interpreter.push_this_value(this_value);
-
-    auto result = function->call(interpreter, move(argument_values));
-
-    if (!this_value.is_undefined())
-        interpreter.pop_this_value();
+    auto result = function->call(interpreter, call_frame.arguments);
+    interpreter.pop_call_frame();
     return result;
 }
 
diff --git a/Libraries/LibJS/Interpreter.cpp b/Libraries/LibJS/Interpreter.cpp
index a0248c5f9e..97645092de 100644
--- a/Libraries/LibJS/Interpreter.cpp
+++ b/Libraries/LibJS/Interpreter.cpp
@@ -151,9 +151,13 @@ void Interpreter::gather_roots(Badge<Heap>, HashTable<Cell*>& roots)
         }
     }
 
-    for (auto& this_value : m_this_stack) {
-        if (this_value.is_cell())
-            roots.set(this_value.as_cell());
+    for (auto& call_frame : m_call_stack) {
+        if (call_frame.this_value.is_cell())
+            roots.set(call_frame.this_value.as_cell());
+        for (auto& argument : call_frame.arguments) {
+            if (argument.is_cell())
+                roots.set(argument.as_cell());
+        }
     }
 }
 
diff --git a/Libraries/LibJS/Interpreter.h b/Libraries/LibJS/Interpreter.h
index fe9db39d90..897c210c32 100644
--- a/Libraries/LibJS/Interpreter.h
+++ b/Libraries/LibJS/Interpreter.h
@@ -51,6 +51,11 @@ struct ScopeFrame {
     HashMap<String, Variable> variables;
 };
 
+struct CallFrame {
+    Value this_value;
+    Vector<Value> arguments;
+};
+
 struct Argument {
     String name;
     Value value;
@@ -79,13 +84,13 @@ public:
     void enter_scope(const ScopeNode&, Vector<Argument>, ScopeType);
     void exit_scope(const ScopeNode&);
 
-    void push_this_value(Value value) { m_this_stack.append(move(value)); }
-    void pop_this_value() { m_this_stack.take_last(); }
+    CallFrame& push_call_frame() { m_call_stack.append({ js_undefined(), {} }); return m_call_stack.last(); }
+    void pop_call_frame() { m_call_stack.take_last(); }
     Value this_value() const
     {
-        if (m_this_stack.is_empty())
+        if (m_call_stack.is_empty())
             return m_global_object;
-        return m_this_stack.last();
+        return m_call_stack.last().this_value;
     }
 
     Object* string_prototype() { return m_string_prototype; }
@@ -95,7 +100,7 @@ private:
     Heap m_heap;
 
     Vector<ScopeFrame> m_scope_stack;
-    Vector<Value> m_this_stack;
+    Vector<CallFrame> m_call_stack;
 
     Object* m_global_object { nullptr };
     Object* m_string_prototype { nullptr };