From 33bfcc467bbb685339b15131ef5ac12bb7e36ca3 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 8 Sep 2024 11:12:15 +0200 Subject: [PATCH] LibJS: Don't copy current program counter into new execution contexts This didn't make any sense, and was already handled by pushing a new execution context anyway. By simply removing these bogus lines of code, we fix a bug where throwing inside a function whose bytecode was shorter than the calling function would crash trying to generate an Error stack trace (because the bytecode offset we were trying to symbolicate was actually from the longer caller function, and not valid in the callee function.) This makes --log-all-js-exceptions less crash prone and more helpful. (cherry picked from commit b3f77e47690cfd07058d824ea6f0b652489778bf) --- .../Runtime/ECMAScriptFunctionObject.cpp | 2 -- .../LibJS/Runtime/NativeFunction.cpp | 2 -- .../Tests/regress/bogus-program-counter.js | 20 +++++++++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 Userland/Libraries/LibJS/Tests/regress/bogus-program-counter.js diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp index 7a2e7c51e1a..97e9ad4c390 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp @@ -387,7 +387,6 @@ ThrowCompletionOr ECMAScriptFunctionObject::internal_call(Value this_argu // Non-standard callee_context->arguments.ensure_capacity(max(arguments_list.size(), m_formal_parameters.size())); callee_context->arguments.append(arguments_list.data(), arguments_list.size()); - callee_context->program_counter = vm.bytecode_interpreter().program_counter(); callee_context->passed_argument_count = arguments_list.size(); if (arguments_list.size() < m_formal_parameters.size()) { for (size_t i = arguments_list.size(); i < m_formal_parameters.size(); ++i) @@ -462,7 +461,6 @@ ThrowCompletionOr> ECMAScriptFunctionObject::internal_const // Non-standard callee_context->arguments.ensure_capacity(max(arguments_list.size(), m_formal_parameters.size())); callee_context->arguments.append(arguments_list.data(), arguments_list.size()); - callee_context->program_counter = vm.bytecode_interpreter().program_counter(); callee_context->passed_argument_count = arguments_list.size(); if (arguments_list.size() < m_formal_parameters.size()) { for (size_t i = arguments_list.size(); i < m_formal_parameters.size(); ++i) diff --git a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp index d04b89ef1eb..139d5845555 100644 --- a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp +++ b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp @@ -147,7 +147,6 @@ ThrowCompletionOr NativeFunction::internal_call(Value this_argument, Read // 8. Perform any necessary implementation-defined initialization of calleeContext. callee_context->this_value = this_argument; callee_context->arguments.append(arguments_list.data(), arguments_list.size()); - callee_context->program_counter = vm.bytecode_interpreter().program_counter(); callee_context->lexical_environment = caller_context.lexical_environment; callee_context->variable_environment = caller_context.variable_environment; @@ -210,7 +209,6 @@ ThrowCompletionOr> NativeFunction::internal_construct(Reado // 8. Perform any necessary implementation-defined initialization of calleeContext. callee_context->arguments.append(arguments_list.data(), arguments_list.size()); - callee_context->program_counter = vm.bytecode_interpreter().program_counter(); callee_context->lexical_environment = caller_context.lexical_environment; callee_context->variable_environment = caller_context.variable_environment; diff --git a/Userland/Libraries/LibJS/Tests/regress/bogus-program-counter.js b/Userland/Libraries/LibJS/Tests/regress/bogus-program-counter.js new file mode 100644 index 00000000000..46623cda650 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/regress/bogus-program-counter.js @@ -0,0 +1,20 @@ +test("Don't crash when throwing exception inside a callee smaller than the caller", () => { + function make() { + let o = {}; + Object.defineProperty(o, "go", { + get: function () { + return doesNotExist; + }, + }); + return o; + } + + // Some nonsense to make sure this function has a longer bytecode than the throwing getter. + function x() { + return 3; + } + function b() {} + b(x() + x() + x()); + + expect(() => make().go()).toThrowWithMessage(ReferenceError, "'doesNotExist' is not defined"); +});