Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of variables + for loop + switch in a generator function causes inspector to crash #11746

Closed
matthewp opened this issue Mar 8, 2017 · 6 comments
Labels
confirmed-bug Issues with confirmed bugs. inspector Issues and PRs related to the V8 inspector protocol v8 engine Issues and PRs related to the V8 dependency.

Comments

@matthewp
Copy link

matthewp commented Mar 8, 2017

  • Version: 7.6.0
  • Platform: Darwin Matthews-MBP-2.lan 16.3.0 Darwin Kernel Version 16.3.0: Thu Nov 17 20:23:58
  • Subsystem: Unknown

I was able to reduce it down to this code:

tcase.js

function* serialize() {
  for(let i = 0; i < 10; i++) {
    let value;
    let section = {};

    debugger;

    switch('foo') {
      case 'bar':
        let items = [];
        break;
    }
  }
}

let gen = serialize();
gen.next();

Run with node --debug-brk --inspect tcase.js. Stop on the debugger and then click to step over. It will crash with 1] 50320 illegal hardware instruction node --debug-brk --inspect tcase.js.

I dumped the output into this gist.

@targos
Copy link
Member

targos commented Mar 8, 2017

I can reproduce on master

@targos targos added inspector Issues and PRs related to the V8 inspector protocol v8 engine Issues and PRs related to the V8 dependency. labels Mar 8, 2017
@mscdex mscdex added the confirmed-bug Issues with confirmed bugs. label Mar 8, 2017
@targos
Copy link
Member

targos commented Mar 8, 2017

Backtrace with a debug build:

(gdb) bt
#0  v8::base::OS::Abort () at ../deps/v8/src/base/platform/platform-posix.cc:230
#1  0x00000000026fb2b7 in V8_Fatal (file=0x293a348 "../deps/v8/src/debug/debug-scopes.cc", line=257, format=0x293983a "Check failed: %s.") at ../deps/v8/src/base/logging.cc:67
#2  0x0000000001cc562f in v8::internal::ScopeIterator::Type (this=0x7fffffffa4f0) at ../deps/v8/src/debug/debug-scopes.cc:257
#3  0x0000000001cc4d5f in v8::internal::ScopeIterator::MaterializeScopeDetails (this=0x7fffffffa4f0) at ../deps/v8/src/debug/debug-scopes.cc:179
#4  0x000000000210307a in v8::internal::__RT_impl_Runtime_GetAllScopesDetails (args=..., isolate=0x3863600) at ../deps/v8/src/runtime/runtime-debug.cc:886
#5  0x0000000002102ca7 in v8::internal::Runtime_GetAllScopesDetails (args_length=4, args_object=0x7fffffffaa48, isolate=0x3863600) at ../deps/v8/src/runtime/runtime-debug.cc:861
#6  0x0000273d428843a7 in ?? ()
#7  0x0000273d429dfa8e in ?? ()
#8  0x0000273d428842e1 in ?? ()
#9  0x00007fffffffaa00 in ?? ()
#10 0x0000000300000000 in ?? ()
#11 0x00007fffffffaa78 in ?? ()
#12 0x0000273d429df5f5 in ?? ()
#13 0x0000140895d02421 in ?? ()
#14 0x0000000000000000 in ?? ()

@matthewp matthewp changed the title Use of variables + for loop + switch in a generator function cases inspector to crash Use of variables + for loop + switch in a generator function causes inspector to crash Mar 8, 2017
@hashseed
Copy link
Member

Will take a look.

@hashseed
Copy link
Member

I can reproduce this in d8. File V8 bug here: https://bugs.chromium.org/p/v8/issues/detail?id=6085

@hashseed
Copy link
Member

Fix upcoming.

@hashseed
Copy link
Member

https://chromium.googlesource.com/v8/v8.git/+/09de9969ccb9bc3bbd667788afad665b309d02f5

fhinkel added a commit to fhinkel/node that referenced this issue Mar 17, 2017
Original commit message:
  [debugger] fix switch block source positions.

  The switch statement itself is part of the switch block.
  However, the source position of the statement is outside of
  the block. This leads to confusion for the debugger, if the
  switch block pushes a block context: the current context is
  a block context, but the scope analysis based on the current
  source position tells the debugger that we should be outside
  the scope, so we should have the function context.

  [email protected]
  BUG=v8:6085
  Review-Url: https://codereview.chromium.org/2744213003
  Cr-Commit-Position: refs/heads/master@{nodejs#43744}
  Committed: https://chromium.googlesource.com/v8/v8/+/09de9969ccb9bc3bbd667788afad665b309d02f5

Fixes: nodejs#11746
@fhinkel fhinkel closed this as completed Mar 19, 2017
fhinkel added a commit that referenced this issue Mar 19, 2017
Original commit message:
  [debugger] fix switch block source positions.

  The switch statement itself is part of the switch block.
  However, the source position of the statement is outside of
  the block. This leads to confusion for the debugger, if the
  switch block pushes a block context: the current context is
  a block context, but the scope analysis based on the current
  source position tells the debugger that we should be outside
  the scope, so we should have the function context.

  [email protected]
  BUG=v8:6085
  Review-Url: https://codereview.chromium.org/2744213003
  Cr-Commit-Position: refs/heads/master@{#43744}
  Committed: https://chromium.googlesource.com/v8/v8/+/09de9969ccb9bc3bbd667788afad665b309d02f5

Fixes: #11746
PR-URL: #11905
Fixes: #11746
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
jungx098 pushed a commit to jungx098/node that referenced this issue Mar 21, 2017
Original commit message:
  [debugger] fix switch block source positions.

  The switch statement itself is part of the switch block.
  However, the source position of the statement is outside of
  the block. This leads to confusion for the debugger, if the
  switch block pushes a block context: the current context is
  a block context, but the scope analysis based on the current
  source position tells the debugger that we should be outside
  the scope, so we should have the function context.

  [email protected]
  BUG=v8:6085
  Review-Url: https://codereview.chromium.org/2744213003
  Cr-Commit-Position: refs/heads/master@{nodejs#43744}
  Committed: https://chromium.googlesource.com/v8/v8/+/09de9969ccb9bc3bbd667788afad665b309d02f5

Fixes: nodejs#11746
PR-URL: nodejs#11905
Fixes: nodejs#11746
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. inspector Issues and PRs related to the V8 inspector protocol v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants