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

Debugger misbehavior in function called via symbol property reference #7536

Closed
kkeri opened this issue Jul 4, 2016 · 7 comments
Closed

Debugger misbehavior in function called via symbol property reference #7536

kkeri opened this issue Jul 4, 2016 · 7 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@kkeri
Copy link

kkeri commented Jul 4, 2016

  • Version: v4.4.7, v6.2.2
  • Platform: Windows7 64-bit
  • Subsystem: debugger
Steps to Reproduce
  1. Create the file test.js with the following code.
  2. Invoke node debug test.js.
    The debugger stops at line 1.
  3. Type the command 'c'
var S = Symbol();
var o = {
    [S]() {
        console.log("before");
        debugger;
        console.log("at breakpoint");
        console.log("after");
    }
}
o[S]();
Expected Behavior

"before" is printed then the debugger stops at debugger;.

Actual Behavior

"before" is printed and I get a debug> prompt without any code listing.
If I type 'bt' I get the message 'Can't request backtrace now'.
If I type 'list(5)' I get a list when the current line is marked at line 1.
If I type 'c' then "at breakpoint" and "after" is printed and
the program terminates without ever stopping at the breakpoint.

If I change the last line as follows, everything goes fine.

var S = Symbol();
var o = {
    [S]() {
        console.log("before");
        debugger;
        console.log("at breakpoint");
        console.log("after");
    }
}
var f = o[S];
f();

Node Inspector and VS 2015 Node.js tools also show weird behavior with the sample code. Effectively they are unable to stop at the breakpoint in o[S].

@bnoordhuis
Copy link
Member

Confirmed. The debugger statement generates a v8::Break debug event without a JSON payload, tricking src/debug-agent.cc into thinking that the debug session has terminated. I'll put together a fix.

@bnoordhuis
Copy link
Member

Turns out it's (also) a V8 bug. The JSON-ification of the event data fails with a "Cannot convert a Symbol value to a string" exception, that is why the payload is empty.

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

@kkeri
Copy link
Author

kkeri commented Jul 6, 2016

@bnoordhuis: Good news. I'm ready to try out the fix as soon as it lands in node.

kisg pushed a commit to paul99/v8mips that referenced this issue Jul 7, 2016
Fix a TypeError when putting together the invocationText for a symbol
method's stack frame.

See nodejs/node#7536.

Review-Url: https://codereview.chromium.org/2122793003
Cr-Commit-Position: refs/heads/master@{#37597}
@bnoordhuis
Copy link
Member

Tracking bug for back-porting the fix to the 5.2 and 5.3 branches: https://bugs.chromium.org/p/v8/issues/detail?id=5191

I'll take care of back-porting it to older branches.

@kkeri
Copy link
Author

kkeri commented Jul 8, 2016

Trying to find a workaround, I ran into a more tricky scenario. It would be nice to see if the proposed fix repairs this one as well.

This piece of code produces the same effect as my original sample. However if the breakpoint is moved into o.g, it works as expected.

var S = Symbol();
var o = {
    [S]() {
        debugger;
        console.log("in o[S]");
    },
    g() {
        console.log("in o.g");
    }
}

var f = o[S];
var g = o.g;

f.call(o);
g.call(o);

@kkeri
Copy link
Author

kkeri commented Jul 9, 2016

One more exotic test case just to make sure that it is covered.

var S = Symbol();
var o = {
    [S]() {
        debugger;
        console.log("in o[S]");
    },
}
var f = o[S].bind(o);
f();

kkeri added a commit to metamaya/metamaya that referenced this issue Jul 14, 2016
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Jul 18, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See nodejs#7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{nodejs#37597}

Fixes: nodejs#7536
bnoordhuis added a commit that referenced this issue Jul 18, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See #7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{#37597}

Fixes: #7536
PR-URL: #7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Jul 19, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See nodejs#7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{nodejs#37597}

Fixes: nodejs#7536
PR-URL: nodejs#7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
evanlucas pushed a commit that referenced this issue Jul 21, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See #7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{#37597}

Fixes: #7536
PR-URL: #7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit to ibmruntimes/node that referenced this issue Jul 22, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See nodejs/node#7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{#37597}

Fixes: nodejs/node#7536
PR-URL: nodejs/node#7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
ofrobots pushed a commit to ofrobots/node that referenced this issue Aug 25, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See nodejs#7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{nodejs#37597}

Fixes: nodejs#7536
PR-URL: nodejs#7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 10, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See #7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{#37597}

Fixes: #7536
PR-URL: #7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 10, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See #7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{#37597}

Fixes: #7536
PR-URL: #7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
rvagg pushed a commit that referenced this issue Oct 18, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See #7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{#37597}

Fixes: #7536
PR-URL: #7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 26, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See #7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{#37597}

Fixes: #7536
PR-URL: #7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit to ibmruntimes/node that referenced this issue Nov 9, 2016
Original commit message:

    Handle symbols in FrameMirror#invocationText().

    Fix a TypeError when putting together the invocationText for a
    symbol method's stack frame.

    See nodejs/node#7536.

    Review-Url: https://codereview.chromium.org/2122793003
    Cr-Commit-Position: refs/heads/master@{#37597}

Fixes: nodejs/node#7536
PR-URL: nodejs/node#7612
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
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

3 participants