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

Activity

1. mmarchini commented on Oct 3, 2019

   

   mmarchini

   on Oct 3, 2019

   Last edited by mmarchini

   ContributorAuthor

   More actions

   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.
2. mmarchini commented on Oct 3, 2019

   

   mmarchini

   on Oct 3, 2019

   ContributorAuthor

   More actions

   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.
3. mmarchini

   changed the title [-]Handle `dont_adapt_arguments` behavior of <adaptor> frames[/-] [+]Handle <adaptor> frames correctly on Node.js v12[/+] on Oct 3, 2019
4. mmarchini commented on Oct 3, 2019

   

   mmarchini

   on Oct 3, 2019

   ContributorAuthor

   More actions

   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.
5. mmarchini commented on Oct 3, 2019

   

   mmarchini

   on Oct 3, 2019

   ContributorAuthor

   More actions

   The issue is fixed on LLDB 8. Closing.
6. mmarchini

   closed this as completedon Oct 3, 2019