LibJS: Only update EC instruction pointer when pushing to EC stack

Instead of trying to keep a live reference to the bytecode interpreter's
current instruction stream iterator, we now simply copy the current
iterator whenever pushing to the ExecutionContext stack.

This fixes a stack-use-after-return issue reported by ASAN.
This commit is contained in:
Andreas Kling 2023-09-02 17:38:17 +02:00
parent 2966188ea3
commit c78506d79b
Notes: sideshowbarker 2024-07-17 09:47:09 +09:00
4 changed files with 19 additions and 15 deletions

View file

@ -201,7 +201,6 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu
for (;;) {
auto pc = InstructionStreamIterator { m_current_block->instruction_stream(), m_current_executable };
TemporaryChange temp_change { m_pc, Optional<InstructionStreamIterator&>(pc) };
TemporaryChange context_change { vm().running_execution_context().instruction_stream_iterator, Optional<InstructionStreamIterator&>(pc) };
// FIXME: This is getting kinda spaghetti-y
bool will_jump = false;

View file

@ -44,7 +44,7 @@ public:
// Non-standard: This points at something that owns this ExecutionContext, in case it needs to be protected from GC.
GCPtr<Cell> context_owner;
Optional<Bytecode::InstructionStreamIterator&> instruction_stream_iterator;
Optional<Bytecode::InstructionStreamIterator> instruction_stream_iterator;
DeprecatedFlyString function_name;
Value this_value;
MarkedVector<Value> arguments;

View file

@ -746,7 +746,7 @@ void VM::dump_backtrace() const
{
for (ssize_t i = m_execution_context_stack.size() - 1; i >= 0; --i) {
auto& frame = m_execution_context_stack[i];
if (frame->instruction_stream_iterator->source_code()) {
if (frame->instruction_stream_iterator.has_value() && frame->instruction_stream_iterator->source_code()) {
auto source_range = frame->instruction_stream_iterator->source_range().realize();
dbgln("-> {} @ {}:{},{}", frame->function_name, source_range.filename(), source_range.start.line, source_range.start.column);
} else {
@ -1121,4 +1121,18 @@ void VM::finish_dynamic_import(ScriptOrModule referencing_script_or_module, Modu
// 6. Return unused.
}
void VM::push_execution_context(ExecutionContext& context)
{
if (!m_execution_context_stack.is_empty())
m_execution_context_stack.last()->instruction_stream_iterator = bytecode_interpreter().instruction_stream_iterator();
m_execution_context_stack.append(&context);
}
void VM::pop_execution_context()
{
m_execution_context_stack.take_last();
if (m_execution_context_stack.is_empty() && on_call_stack_emptied)
on_call_stack_emptied();
}
}

View file

@ -93,11 +93,6 @@ public:
return m_stack_info.size_free() < 32 * KiB;
}
void push_execution_context(ExecutionContext& context)
{
m_execution_context_stack.append(&context);
}
// TODO: Rename this function instead of providing a second argument, now that the global object is no longer passed in.
struct CheckStackSpaceLimitTag { };
@ -106,16 +101,12 @@ public:
// Ensure we got some stack space left, so the next function call doesn't kill us.
if (did_reach_stack_space_limit())
return throw_completion<InternalError>(ErrorType::CallStackSizeExceeded);
m_execution_context_stack.append(&context);
push_execution_context(context);
return {};
}
void pop_execution_context()
{
m_execution_context_stack.take_last();
if (m_execution_context_stack.is_empty() && on_call_stack_emptied)
on_call_stack_emptied();
}
void push_execution_context(ExecutionContext&);
void pop_execution_context();
// https://tc39.es/ecma262/#running-execution-context
// At any point in time, there is at most one execution context per agent that is actually executing code.