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

stepping over a function containing a throw does not work #7219

Closed
weinand opened this issue Jun 8, 2016 · 4 comments
Closed

stepping over a function containing a throw does not work #7219

weinand opened this issue Jun 8, 2016 · 4 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@weinand
Copy link

weinand commented Jun 8, 2016

  • Version: 6.0.0-6.2.1
  • Platform: all
  • Subsystem: debugging

Save this snippet as throw.js:

function throwAndCatch() {
    try {
        throw new Object();
    }
    catch (e) {
    }
}

console.log("before");
throwAndCatch();
console.log("after");
console.log("the end");

Debug snippet with node builtin debugger:

node debug throw.js
  • step over console.log("before");
  • step over throwAndCatch();

Observe: the first step works fine but the second doesn't. The program runs to the end.

Commenting out the line with the throw makes stepping work.

The same problem can be observed when debugging in node-inspector or VS Code.

In node versions < 6.0 stepping was working fine.

@cjihrig
Copy link
Contributor

cjihrig commented Jun 13, 2016

Looks like this is an issue with the new inspector debugger as well.

@weinand
Copy link
Author

weinand commented Jul 6, 2016

Still a problem in 6.2.2 and 7.0.0-nightly...

@bnoordhuis
Copy link
Member

I can confirm. If you execute breakOnException before stepping over the function, the debugger will break on the exception and you can safely step out again.

I'm 95% sure it's a V8 bug because I can make it work with the small patch below:

diff --git a/deps/v8/src/debug/debug.cc b/deps/v8/src/debug/debug.cc
index 7c76742..30f79ee 100644
--- a/deps/v8/src/debug/debug.cc
+++ b/deps/v8/src/debug/debug.cc
@@ -970,6 +970,12 @@ void Debug::PrepareStepOnThrow() {

   if (it.done()) return;  // No suitable Javascript catch handler.

+  if (last_step_action() == StepNext) {
+    it.Advance();
+    if (it.done()) return;
+    if (!it.frame()->function()->shared()->IsSubjectToDebugging()) return;
+  }
+
   FloodWithOneShot(Handle<JSFunction>(it.frame()->function()));
 }

IOW, the bug appears to be that the wrong stack frame is flooded with breakpoints. I'll see if I can turn it into a patch that I can upstream.

@bnoordhuis bnoordhuis added the v8 engine Issues and PRs related to the V8 dependency. label Jul 7, 2016
@hashseed
Copy link
Member

This should be the same as crbug.com/604495 and already fixed on ToT V8?

targos added a commit to targos/node that referenced this issue Aug 14, 2016
Original commit message:

    [Debugger] Fix StepNext over function with caught exception

    Without CL debugger on StepNext adds breakpoint to function where
    throw instruction is located. In case of StepNext we will skip pause
    in this function because StepNext shouldn't break in a deeper frame.

    BUG=chromium:604495
    R=yangguo@chromium.org
    LOG=N

    Review URL: https://codereview.chromium.org/1894263002

    Cr-Commit-Position: refs/heads/master@{nodejs#35627}

Fixes: nodejs#7219
@targos targos closed this as completed in a1d3a8d Aug 17, 2016
ofrobots pushed a commit to ofrobots/node that referenced this issue Aug 25, 2016
Original commit message:

    [Debugger] Fix StepNext over function with caught exception

    Without CL debugger on StepNext adds breakpoint to function where
    throw instruction is located. In case of StepNext we will skip pause
    in this function because StepNext shouldn't break in a deeper frame.

    BUG=chromium:604495
    R=yangguo@chromium.org
    LOG=N

    Review URL: https://codereview.chromium.org/1894263002

    Cr-Commit-Position: refs/heads/master@{nodejs#35627}

Fixes: nodejs#7219
PR-URL: nodejs#8099
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

5 participants