Skip to content

Commit 92a5707

Browse files
committed
deps: cherry-pick 5ba9200 from V8 upstream
Original commit message: 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'. [email protected] Fixed: 384413079,399002824 Change-Id: Ia2d0008d574e7eaf6c06b640053df696014d37f8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6507402 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Simon Zünd <[email protected]> Cr-Commit-Position: refs/heads/main@{#100029} Refs: v8/v8@5ba9200 Fixes: #60580 PR-URL: #60620 Signed-off-by: Juan José Arboleda <[email protected]>
1 parent dd9a117 commit 92a5707

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

common.gypi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838

3939
# Reset this number to 0 on major V8 upgrades.
4040
# Increment by one for each non-official patch applied to deps/v8.
41-
'v8_embedder_string': '-node.28',
41+
'v8_embedder_string': '-node.29',
4242

4343
##### V8 defaults for Node.js #####
4444

deps/v8/src/debug/debug-scopes.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,22 @@ bool ScopeIterator::DeclaresLocals(Mode mode) const {
431431
}
432432

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

@@ -538,6 +554,11 @@ void ScopeIterator::Next() {
538554
MaybeCollectAndStoreLocalBlocklists();
539555
UnwrapEvaluationContext();
540556

557+
DCHECK_IMPLIES(current_scope_ && !current_scope_->is_function_scope() &&
558+
NeedsAndHasContext(),
559+
current_scope_->UniqueIdInScript() ==
560+
context_->scope_info()->UniqueIdInScript());
561+
541562
if (leaving_closure) function_ = Handle<JSFunction>();
542563
}
543564

0 commit comments

Comments
 (0)