From d70aa6e72393f1bc4f085a0c3f3519057082bd59 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Tue, 22 May 2018 14:05:02 -0300 Subject: [PATCH 1/6] src: fix context inpection for V8 6.8 V8 6.8 replaces Closure inside Context with ScopeInfo. Those are minimal changes to adopt llnode to the new behavior. Ref: https://chromium-review.googlesource.com/#/c/785151/ Fixes: https://github.com/nodejs/llnode/issues/193 --- src/llv8-constants.cc | 5 +++-- src/llv8-constants.h | 1 + src/llv8.cc | 26 ++++++++++++++++++-------- src/llv8.h | 5 ++++- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/llv8-constants.cc b/src/llv8-constants.cc index 8d100d0d..b9400156 100644 --- a/src/llv8-constants.cc +++ b/src/llv8-constants.cc @@ -244,8 +244,9 @@ void ScopeInfo::Load() { void Context::Load() { - kClosureIndex = - LoadConstant("class_Context__closure_index__int", "context_idx_closure"); + kClosureIndex = LoadConstant("class_Context__closure_index__int", + "context_idx_closure", -1); + kScopeInfoIndex = LoadConstant("context_idx_scope_info", -1); kPreviousIndex = LoadConstant("class_Context__previous_index__int", "context_idx_prev"); // TODO (mmarchini) change LoadConstant to accept variable arguments, a list diff --git a/src/llv8-constants.h b/src/llv8-constants.h index 47ed84ab..0cb00ba2 100644 --- a/src/llv8-constants.h +++ b/src/llv8-constants.h @@ -199,6 +199,7 @@ class Context : public Module { CONSTANTS_DEFAULT_METHODS(Context); int64_t kClosureIndex; + int64_t kScopeInfoIndex; int64_t kGlobalObjectIndex; int64_t kPreviousIndex; int64_t kNativeIndex; diff --git a/src/llv8.cc b/src/llv8.cc index efec4cfa..60cf5571 100644 --- a/src/llv8.cc +++ b/src/llv8.cc @@ -1047,6 +1047,18 @@ std::string FixedArray::InspectContents(int length, Error& err) { return res; } +HeapObject Context::GetScopeInfo(Error& err) { + if (v8()->context()->kScopeInfoIndex != -1) { + return FixedArray::Get(v8()->context()->kScopeInfoIndex, err); + } + JSFunction closure = Closure(err); + if (err.Fail()) return HeapObject(); + + SharedFunctionInfo info = closure.Info(err); + if (err.Fail()) return HeapObject(); + + return info.GetScopeInfo(err); +} std::string Context::Inspect(Error& err) { std::string res; @@ -1058,13 +1070,7 @@ std::string Context::Inspect(Error& err) { Value previous = Previous(err); if (err.Fail()) return std::string(); - JSFunction closure = Closure(err); - if (err.Fail()) return std::string(); - - SharedFunctionInfo info = closure.Info(err); - if (err.Fail()) return std::string(); - - HeapObject scope_obj = info.GetScopeInfo(err); + HeapObject scope_obj = GetScopeInfo(err); if (err.Fail()) return std::string(); ScopeInfo scope(scope_obj); @@ -1086,7 +1092,11 @@ std::string Context::Inspect(Error& err) { } if (!res.empty()) res += "\n"; - { + + if (v8()->context()->kClosureIndex != -1) { + JSFunction closure; + closure = Closure(err); + if (err.Fail()) return std::string(); char tmp[128]; snprintf(tmp, sizeof(tmp), " (closure)=0x%016" PRIx64 " {", closure.raw()); diff --git a/src/llv8.h b/src/llv8.h index 2d9ef37d..96818c6e 100644 --- a/src/llv8.h +++ b/src/llv8.h @@ -382,7 +382,7 @@ class Context : public FixedArray { public: V8_VALUE_DEFAULT_METHODS(Context, FixedArray) - inline JSFunction Closure(Error& err); + inline HeapObject GetScopeInfo(Error& err); inline Value Previous(Error& err); inline Value Native(Error& err); inline bool IsNative(Error& err); @@ -391,6 +391,9 @@ class Context : public FixedArray { inline Value ContextSlot(int index, Error& err); std::string Inspect(Error& err); + + private: + inline JSFunction Closure(Error& err); }; class ScopeInfo : public FixedArray { From 63ec96a79c2265660e96f3780fe0f08afaf698e4 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Fri, 27 Jul 2018 16:40:53 -0700 Subject: [PATCH 2/6] fixup! src: fix context inpection for V8 6.8 --- src/llv8.cc | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/llv8.cc b/src/llv8.cc index 60cf5571..c9919e89 100644 --- a/src/llv8.cc +++ b/src/llv8.cc @@ -392,7 +392,7 @@ std::string JSFunction::Inspect(InspectOptions* options, Error& err) { std::string context_str = context.Inspect(err); if (err.Fail()) return std::string(); - if (!context_str.empty()) res += "{\n" + context_str + "}"; + if (!context_str.empty()) res += ":" + context_str; if (options->print_source) { SharedFunctionInfo info = Info(err); @@ -1061,11 +1061,12 @@ HeapObject Context::GetScopeInfo(Error& err) { } std::string Context::Inspect(Error& err) { - std::string res; // Not enough postmortem information, return bare minimum if (v8()->shared_info()->kScopeInfoOffset == -1 && v8()->shared_info()->kNameOrScopeInfoOffset == -1) - return res; + return std::string(); + + std::string res = ","; } if (!res.empty()) res += "\n"; if (v8()->context()->kClosureIndex != -1) { - JSFunction closure; - closure = Closure(err); + JSFunction closure = Closure(err); if (err.Fail()) return std::string(); char tmp[128]; snprintf(tmp, sizeof(tmp), " (closure)=0x%016" PRIx64 " {", @@ -1105,6 +1105,21 @@ std::string Context::Inspect(Error& err) { InspectOptions options; res += closure.Inspect(&options, err) + "}"; if (err.Fail()) return std::string(); + } else { + char tmp[128]; + snprintf(tmp, sizeof(tmp), " (scope_info)=0x%016" PRIx64, + scope.raw()); + + res += std::string(tmp) + ":"; } From d680b8059a31f3ff54cc1d1ad490fdee605548c3 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Mon, 20 Aug 2018 14:21:06 -0300 Subject: [PATCH 3/6] fixup! fixup! src: fix context inpection for V8 6.8 --- test/plugin/inspect-test.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/plugin/inspect-test.js b/test/plugin/inspect-test.js index ed4da485..0d4dbcdb 100644 --- a/test/plugin/inspect-test.js +++ b/test/plugin/inspect-test.js @@ -51,7 +51,7 @@ const hashMapTests = { const arrowSource = 'source:\n' + 'function c.hashmap.(anonymous function)(a,b)=>{a+b}\n' + '>'; - + t.ok(lines.includes(arrowSource), 'hashmap[25] should have the correct function source'); cb(null); @@ -305,14 +305,22 @@ const hashMapTests = { const contextTests = { 'previous': { - re: /\(previous\)/, + re: /\(previous\)=(0x[0-9a-f]+)[^\n]+/, desc: '.(previous)' }, 'closure': { - re: /\(closure\)=(0x[0-9a-f]+)[^\n]+function: closure/i, + re: /(\((?:closure|scope_info)\)=0x[0-9a-f]+)[^\n]+/i, desc: '.(closure)', validator(t, sess, addresses, name, cb) { - const address = addresses[name]; + const type = addresses[name].split("=")[0]; + let address = undefined; + if (type === "(closure)") { + address = addresses[name].split("=")[1]; + } else if (type === "(scope_info)") { + address = addresses["previous"]; + } else { + return cb(new Error("unknown field")); + } sess.send(`v8 inspect ${address}`); sess.linesUntil(/}>/, (err, lines) => { if (err) return cb(err); From 8676013d3a7040ddf7b1b74d732b19121660cb87 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Mon, 20 Aug 2018 14:52:27 -0300 Subject: [PATCH 4/6] src: inspect context objects (Node.js v10.x+) This patch teaches llnode how to inspect Context objects. This is useful to inspect context variables. Fix: https://github.com/nodejs/llnode/issues/211 --- src/llv8-constants.cc | 4 ++++ src/llv8-constants.h | 4 ++++ src/llv8.cc | 46 +++++++++++++++++++++++++++++-------------- src/llv8.h | 10 ++++++++-- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/llv8-constants.cc b/src/llv8-constants.cc index b9400156..16d2eebb 100644 --- a/src/llv8-constants.cc +++ b/src/llv8-constants.cc @@ -487,6 +487,9 @@ void Types::Load() { kFirstJSObjectType = LoadConstant("type_JSGlobalObject__JS_GLOBAL_OBJECT_TYPE"); + kFirstContextType = LoadConstant("FirstContextType"); + kLastContextType = LoadConstant("LastContextType"); + kHeapNumberType = LoadConstant("type_HeapNumber__HEAP_NUMBER_TYPE"); kMapType = LoadConstant("type_Map__MAP_TYPE"); kGlobalObjectType = @@ -508,6 +511,7 @@ void Types::Load() { kSharedFunctionInfoType = LoadConstant("type_SharedFunctionInfo__SHARED_FUNCTION_INFO_TYPE"); kScriptType = LoadConstant("type_Script__SCRIPT_TYPE"); + kScopeInfoType = LoadConstant("type_ScopeInfo__SCOPE_INFO_TYPE"); if (kJSAPIObjectType == -1) { common_->Load(); diff --git a/src/llv8-constants.h b/src/llv8-constants.h index 0cb00ba2..d093a5e6 100644 --- a/src/llv8-constants.h +++ b/src/llv8-constants.h @@ -466,6 +466,9 @@ class Types : public Module { int64_t kFirstNonstringType; int64_t kFirstJSObjectType; + int64_t kFirstContextType; + int64_t kLastContextType; + int64_t kHeapNumberType; int64_t kMapType; int64_t kGlobalObjectType; @@ -484,6 +487,7 @@ class Types : public Module { int64_t kJSDateType; int64_t kSharedFunctionInfoType; int64_t kScriptType; + int64_t kScopeInfoType; protected: void Load(); diff --git a/src/llv8.cc b/src/llv8.cc index c9919e89..a2e24691 100644 --- a/src/llv8.cc +++ b/src/llv8.cc @@ -389,10 +389,15 @@ std::string JSFunction::Inspect(InspectOptions* options, Error& err) { snprintf(tmp, sizeof(tmp), "\n context=0x%016" PRIx64, context.raw()); res += tmp; - std::string context_str = context.Inspect(err); - if (err.Fail()) return std::string(); + { + InspectOptions ctx_options; + ctx_options.detailed = true; + ctx_options.indent_depth = options->indent_depth + 1; + std::string context_str = context.Inspect(&ctx_options, err); + if (err.Fail()) return std::string(); - if (!context_str.empty()) res += ":" + context_str; + if (!context_str.empty()) res += ":" + context_str; + } if (options->print_source) { SharedFunctionInfo info = Info(err); @@ -821,6 +826,12 @@ std::string HeapObject::Inspect(InspectOptions* options, Error& err) { return pre + str.Inspect(options, err); } + if (type >= v8()->types()->kFirstContextType && + type <= v8()->types()->kLastContextType) { + Context ctx(this); + return pre + ctx.Inspect(options, err); + } + if (type == v8()->types()->kFixedArrayType) { FixedArray arr(this); return pre + arr.Inspect(options, err); @@ -1060,13 +1071,19 @@ HeapObject Context::GetScopeInfo(Error& err) { return info.GetScopeInfo(err); } -std::string Context::Inspect(Error& err) { +std::string Context::Inspect(InspectOptions* options, Error& err) { // Not enough postmortem information, return bare minimum if (v8()->shared_info()->kScopeInfoOffset == -1 && v8()->shared_info()->kNameOrScopeInfoOffset == -1) return std::string(); - std::string res = "detailed) { + return res + ">"; + } + + res += ": {\n"; Value previous = Previous(err); if (err.Fail()) return std::string(); @@ -1083,12 +1100,10 @@ std::string Context::Inspect(Error& err) { Smi local_count_smi = scope.ContextLocalCount(err); if (err.Fail()) return std::string(); - InspectOptions options; - HeapObject heap_previous = HeapObject(previous); if (heap_previous.Check()) { char tmp[128]; - snprintf(tmp, sizeof(tmp), " (previous)=0x%016" PRIx64, previous.raw()); + snprintf(tmp, sizeof(tmp), (options->get_indent_spaecs() + "(previous)=0x%016" PRIx64).c_str(), previous.raw()); res += std::string(tmp) + ":,"; } @@ -1098,16 +1113,16 @@ std::string Context::Inspect(Error& err) { JSFunction closure = Closure(err); if (err.Fail()) return std::string(); char tmp[128]; - snprintf(tmp, sizeof(tmp), " (closure)=0x%016" PRIx64 " {", + snprintf(tmp, sizeof(tmp), (options->get_indent_spaecs() + "(closure)=0x%016" PRIx64 " {").c_str(), closure.raw()); res += tmp; - InspectOptions options; - res += closure.Inspect(&options, err) + "}"; + InspectOptions closure_options; + res += closure.Inspect(&closure_options, err) + "}"; if (err.Fail()) return std::string(); } else { char tmp[128]; - snprintf(tmp, sizeof(tmp), " (scope_info)=0x%016" PRIx64, + snprintf(tmp, sizeof(tmp), (options->get_indent_spaecs() + "(scope_info)=0x%016" PRIx64).c_str(), scope.raw()); res += std::string(tmp) + ":"; + return res + "}>"; } diff --git a/src/llv8.h b/src/llv8.h index 96818c6e..3a68a0fc 100644 --- a/src/llv8.h +++ b/src/llv8.h @@ -44,14 +44,20 @@ class Value { : detailed(false), print_map(false), print_source(false), - length(kLength) {} + length(kLength), + indent_depth(1) {} static const unsigned int kLength = 16; + static const unsigned int kIndentSize = 2; + inline std::string get_indent_spaecs() { + return std::string(indent_depth * kIndentSize, ' '); + } bool detailed; bool print_map; bool print_source; unsigned int length; + unsigned int indent_depth; }; Value(const Value& v) = default; @@ -390,7 +396,7 @@ class Context : public FixedArray { inline T GetEmbedderData(int64_t index, Error& err); inline Value ContextSlot(int index, Error& err); - std::string Inspect(Error& err); + std::string Inspect(InspectOptions *options, Error& err); private: inline JSFunction Closure(Error& err); From 3a9ae6b36c7ffea419893bb61bb380e35f466410 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Thu, 30 Aug 2018 08:57:29 -0300 Subject: [PATCH 5/6] fixup! src: inspect context objects (Node.js v10.x+) --- src/llv8.cc | 8 ++++---- src/llv8.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/llv8.cc b/src/llv8.cc index a2e24691..20e46403 100644 --- a/src/llv8.cc +++ b/src/llv8.cc @@ -1103,7 +1103,7 @@ std::string Context::Inspect(InspectOptions* options, Error& err) { HeapObject heap_previous = HeapObject(previous); if (heap_previous.Check()) { char tmp[128]; - snprintf(tmp, sizeof(tmp), (options->get_indent_spaecs() + "(previous)=0x%016" PRIx64).c_str(), previous.raw()); + snprintf(tmp, sizeof(tmp), (options->get_indent_spaces() + "(previous)=0x%016" PRIx64).c_str(), previous.raw()); res += std::string(tmp) + ":,"; } @@ -1113,7 +1113,7 @@ std::string Context::Inspect(InspectOptions* options, Error& err) { JSFunction closure = Closure(err); if (err.Fail()) return std::string(); char tmp[128]; - snprintf(tmp, sizeof(tmp), (options->get_indent_spaecs() + "(closure)=0x%016" PRIx64 " {").c_str(), + snprintf(tmp, sizeof(tmp), (options->get_indent_spaces() + "(closure)=0x%016" PRIx64 " {").c_str(), closure.raw()); res += tmp; @@ -1122,7 +1122,7 @@ std::string Context::Inspect(InspectOptions* options, Error& err) { if (err.Fail()) return std::string(); } else { char tmp[128]; - snprintf(tmp, sizeof(tmp), (options->get_indent_spaecs() + "(scope_info)=0x%016" PRIx64).c_str(), + snprintf(tmp, sizeof(tmp), (options->get_indent_spaces() + "(scope_info)=0x%016" PRIx64).c_str(), scope.raw()); res += std::string(tmp) + ":get_indent_spaecs() + name.ToString(err) + "="; + res += options->get_indent_spaces() + name.ToString(err) + "="; if (err.Fail()) return std::string(); Value value = ContextSlot(i, err); diff --git a/src/llv8.h b/src/llv8.h index 3a68a0fc..02ced191 100644 --- a/src/llv8.h +++ b/src/llv8.h @@ -49,7 +49,7 @@ class Value { static const unsigned int kLength = 16; static const unsigned int kIndentSize = 2; - inline std::string get_indent_spaecs() { + inline std::string get_indent_spaces() { return std::string(indent_depth * kIndentSize, ' '); } From 65f76cbae41e0490f0a59d902c05841d76368724 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Thu, 30 Aug 2018 09:10:28 -0300 Subject: [PATCH 6/6] fixup! fixup! src: inspect context objects (Node.js v10.x+) --- src/llv8-constants.h | 9 +++++++++ src/llv8.cc | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/llv8-constants.h b/src/llv8-constants.h index d093a5e6..80fc6e14 100644 --- a/src/llv8-constants.h +++ b/src/llv8-constants.h @@ -206,6 +206,15 @@ class Context : public Module { int64_t kEmbedderDataIndex; int64_t kMinContextSlots; + inline bool hasClosure() { + // NOTE (mmarchini): V8 6.8 replaced the closure field (which was a + // JSFunction) with a scope_info field (which is a ScopeInfo). The change + // made it easier to get the scope info for a context, but removed our + // ability to get the outer function for a given context. We can still get + // the outer context through the previous field though. + return kClosureIndex != -1; + } + protected: void Load(); }; diff --git a/src/llv8.cc b/src/llv8.cc index 20e46403..d83bb7ad 100644 --- a/src/llv8.cc +++ b/src/llv8.cc @@ -1109,7 +1109,7 @@ std::string Context::Inspect(InspectOptions* options, Error& err) { if (!res.empty()) res += "\n"; - if (v8()->context()->kClosureIndex != -1) { + if (v8()->context()->hasClosure()) { JSFunction closure = Closure(err); if (err.Fail()) return std::string(); char tmp[128];