From 842fedcea617bd80373c783dd55bad57406c11a5 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 19 May 2017 19:59:03 +0200 Subject: [PATCH 1/4] src: use explicit imports Replace `using namespace lldb` with explicit `using lldb::` imports. --- src/llnode.cc | 14 +++++++++++++- src/llscan.cc | 14 ++++++++++++-- src/llv8-constants.cc | 9 ++++++++- src/llv8.cc | 4 +++- 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/llnode.cc b/src/llnode.cc index d7eb70f2..80eb13fe 100644 --- a/src/llnode.cc +++ b/src/llnode.cc @@ -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; diff --git a/src/llscan.cc b/src/llscan.cc index 821068c1..ddc7d935 100644 --- a/src/llscan.cc +++ b/src/llscan.cc @@ -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) { diff --git a/src/llv8-constants.cc b/src/llv8-constants.cc index 93cca3c5..d36cec53 100644 --- a/src/llv8-constants.cc +++ b/src/llv8-constants.cc @@ -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_"; diff --git a/src/llv8.cc b/src/llv8.cc index af1ba80c..e59591f5 100644 --- a/src/llv8.cc +++ b/src/llv8.cc @@ -8,7 +8,9 @@ namespace llnode { namespace v8 { -using namespace lldb; +using lldb::SBError; +using lldb::SBTarget; +using lldb::addr_t; static std::string kConstantPrefix = "v8dbg_"; From 219585c216cfc1e6aaf209a4637e5b429aba64a9 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 20 Jun 2017 13:41:51 +0200 Subject: [PATCH 2/4] 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: https://github.com/nodejs/node/pull/13374 --- test/scan-test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/scan-test.js b/test/scan-test.js index b90ec05b..496c3f58 100644 --- a/test/scan-test.js +++ b/test/scan-test.js @@ -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'); From 83ff88b1cd8912ef7166c862df3484fbd56f20ba Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 20 Jun 2017 19:57:22 +0200 Subject: [PATCH 3/4] 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: https://github.com/nodejs/llnode/issues/99 --- src/llnode.cc | 50 +++++++++++++++++++++++++++------------------- test/frame-test.js | 4 ++++ 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/llnode.cc b/src/llnode.cc index 80eb13fe..3ddcb731 100644 --- a/src/llnode.cc +++ b/src/llnode.cc @@ -17,6 +17,7 @@ using lldb::SBDebugger; using lldb::SBError; using lldb::SBExpressionOptions; using lldb::SBFrame; +using lldb::SBMemoryRegionInfo; using lldb::SBStream; using lldb::SBSymbol; using lldb::SBTarget; @@ -109,30 +110,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(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(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. + { + SBMemoryRegionInfo info; + if (target.GetProcess().GetMemoryRegionInfo(pc, info).Success() && + info.IsExecutable() && info.IsWritable()) { + result.Printf(" %c frame #%u: 0x%016llx \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(frame.GetPC()), - res.c_str()); + // C++ stack frame. + SBStream desc; + if (frame.GetDescription(desc)) + result.Printf(" %c %s", star, desc.GetData()); } result.SetStatus(eReturnStatusSuccessFinishResult); diff --git a/test/frame-test.js b/test/frame-test.js index a9c4068f..69861344 100644 --- a/test/frame-test.js +++ b/test/frame-test.js @@ -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) => !//.test(s)); const eyecatcher = lines[0]; const adapter = lines[1]; const crasher = lines[2]; From 838e3d72477763bb171914aa623df7eec1279b3b Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 29 Jun 2017 14:56:31 +0200 Subject: [PATCH 4/4] fixup! SBMemoryRegionInfo is lldb >= 3.9 --- src/llnode.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/llnode.cc b/src/llnode.cc index 3ddcb731..ec2aa36b 100644 --- a/src/llnode.cc +++ b/src/llnode.cc @@ -17,7 +17,6 @@ using lldb::SBDebugger; using lldb::SBError; using lldb::SBExpressionOptions; using lldb::SBFrame; -using lldb::SBMemoryRegionInfo; using lldb::SBStream; using lldb::SBSymbol; using lldb::SBTarget; @@ -128,7 +127,7 @@ bool BacktraceCmd::DoExecute(SBDebugger d, char** cmd, // 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. { - SBMemoryRegionInfo info; + lldb::SBMemoryRegionInfo info; if (target.GetProcess().GetMemoryRegionInfo(pc, info).Success() && info.IsExecutable() && info.IsWritable()) { result.Printf(" %c frame #%u: 0x%016llx \n", star, i, pc);