Skip to content

deps: V8: cherry-pick 383a6d80e857 - #65989

Open
avivkeller wants to merge 2 commits into
nodejs:mainfrom
avivkeller:v8-backport-instrumentation
Open

deps: V8: cherry-pick 383a6d80e857#65989
avivkeller wants to merge 2 commits into
nodejs:mainfrom
avivkeller:v8-backport-instrumentation

Conversation

@avivkeller

Copy link
Copy Markdown
Member

Original commit message:

[inspector] Fix instrumentation breakpoint on wrapped scripts

SetInstrumentationBreakpoint iterates the SFIs in a Script and picks
the toplevel SFI to attach the instrumentation breakpoint to. For
scripts compiled via ScriptCompiler::CompileFunction (e.g. Node.js CJS
modules), the resulting Script contains two SFIs: a synthetic toplevel
that just returns the wrapped function, and the wrapped SFI that the
embedder actually invokes. Picking the toplevel for such a script
means the breakpoint never fires when execution enters the user's
code.

Select the wrapped SFI for wrapped Scripts so that the breakpoint is
attached where execution will actually pause.

Bug: 516452552
Change-Id: I424c709655a6d5809d527f19f90dfb89e8acb7a8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7883039
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Eric Leese <leese@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107643}

Refs: v8/v8@383a6d8


Fixes: #31138
Ref: #31138 (comment)

Soarex16 and others added 2 commits September 11, 2026 15:36
Original commit message:

    [inspector] Fix instrumentation breakpoint on wrapped scripts

    SetInstrumentationBreakpoint iterates the SFIs in a Script and picks
    the toplevel SFI to attach the instrumentation breakpoint to. For
    scripts compiled via ScriptCompiler::CompileFunction (e.g. Node.js CJS
    modules), the resulting Script contains two SFIs: a synthetic toplevel
    that just returns the wrapped function, and the wrapped SFI that the
    embedder actually invokes. Picking the toplevel for such a script
    means the breakpoint never fires when execution enters the user's
    code.

    Select the wrapped SFI for wrapped Scripts so that the breakpoint is
    attached where execution will actually pause.

    Bug: 516452552
    Change-Id: I424c709655a6d5809d527f19f90dfb89e8acb7a8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7883039
    Commit-Queue: Simon Zünd <szuend@chromium.org>
    Reviewed-by: Simon Zünd <szuend@chromium.org>
    Reviewed-by: Eric Leese <leese@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#107643}

Refs: v8/v8@383a6d8
Signed-off-by: Aviv Keller <me@aviv.sh>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Sep 11, 2026
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.04%. Comparing base (a8f2bf0) to head (aa24eb7).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65989      +/-   ##
==========================================
+ Coverage   89.98%   90.04%   +0.05%     
==========================================
  Files         784      784              
  Lines      268263   268361      +98     
  Branches    51102    51141      +39     
==========================================
+ Hits       241401   241644     +243     
+ Misses      17426    17253     -173     
- Partials     9436     9464      +28     

see 37 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Debugger.setInstrumentationBreakpoint is broken

4 participants