Skip to content

Commit

Permalink
src: print builtins and unnamed stack frames (#104)
Browse files Browse the repository at this point in the history
* src: use explicit imports
Replace `using namespace lldb` with explicit `using lldb::<name>`
imports.

* test: fix scan-test.js with node >= 8.1.0
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

* src: print builtins and unnamed stack frames
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
* fixup! SBMemoryRegionInfo is lldb >= 3.9

Fix: #99
PR-URL: #104
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
bnoordhuis authored and hhellyer committed Jun 29, 2017
1 parent 9014fd4 commit 557bb0f
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 31 deletions.
63 changes: 41 additions & 22 deletions src/llnode.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,19 @@

namespace llnode {

using namespace lldb;
using lldb::SBCommandInterpreter;
using lldb::SBCommandReturnObject;
using lldb::SBDebugger;
using lldb::SBError;
using lldb::SBExpressionOptions;
using lldb::SBFrame;
using lldb::SBStream;
using lldb::SBSymbol;
using lldb::SBTarget;
using lldb::SBThread;
using lldb::SBValue;
using lldb::eReturnStatusFailed;
using lldb::eReturnStatusSuccessFinishResult;

v8::LLV8 llv8;

Expand Down Expand Up @@ -101,30 +113,37 @@ bool BacktraceCmd::DoExecute(SBDebugger d, char** cmd,
if (number != -1) num_frames = number;
for (uint32_t i = 0; i < num_frames; i++) {
SBFrame frame = thread.GetFrameAtIndex(i);
SBSymbol symbol = frame.GetSymbol();

// C++ symbol
if (symbol.IsValid()) {
SBStream desc;
if (!frame.GetDescription(desc)) continue;
result.Printf(frame == selected_frame ? " * %s" : " %s",
desc.GetData());
continue;
const char star = (frame == selected_frame ? '*' : ' ');
const uint64_t pc = frame.GetPC();

if (!frame.GetSymbol().IsValid()) {
v8::Error err;
v8::JSFrame v8_frame(&llv8, static_cast<int64_t>(frame.GetFP()));
std::string res = v8_frame.Inspect(true, err);
if (err.Success()) {
result.Printf(" %c frame #%u: 0x%016llx %s\n", star, i, pc,
res.c_str());
continue;
}
}

// V8 frame
v8::Error err;
v8::JSFrame v8_frame(&llv8, static_cast<int64_t>(frame.GetFP()));
std::string res = v8_frame.Inspect(true, err);

// Skip invalid frames
if (err.Fail()) continue;
#ifdef LLDB_SBMemoryRegionInfoList_h_
// Heuristic: a PC in WX memory is almost certainly a V8 builtin.
// TODO(bnoordhuis) Find a way to map the PC to the builtin's name.
{
lldb::SBMemoryRegionInfo info;
if (target.GetProcess().GetMemoryRegionInfo(pc, info).Success() &&
info.IsExecutable() && info.IsWritable()) {
result.Printf(" %c frame #%u: 0x%016llx <builtin>\n", star, i, pc);
continue;
}
}
#endif // LLDB_SBMemoryRegionInfoList_h_

// V8 symbol
result.Printf(frame == selected_frame ? " * frame #%u: 0x%016llx %s\n"
: " frame #%u: 0x%016llx %s\n",
i, static_cast<unsigned long long int>(frame.GetPC()),
res.c_str());
// C++ stack frame.
SBStream desc;
if (frame.GetDescription(desc))
result.Printf(" %c %s", star, desc.GetData());
}

result.SetStatus(eReturnStatusSuccessFinishResult);
Expand Down
14 changes: 12 additions & 2 deletions src/llscan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,23 @@

namespace llnode {

using lldb::SBCommandReturnObject;
using lldb::SBDebugger;
using lldb::SBError;
using lldb::SBExpressionOptions;
using lldb::SBMemoryRegionInfo;
using lldb::SBMemoryRegionInfoList;
using lldb::SBStream;
using lldb::SBTarget;
using lldb::SBValue;
using lldb::eReturnStatusFailed;
using lldb::eReturnStatusSuccessFinishResult;

// Defined in llnode.cc
extern v8::LLV8 llv8;

LLScan llscan;

using namespace lldb;


bool FindObjectsCmd::DoExecute(SBDebugger d, char** cmd,
SBCommandReturnObject& result) {
Expand Down
9 changes: 8 additions & 1 deletion src/llv8-constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,14 @@ namespace llnode {
namespace v8 {
namespace constants {

using namespace lldb;
using lldb::SBAddress;
using lldb::SBError;
using lldb::SBProcess;
using lldb::SBSymbol;
using lldb::SBSymbolContext;
using lldb::SBSymbolContextList;
using lldb::SBTarget;
using lldb::addr_t;

static std::string kConstantPrefix = "v8dbg_";

Expand Down
4 changes: 3 additions & 1 deletion src/llv8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
namespace llnode {
namespace v8 {

using namespace lldb;
using lldb::SBError;
using lldb::SBTarget;
using lldb::addr_t;

static std::string kConstantPrefix = "v8dbg_";

Expand Down
4 changes: 4 additions & 0 deletions test/frame-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ tape('v8 stack', (t) => {
sess.linesUntil(/eyecatcher/, (lines) => {
lines.reverse();
t.ok(lines.length > 4, 'frame count');
// FIXME(bnoordhuis) This can fail with versions of lldb that don't
// support the GetMemoryRegions() API; llnode won't be able to identify
// V8 builtins stack frames, it just prints them as anonymous frames.
lines = lines.filter((s) => !/<builtin>/.test(s));
const eyecatcher = lines[0];
const adapter = lines[1];
const crasher = lines[2];
Expand Down
10 changes: 5 additions & 5 deletions test/scan-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ tape('v8 findrefs and friends', (t) => {

sess.linesUntil(/lldb\-/, (lines) => {
// `class Deflate extends Zlib` makes instances show up as
// Transform objects (which Zlib inherits from) in node.js >= 8.
// Note that the version check will have to be redone for node.js >= 10
// but that is still a year out and by then llnode probably needs more
// fixups anyway.
// Transform objects (which Zlib inherits from) in node.js 8.0.0.
// That change was reverted in https://github.com/nodejs/node/pull/13374
// and released in 8.1.0.
const re =
(process.version >= 'v8.' ? /Transform\._handle/ : /Deflate\._handle/);
(process.version === 'v8.0.0' ?
/Transform\._handle/ : /Deflate\._handle/);
t.ok(re.test(lines.join('\n')), 'Should find reference');
t.ok(/Object\.holder/.test(lines.join('\n')), 'Should find reference #2');
t.ok(/\(Array\)\[1\]/.test(lines.join('\n')), 'Should find reference #3');
Expand Down

2 comments on commit 557bb0f

@avolfson
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me that the src/llscan.cc change breaks Linux installs with:
�[91m../src/llnode.cc: In member function 'virtual bool llnode::BacktraceCmd::DoExecute(lldb::SBDebugger, char**, lldb::SBCommandReturnObject&)':
../src/llnode.cc:125:34: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'uint64_t {aka long unsigned int}' [-Wformat=]
res.c_str());
^
�[0m
�[91m../src/llscan.cc:24:13: error: 'lldb::SBMemoryRegionInfo' has not been declared
using lldb::SBMemoryRegionInfo;
^
�[0m
�[91m../src/llscan.cc:25:13: error: 'lldb::SBMemoryRegionInfoList' has not been declared
using lldb::SBMemoryRegionInfoList;
^

See https://hub.docker.com/r/avolfson/purifico/builds/bygdt37urdct35jus3hfjdy/
I was able to install successfully by adding "RUN git reset HEAD~ --hard" :D

@bnoordhuis
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that. #109.

Please sign in to comment.