Skip to content

src: print builtins and unnamed stack frames#104

Merged
hhellyer merged 4 commits intonodejs:masterfrom
bnoordhuis:fix99
Jun 29, 2017
Merged

src: print builtins and unnamed stack frames#104
hhellyer merged 4 commits intonodejs:masterfrom
bnoordhuis:fix99

Conversation

@bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Jun 21, 2017

Previously, v8 bt would exclude frames that didn't map to a C++ symbol
or a JS stack frame. llnode does not currently know how to identify the
stack frames of V8 builtins so those were omitted as well.

This commit makes those stack frames visible and introduces a heuristic
(in lldb >= 3.9) where frames whose PC is inside a WX memory segment are
assumed to belong to V8 builtins.

Fixes: #99

First commit is cleanup, second commit fixes a test, third one is the fix for #99.

@bnoordhuis
src: use explicit imports
842fedc 
Replace `using namespace lldb` with explicit `using lldb::<name>`
imports.
@bnoordhuis
test: fix scan-test.js with node >= 8.1.0
219585c 
The object change that commit b73e042 ("src,test: support node.js >= 8")
from April addressed has been reverted again in 8.1.0.  Update the test.

Refs: nodejs/node#13374
@bnoordhuis
src: print builtins and unnamed stack frames
83ff88b 
Previously, `v8 bt` would exclude frames that didn't map to a C++ symbol
or a JS stack frame.  llnode does not currently know how to identify the
stack frames of V8 builtins so those were omitted as well.

This commit makes those stack frames visible and introduces a heuristic
(in lldb >= 3.9) where frames whose PC is inside a WX memory segment are
assumed to belong to V8 builtins.

Fixes: #99
@bnoordhuis bnoordhuis mentioned this pull request Jun 21, 2017
Closed
cjihrig

This comment was marked as off-topic.

Sign in to view

indutny

This comment was marked as off-topic.

Sign in to view

indutny

This comment was marked as off-topic.

Sign in to view

@indutny
Copy link
Member

indutny commented Jun 21, 2017 •
edited
Loading

Thank you

@brendangregg
Copy link
Contributor

brendangregg commented Jun 23, 2017

+1 Tested, works, thanks.

(lldb) v8 bt
 * thread #1: tid = 13, 0x00007fda23874d44 libpthread.so.0`__GI___pthread_mutex_lock + 4, name = 'node', stop reason = signal SIGSEGV
  * frame #0: 0x00007fda23874d44 libpthread.so.0`__GI___pthread_mutex_lock + 4
    frame #1: 0x00007fda20b0ed19 node
    frame #2: 0x00007fda20ac5e39 node
    frame #3: 0x00007fda20a9dd4b node
    frame #4: 0x00007fda20a9c9f6 node
    frame #5: 0x00007fda20ffe212 node`v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) + 290
[...]

That is an improvement - better than skipping!

hhellyer

This comment was marked as off-topic.

Sign in to view

@hhellyer hhellyer merged commit 557bb0f into nodejs:master Jun 29, 2017
@bnoordhuis
Copy link
Member Author

bnoordhuis commented Jun 29, 2017

@hhellyer Next time, can you not squash commits unless they're marked as such? It buries the actual change in unrelated changes.

As well, can you use full URLs in Fixes and PR-URL tags? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@indutny indutny indutny approved these changes

+2 more reviewers

@hhellyer hhellyer hhellyer left review comments

@cjihrig cjihrig cjihrig approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

v8 bt skips some frames

5 participants

@bnoordhuis @indutny @brendangregg @cjihrig @hhellyer