Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
deps: V8: cherry-pick 5ba9200cd046
Original commit message:

    [debug] Fix crash when pausing in for-of loop header

    Given a for-of loop:

      for (const each of subject) {

    The bytecode generator emits the iterator.next call + done check +
    assigning to `each` all into the source position of `const each`.

    The pseudo-desugared code looks something like:

      var tmp;
      loop {
        var result = iterator.next();
        if (result.done) break;
        tmp = result.value;

        PushBlockContext;
        const each = tmp;

        // rest of the loop.
      }

    This is a problem, as the parser starts the block scope already on
    the `const each`. If the scope requires a context we can pause on
    bytecode that has or has not pushed the block context yet, while
    the source position looks the same.

    The recent addition of per-script unique scope IDs lets us fix
    this problem in the debugger: We can check if the scope ID of
    the runtime scope matches the parser scope. If not, the context
    was not pushed yet.

    The debugger already has a `HasContext` helper. We extend it to
    also check for matching scope IDs and then use `HasContext` where
    we would read variable values off the context. If the context was
    not pushed yet, we report them as 'unavailable'.

    R=leszeks@chromium.org

    Fixed: 384413079,399002824
    Change-Id: Ia2d0008d574e7eaf6c06b640053df696014d37f8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6507402
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Simon Zünd <szuend@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#100029}

Refs: v8/v8@5ba9200
Fixes: #60580
PR-URL: #60620
Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com>
  • Loading branch information
juanarbol authored and targos committed Dec 5, 2025
commit a12b52bf1b0ffc751a7bd16746cd6f812c8fc416
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,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.31',
'v8_embedder_string': '-node.32',

##### V8 defaults for Node.js #####

Expand Down
48 changes: 38 additions & 10 deletions deps/v8/src/debug/debug-scopes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,22 @@ bool ScopeIterator::DeclaresLocals(Mode mode) const {
}

bool ScopeIterator::HasContext() const {
// In rare cases we pause in a scope that doesn't have its context pushed yet.
// E.g. when pausing in for-of loop headers (see https://crbug.com/399002824).
//
// We can detect this by comparing the scope ID of the parsed scope and the
// runtime scope.
// We can skip this check for function scopes, those will have their context
// always pushed. Also, there is an oddity where parsing::ParseFunction
// produces function scopes with (-1, -1) as the start/end position,
// which messes up the unique ID.
if (current_scope_ && !current_scope_->is_function_scope() &&
NeedsContext() &&
current_scope_->UniqueIdInScript() !=
context_->scope_info()->UniqueIdInScript()) {
return false;
}

return !InInnerScope() || NeedsContext();
}

Expand Down Expand Up @@ -475,7 +491,7 @@ void ScopeIterator::AdvanceScope() {
DCHECK(InInnerScope());

do {
if (NeedsContext()) {
if (NeedsAndHasContext()) {
// current_scope_ needs a context so moving one scope up requires us to
// also move up one context.
AdvanceOneContext();
Expand Down Expand Up @@ -538,6 +554,11 @@ void ScopeIterator::Next() {
MaybeCollectAndStoreLocalBlocklists();
UnwrapEvaluationContext();

DCHECK_IMPLIES(current_scope_ && !current_scope_->is_function_scope() &&
NeedsAndHasContext(),
current_scope_->UniqueIdInScript() ==
context_->scope_info()->UniqueIdInScript());

if (leaving_closure) function_ = Handle<JSFunction>();
}

Expand All @@ -547,32 +568,33 @@ ScopeIterator::ScopeType ScopeIterator::Type() const {
if (InInnerScope()) {
switch (current_scope_->scope_type()) {
case FUNCTION_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsFunctionContext() ||
context_->IsDebugEvaluateContext());
DCHECK_IMPLIES(NeedsAndHasContext(),
context_->IsFunctionContext() ||
context_->IsDebugEvaluateContext());
return ScopeTypeLocal;
case MODULE_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsModuleContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsModuleContext());
return ScopeTypeModule;
case SCRIPT_SCOPE:
case REPL_MODE_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsScriptContext() ||
IsNativeContext(*context_));
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsScriptContext() ||
IsNativeContext(*context_));
return ScopeTypeScript;
case WITH_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsWithContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsWithContext());
return ScopeTypeWith;
case CATCH_SCOPE:
DCHECK(context_->IsCatchContext());
return ScopeTypeCatch;
case BLOCK_SCOPE:
case CLASS_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsBlockContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsBlockContext());
return ScopeTypeBlock;
case EVAL_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsEvalContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsEvalContext());
return ScopeTypeEval;
case SHADOW_REALM_SCOPE:
DCHECK_IMPLIES(NeedsContext(), IsNativeContext(*context_));
DCHECK_IMPLIES(NeedsAndHasContext(), IsNativeContext(*context_));
// TODO(v8:11989): New ScopeType for ShadowRealms?
return ScopeTypeScript;
}
Expand Down Expand Up @@ -962,6 +984,12 @@ bool ScopeIterator::VisitLocals(const Visitor& visitor, Mode mode,

case VariableLocation::CONTEXT:
if (mode == Mode::STACK) continue;
if (!HasContext()) {
// If the context was not yet pushed we report the variable as
// unavailable.
value = isolate_->factory()->the_hole_value();
break;
}
DCHECK(var->IsContextSlot());

DCHECK_EQ(context_->scope_info()->ContextSlotIndex(var->name()), index);
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/debug/debug-scopes.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ class V8_EXPORT_PRIVATE ScopeIterator {
bool InInnerScope() const { return !function_.is_null(); }
bool HasContext() const;
bool NeedsContext() const;
bool NeedsAndHasContext() const { return NeedsContext() && HasContext(); }
Handle<Context> CurrentContext() const {
DCHECK(HasContext());
return context_;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Don't crash when pausing on the iterator ".next" call
function crashMe() {
for (const #e of iter()) {
() => e; // Context allocate e.

35 changes: 35 additions & 0 deletions deps/v8/test/inspector/regress/regress-crbug-399002824.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2025 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

const { session, Protocol, contextGroup } =
InspectorTest.start('Don\'t crash when pausing on the iterator ".next" call');

session.setupScriptMap();
contextGroup.addScript(`
function *iter() {
yield 1;
debugger;
yield 2;
}

function crashMe() {
for (const e of iter()) {
() => e; // Context allocate e.
}
}
`);

Protocol.Debugger.enable();

(async () => {
let pausedPromise = Protocol.Debugger.oncePaused();
Protocol.Runtime.evaluate({ expression: 'crashMe()' });

let { params: { callFrames } } = await pausedPromise;
await session.logSourceLocation(callFrames[1].location);

Protocol.Debugger.resume();

InspectorTest.completeTest();
})();
Loading