From 43ff2ea8d8869f0e51db368b625d6e32753c7875 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 7 Nov 2020 11:07:17 +0100 Subject: [PATCH] LibJS: Use regular stack for VM call frames instead of Vector storage Keeping the VM call frames in a Vector could cause them to move around underneath us due to Vector resizing. Avoid this issue by allocating CallFrame objects on the stack and having the VM simply keep a list of pointers to each CallFrame, instead of the CallFrames themselves. Fixes #3830. Fixes #3951. --- Libraries/LibJS/Console.cpp | 2 +- Libraries/LibJS/Interpreter.cpp | 4 ++-- Libraries/LibJS/Runtime/Exception.cpp | 2 +- Libraries/LibJS/Runtime/Object.cpp | 8 +++++-- Libraries/LibJS/Runtime/VM.cpp | 16 ++++++++------ Libraries/LibJS/Runtime/VM.h | 31 ++++++++++++++------------- 6 files changed, 36 insertions(+), 27 deletions(-) diff --git a/Libraries/LibJS/Console.cpp b/Libraries/LibJS/Console.cpp index 4bddbcdfabe..5d549d0c47f 100644 --- a/Libraries/LibJS/Console.cpp +++ b/Libraries/LibJS/Console.cpp @@ -131,7 +131,7 @@ Vector ConsoleClient::get_trace() const auto& call_stack = m_console.global_object().vm().call_stack(); // -2 to skip the console.trace() call frame for (ssize_t i = call_stack.size() - 2; i >= 0; --i) - trace.append(call_stack[i].function_name); + trace.append(call_stack[i]->function_name); return trace; } diff --git a/Libraries/LibJS/Interpreter.cpp b/Libraries/LibJS/Interpreter.cpp index 82378b58173..07a97fd30c1 100644 --- a/Libraries/LibJS/Interpreter.cpp +++ b/Libraries/LibJS/Interpreter.cpp @@ -76,8 +76,8 @@ Value Interpreter::run(GlobalObject& global_object, const Program& program) global_call_frame.is_strict_mode = program.is_strict_mode(); if (vm().exception()) return {}; - vm().call_stack().append(move(global_call_frame)); + vm().push_call_frame(global_call_frame); auto result = program.execute(*this, global_object); vm().pop_call_frame(); return result; @@ -128,7 +128,7 @@ void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector argume if (!scope_variables_with_declaration_kind.is_empty()) { auto* block_lexical_environment = heap().allocate(global_object, move(scope_variables_with_declaration_kind), current_environment()); - vm().call_stack().last().environment = block_lexical_environment; + vm().call_frame().environment = block_lexical_environment; pushed_lexical_environment = true; } diff --git a/Libraries/LibJS/Runtime/Exception.cpp b/Libraries/LibJS/Runtime/Exception.cpp index 071a2a97ebe..8695b827204 100644 --- a/Libraries/LibJS/Runtime/Exception.cpp +++ b/Libraries/LibJS/Runtime/Exception.cpp @@ -35,7 +35,7 @@ Exception::Exception(Value value) { auto& call_stack = vm().call_stack(); for (ssize_t i = call_stack.size() - 1; i >= 0; --i) { - String function_name = call_stack[i].function_name; + String function_name = call_stack[i]->function_name; if (function_name.is_empty()) function_name = ""; m_trace.append(function_name); diff --git a/Libraries/LibJS/Runtime/Object.cpp b/Libraries/LibJS/Runtime/Object.cpp index 9bf6f1376f9..43f1e813d17 100644 --- a/Libraries/LibJS/Runtime/Object.cpp +++ b/Libraries/LibJS/Runtime/Object.cpp @@ -899,8 +899,10 @@ Value Object::invoke(const StringOrSymbol& property_name, Optional& roots) roots.set(m_last_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 (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()); } - roots.set(call_frame.environment); + roots.set(call_frame->environment); } #define __JS_ENUMERATE(SymbolName, snake_name) \ @@ -194,7 +194,9 @@ Reference VM::get_reference(const FlyString& name) Value VM::construct(Function& function, Function& new_target, Optional arguments, GlobalObject& global_object) { - auto& call_frame = push_call_frame(function.is_strict_mode()); + CallFrame call_frame; + call_frame.is_strict_mode = function.is_strict_mode(); + push_call_frame(call_frame); ArmedScopeGuard call_frame_popper = [&] { pop_call_frame(); @@ -310,7 +312,8 @@ Value VM::call_internal(Function& function, Value this_value, Optionalthis_binding_status() == LexicalEnvironment::ThisBindingStatus::Uninitialized); call_frame.environment->bind_this_value(function.global_object(), call_frame.this_value); + push_call_frame(call_frame); auto result = function.call(); pop_call_frame(); return result; diff --git a/Libraries/LibJS/Runtime/VM.h b/Libraries/LibJS/Runtime/VM.h index f4adc144f9e..8c868309062 100644 --- a/Libraries/LibJS/Runtime/VM.h +++ b/Libraries/LibJS/Runtime/VM.h @@ -114,19 +114,20 @@ public: return *m_single_ascii_character_strings[character]; } - CallFrame& push_call_frame(bool strict_mode = false) + void push_call_frame(CallFrame& call_frame) { - m_call_stack.append({ {}, js_undefined(), {}, nullptr, strict_mode }); - return m_call_stack.last(); + m_call_stack.append(&call_frame); } - void pop_call_frame() { m_call_stack.take_last(); } - CallFrame& call_frame() { return m_call_stack.last(); } - const CallFrame& call_frame() const { return m_call_stack.last(); } - const Vector& call_stack() const { return m_call_stack; } - Vector& call_stack() { return m_call_stack; } - const LexicalEnvironment* current_environment() const { return m_call_stack.last().environment; } - LexicalEnvironment* current_environment() { return m_call_stack.last().environment; } + void pop_call_frame() { m_call_stack.take_last(); } + + CallFrame& call_frame() { return *m_call_stack.last(); } + const CallFrame& call_frame() const { return *m_call_stack.last(); } + const Vector& call_stack() const { return m_call_stack; } + Vector& call_stack() { return m_call_stack; } + + const LexicalEnvironment* current_environment() const { return call_frame().environment; } + LexicalEnvironment* current_environment() { return call_frame().environment; } bool in_strict_mode() const; @@ -135,7 +136,7 @@ public: { if (m_call_stack.is_empty()) return; - for (auto& value : m_call_stack.last().arguments) + for (auto& value : call_frame().arguments) callback(value); } @@ -143,14 +144,14 @@ public: { if (m_call_stack.is_empty()) return 0; - return m_call_stack.last().arguments.size(); + return call_frame().arguments.size(); } Value argument(size_t index) const { if (m_call_stack.is_empty()) return {}; - auto& arguments = m_call_stack.last().arguments; + auto& arguments = call_frame().arguments; return index < arguments.size() ? arguments[index] : js_undefined(); } @@ -158,7 +159,7 @@ public: { if (m_call_stack.is_empty()) return &global_object; - return m_call_stack.last().this_value; + return call_frame().this_value; } Value last_value() const { return m_last_value; } @@ -241,7 +242,7 @@ private: Heap m_heap; Vector m_interpreters; - Vector m_call_stack; + Vector m_call_stack; Value m_last_value; ScopeType m_unwind_until { ScopeType::None };