nodejs / llnode

An lldb plugin for Node.js and V8, which enables inspection of JavaScript states for insights into Node.js processes and their core dumps.
Other
1.15k stars 99 forks source link

Handle <adaptor> frames correctly on Node.js v12 #306

Closed mmarchini closed 4 years ago

mmarchini commented 4 years ago

On Node.js v12.3 (with #302 and #301 and some other changes I'll submit tomorrow):

    frame #7: 0x00000000019f35c4 crasher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:4:17 fn=0x000016460dac10a1
    frame #8: 0x00000000019f35c4 <adaptor>
    frame #9: 0x00000000019f35c4 <adaptor>

It should be:

    frame #7: 0x000028ce7480e458 crasher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:4:17 fn=0x00002b44187bfb79                                
    frame #8: 0x000028ce748076a6 <adaptor>                                                                                                                                    
    frame #9: 0x000028ce7480e458 eyecatcher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:22:20 fn=0x00002b44187bfbb9                            

This is happening because FP is the same for frame 9 and frame 8. I think this is due to this commit: https://chromium-review.googlesource.com/c/v8/v8/+/1488751

From a quick skim, this PR seems to make the frame behave like -fomit-frame-pointer. If that's the case, I would expect The <adaptor> frame to be hidden, but maybe LLDB is guessing it should be loaded because the builtin is now embeded on the binary...

I'm not sure, will need to investigate more later. Opening the issue to share what I found so far.

mmarchini commented 4 years ago

This is interesting. If I change our frame-scenario.js fixture to:

// Note: top-level 'use strict' intentionally omitted.
const common = require('../common');

function crasher(unused) {
  'use strict';
  process._rawDebug(arguments);
  process.abort();  // Creates an exit frame.
  return this;      // Force definition of |this|.
}

function eyecatcher() {
  crasher(1, 2, 3);
  return this;  // Force definition of |this|.
}

eyecatcher();

I get:

    frame #7: 0x00000000019f35c4 crasher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:4:17 fn=0x000034c6a13c10c9
    frame #8: 0x00000000019f35c4 crasher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:4:17 fn=0x000034c6a13c10c9
    frame #9: 0x00000000019f35c4 eyecatcher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:23:20 fn=0x000034c6a13c1109

Which again is not what I was expecting to happen.

mmarchini commented 4 years ago

Another change to frame-scenario.js (after reading https://docs.google.com/document/d/156nh17BpyLNmDmONyC_ZQUbi0h6goNysds2DLXcAzVc/preview#):

// Note: top-level 'use strict' intentionally omitted.
const common = require('../common');

function crasher(unused) {
  'use strict';
  // process._rawDebug(arguments);
  if (unused)
    process.abort();  // Creates an exit frame.
  return this;      // Force definition of |this|.
}

function eyecatcher() {
  crasher(null, null);
  crasher(null, null);
  crasher(null, null);
  crasher(null, null);
  crasher(null, null);
  crasher(null, null);
  crasher(null, null);
  crasher(null, null);
  crasher(null, null);
  crasher(true, null);
  return this;  // Force definition of |this|.
}

eyecatcher();

And I get:

    frame #7: 0x00000000019f35c4 crasher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:4:17 fn=0x0000299e343411c9
    frame #8: 0x00000000019f35c4 eyecatcher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:24:20 fn=0x0000299e34341209

Which is what I was expecting in the previous comment. So the first time eyecatcher runs, V8 doesn't have enough information to decide if it should include a <adaptor> frame or not. Interesting. So what's causing the issue?

One last test with the original frame-scenario.js and --interpreted-frames-native-stack (to force V8 to generate JIT interpreter trampolines, instead of using the embeded one):

    frame #7: 0x000027f5adbcda64 crasher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:4:17 fn=0x00003d3f43fbbbd9
    frame #8: 0x00000000019ec5fc <adaptor>
    frame #9: 0x000027f5adbcd6a4 eyecatcher at /home/mmarchini/workspace/nodejs/llnode/test/fixtures/frame-scenario.js:24:20 fn=0x00003d3f43fbbc19

So maybe the problem is how LLDB is unwinding the stack? If that's the case I'm not sure we can do anything about it... Would be nice to confirm that's the case though. LLDB has a unwind diagnostic tool, I ran it and got the following output: https://gist.github.com/mmarchini/909e06d310e6a246a2069360b1eb6417. I didn't have time to analyze the output yet, but hopefully it will help us understand what's happening.

mmarchini commented 4 years ago

The unwind diagnostic output confirms that the problem is lldb's unwinding. There's a stack trace there using a simple stack walker, and it shows the frame correctly.

mmarchini commented 4 years ago

The issue is fixed on LLDB 8. Closing.