diff --git a/common.gypi b/common.gypi index e6a17d3505a5..563259e21ea7 100644 --- a/common.gypi +++ b/common.gypi @@ -43,7 +43,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.32', + 'v8_embedder_string': '-node.33', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/AUTHORS b/deps/v8/AUTHORS index ef0aeedaca51..823fd85d63f7 100644 --- a/deps/v8/AUTHORS +++ b/deps/v8/AUTHORS @@ -286,6 +286,7 @@ Sergey Markelov Shawn Anastasio Shawn Presser Sho Miyamoto +Shumaf Lovpache Stefan Penner Stefan Stojanovic Stephan Hartmann diff --git a/deps/v8/src/debug/debug-interface.cc b/deps/v8/src/debug/debug-interface.cc index ee13330c84e0..9e9b1060e0b0 100644 --- a/deps/v8/src/debug/debug-interface.cc +++ b/deps/v8/src/debug/debug-interface.cc @@ -774,7 +774,13 @@ bool Script::SetInstrumentationBreakpoint(BreakpointId* id) const { i::SharedFunctionInfo::ScriptIterator it(isolate, *script); for (i::Tagged sfi = it.Next(); !sfi.is_null(); sfi = it.Next()) { - if (sfi->is_toplevel()) { + // Node.js compiles CJS modules via ScriptCompiler::CompileFunction so that + // module-local bindings like __filename can be injected as function + // parameters without leaking into the global scope. The resulting Script + // carries two SFIs: a synthetic toplevel that just returns the wrapped + // function, and the wrapped SFI that the embedder actually invokes. For + // such scripts we should pick the wrapped SFI. + if (script->is_wrapped() ? sfi->is_wrapped() : sfi->is_toplevel()) { return isolate->debug()->SetBreakpointForFunction( handle(sfi, isolate), isolate->factory()->empty_string(), id, internal::Debug::kInstrumentation); diff --git a/deps/v8/test/inspector/debugger/set-instrumentation-breakpoint-expected.txt b/deps/v8/test/inspector/debugger/set-instrumentation-breakpoint-expected.txt index c0fb6f18a488..c1506a1d35ff 100644 --- a/deps/v8/test/inspector/debugger/set-instrumentation-breakpoint-expected.txt +++ b/deps/v8/test/inspector/debugger/set-instrumentation-breakpoint-expected.txt @@ -61,6 +61,37 @@ remove breakpoint.. evaluate script without sourceMappingURL.. evaluate script with sourceMappingURL.. +Running test: testWrappedScriptInstrumentation +set breakpoint and run wrapped script.. +paused with reason: instrumentation +{ + scriptId : + url : cjs-module.js +} +remove breakpoint.. +{ + id : + result : { + } +} + +Running test: testWrappedScriptWithSourceMap +set breakpoint for scriptWithSourceMapParsed.. +run wrapped script without sourceMappingURL.. +run wrapped script with sourceMappingURL.. +paused with reason: instrumentation +{ + scriptId : + sourceMapURL : cjs.js.map + url : cjs-sourcemapped.js +} +remove breakpoint.. +{ + id : + result : { + } +} + Running test: testBlackboxing set breakpoint and evaluate blackboxed script.. evaluate not blackboxed script.. diff --git a/deps/v8/test/inspector/debugger/set-instrumentation-breakpoint.js b/deps/v8/test/inspector/debugger/set-instrumentation-breakpoint.js index c5939bdab0a3..17a04f6fbccb 100644 --- a/deps/v8/test/inspector/debugger/set-instrumentation-breakpoint.js +++ b/deps/v8/test/inspector/debugger/set-instrumentation-breakpoint.js @@ -79,6 +79,48 @@ InspectorTest.runAsyncTestSuite([ await Protocol.Debugger.disable(); }, + async function testWrappedScriptInstrumentation() { + await Protocol.Debugger.enable(); + InspectorTest.log('set breakpoint and run wrapped script..'); + const { result : firstResult } = await Protocol.Debugger.setInstrumentationBreakpoint({ + instrumentation: 'beforeScriptExecution' + }); + utils.compileAndRunWrapped(contextGroup.id, '1 + 2', 'cjs-module.js'); + { + const { params: { reason, data } } = await Protocol.Debugger.oncePaused(); + InspectorTest.log(`paused with reason: ${reason}`); + InspectorTest.logMessage(data); + } + await Protocol.Debugger.resume(); + InspectorTest.log('remove breakpoint..'); + InspectorTest.logMessage(await Protocol.Debugger.removeBreakpoint({ + breakpointId: firstResult.breakpointId + })); + await Protocol.Debugger.disable(); + }, + + async function testWrappedScriptWithSourceMap() { + await Protocol.Debugger.enable(); + InspectorTest.log('set breakpoint for scriptWithSourceMapParsed..'); + const { result : firstResult } = await Protocol.Debugger.setInstrumentationBreakpoint({ + instrumentation: 'beforeScriptWithSourceMapExecution' + }); + InspectorTest.log('run wrapped script without sourceMappingURL..'); + utils.compileAndRunWrapped(contextGroup.id, '1 + 2', 'cjs-plain.js'); + InspectorTest.log('run wrapped script with sourceMappingURL..'); + utils.compileAndRunWrapped(contextGroup.id, '1 + 2\n//# sourceMappingURL=cjs.js.map', 'cjs-sourcemapped.js'); + { + const { params: { reason, data } } = await Protocol.Debugger.oncePaused(); + InspectorTest.log(`paused with reason: ${reason}`); + InspectorTest.logMessage(data); + } + InspectorTest.log('remove breakpoint..'); + InspectorTest.logMessage(await Protocol.Debugger.removeBreakpoint({ + breakpointId: firstResult.breakpointId + })); + await Protocol.Debugger.disable(); + }, + async function testBlackboxing() { await Protocol.Debugger.enable(); await Protocol.Debugger.setBlackboxPatterns({patterns: ['foo\.js']}); diff --git a/deps/v8/test/inspector/inspector-test.cc b/deps/v8/test/inspector/inspector-test.cc index 6b67b785a145..05babe326a5c 100644 --- a/deps/v8/test/inspector/inspector-test.cc +++ b/deps/v8/test/inspector/inspector-test.cc @@ -72,6 +72,9 @@ class UtilsExtension : public InspectorIsolateData::SetupGlobalTask { utils->Set(isolate, "compileAndRunWithOrigin", v8::FunctionTemplate::New( isolate, &UtilsExtension::CompileAndRunWithOrigin)); + utils->Set(isolate, "compileAndRunWrapped", + v8::FunctionTemplate::New( + isolate, &UtilsExtension::CompileAndRunWrapped)); utils->Set(isolate, "setCurrentTimeMSForTest", v8::FunctionTemplate::New( isolate, &UtilsExtension::SetCurrentTimeMSForTest)); @@ -233,6 +236,20 @@ class UtilsExtension : public InspectorIsolateData::SetupGlobalTask { info[4].As(), info[5].As())); } + static void CompileAndRunWrapped( + const v8::FunctionCallbackInfo& info) { + if (info.Length() != 3 || !info[0]->IsInt32() || !info[1]->IsString() || + !info[2]->IsString()) { + FATAL( + "Internal error: compileAndRunWrapped(context_group_id, source, " + "url)."); + } + backend_runner_->Append(std::make_unique( + info.GetIsolate(), info[0].As()->Value(), + ToVector(info.GetIsolate(), info[1].As()), + info[2].As())); + } + static void SetCurrentTimeMSForTest( const v8::FunctionCallbackInfo& info) { if (info.Length() != 1 || !info[0]->IsNumber()) { diff --git a/deps/v8/test/inspector/tasks.cc b/deps/v8/test/inspector/tasks.cc index ce8be5562a67..f098b62db061 100644 --- a/deps/v8/test/inspector/tasks.cc +++ b/deps/v8/test/inspector/tasks.cc @@ -130,5 +130,24 @@ void ExecuteStringTask::Run(InspectorIsolateData* data) { } } +void ExecuteWrappedStringTask::Run(InspectorIsolateData* data) { + v8::HandleScope handle_scope(data->isolate()); + v8::Local context = data->GetDefaultContext(context_group_id_); + v8::MicrotasksScope microtasks_scope(context, + v8::MicrotasksScope::kRunMicrotasks); + v8::Context::Scope context_scope(context); + v8::ScriptOrigin origin(ToV8String(data->isolate(), name_)); + v8::Local source = ToV8String(data->isolate(), expression_); + v8::ScriptCompiler::Source scriptSource(source, origin); + v8::Local function; + if (!v8::ScriptCompiler::CompileFunction(context, &scriptSource) + .ToLocal(&function)) { + return; + } + v8::MaybeLocal result = + function->Call(context, context->Global(), 0, nullptr); + USE(result); +} + } // namespace internal } // namespace v8 diff --git a/deps/v8/test/inspector/tasks.h b/deps/v8/test/inspector/tasks.h index 3b3e68e2e05f..600f7ccc5a3b 100644 --- a/deps/v8/test/inspector/tasks.h +++ b/deps/v8/test/inspector/tasks.h @@ -101,6 +101,27 @@ class ExecuteStringTask : public TaskRunner::Task { int context_group_id_; }; +class ExecuteWrappedStringTask : public TaskRunner::Task { + public: + ExecuteWrappedStringTask(v8::Isolate* isolate, int context_group_id, + const std::vector& expression, + v8::Local name) + : expression_(expression), + name_(ToVector(isolate, name)), + context_group_id_(context_group_id) {} + + ~ExecuteWrappedStringTask() override = default; + ExecuteWrappedStringTask(const ExecuteWrappedStringTask&) = delete; + ExecuteWrappedStringTask& operator=(const ExecuteWrappedStringTask&) = delete; + bool is_priority_task() override { return false; } + void Run(InspectorIsolateData* data) override; + + private: + std::vector expression_; + std::vector name_; + int context_group_id_; +}; + class SetTimeoutTask : public TaskRunner::Task { public: SetTimeoutTask(int context_group_id, v8::Isolate* isolate, diff --git a/test/known_issues/test-inspector-instrumentation-breakpoint.js b/test/known_issues/test-inspector-instrumentation-breakpoint.js deleted file mode 100644 index 0105097f2baf..000000000000 --- a/test/known_issues/test-inspector-instrumentation-breakpoint.js +++ /dev/null @@ -1,54 +0,0 @@ -// This test validates inspector's Debugger.setInstrumentationBreakpoint method. -// Refs: https://github.com/nodejs/node/issues/31138 - -'use strict'; -const common = require('../common'); - -common.skipIfInspectorDisabled(); - -const assert = require('assert'); -const { resolve: UrlResolve } = require('url'); -const fixtures = require('../common/fixtures'); -const { NodeInstance } = require('../common/inspector-helper.js'); - -async function testBreakpointBeforeScriptExecution(session) { - console.log('[test]', - 'Verifying debugger stops on start of each script ' + - '(Debugger.setInstrumentationBreakpoint with beforeScriptExecution)'); - const commands = [ - { 'method': 'Runtime.enable' }, - { 'method': 'Debugger.enable' }, - { 'method': 'Debugger.setInstrumentationBreakpoint', - 'params': { 'instrumentation': 'beforeScriptExecution' } }, - { 'method': 'Runtime.runIfWaitingForDebugger' }, - ]; - - await session.send(commands); - - // Break on start - await session.waitForBreakOnLine( - 0, UrlResolve(session.scriptURL().toString(), 'main.js')); - await session.send([{ 'method': 'Debugger.resume' }]); - - // Script loaded - await session.waitForBreakOnLine( - 0, UrlResolve(session.scriptURL().toString(), 'main.js')); - await session.send([{ 'method': 'Debugger.resume' }]); - - // Script loaded - await session.waitForBreakOnLine( - 0, UrlResolve(session.scriptURL().toString(), 'dep.js')); - await session.send([{ 'method': 'Debugger.resume' }]); -} - -async function runTest() { - const main = fixtures.path('inspector-instrumentation-breakpoint', 'main.js'); - const child = new NodeInstance(['--inspect-brk=0'], '', main); - - const session = await child.connectInspectorSession(); - await testBreakpointBeforeScriptExecution(session); - await session.runToCompletion(); - assert.strictEqual((await child.expectShutdown()).exitCode, 0); -} - -runTest(); diff --git a/test/parallel/test-inspector-instrumentation-breakpoint.js b/test/parallel/test-inspector-instrumentation-breakpoint.js new file mode 100644 index 000000000000..3a905a58ec0b --- /dev/null +++ b/test/parallel/test-inspector-instrumentation-breakpoint.js @@ -0,0 +1,59 @@ +'use strict'; + +const common = require('../common'); +common.skipIfInspectorDisabled(); + +const assert = require('node:assert/strict'); + +const fixtures = require('../common/fixtures'); +const { NodeInstance } = require('../common/inspector-helper'); + +async function testBreakpointBeforeScriptExecution(session) { + console.log( + '[test]', + 'Verifying debugger stops on start of each script ' + + '(Debugger.setInstrumentationBreakpoint with beforeScriptExecution)', + ); + + const commands = [ + { method: 'Runtime.enable' }, + { method: 'Debugger.enable' }, + { + method: 'Debugger.setInstrumentationBreakpoint', + params: { instrumentation: 'beforeScriptExecution' }, + }, + { method: 'Runtime.runIfWaitingForDebugger' }, + ]; + + await session.send(commands); + + const mainURL = new URL('main.js', session.scriptURL()).href; + + // Break on start. + await session.waitForBreakOnLine(3, 'node:internal/main/run_main_module'); + await session.send([{ method: 'Debugger.resume' }]); + + // Script loaded. + await session.waitForBreakOnLine(0, mainURL); + await session.send([{ method: 'Debugger.resume' }]); + + // Dependency loaded. + await session.waitForBreakOnLine(0, mainURL); +} + +async function runTest() { + const main = fixtures.path( + 'inspector-instrumentation-breakpoint', + 'main.js', + ); + + const child = new NodeInstance(['--inspect-brk=0'], '', main); + const session = await child.connectInspectorSession(); + + await testBreakpointBeforeScriptExecution(session); + await session.runToCompletion(); + + assert.strictEqual((await child.expectShutdown()).exitCode, 0); +} + +runTest();