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: nodejs#193 PR-URL: nodejs#201 Reviewed-By: Joyee Cheung <[email protected]>
Drieger · Sep 4, 2018 · e9c7ce4 · e9c7ce4
1 parent a9b6077
commit e9c7ce4
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 20 deletions.
diff --git 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
@@ -199,12 +199,22 @@ class Context : public Module {
   CONSTANTS_DEFAULT_METHODS(Context);
 
   int64_t kClosureIndex;
+  int64_t kScopeInfoIndex;
   int64_t kGlobalObjectIndex;
   int64_t kPreviousIndex;
   int64_t kNativeIndex;
   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
@@ -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);
@@ -1047,24 +1047,31 @@ std::string FixedArray::InspectContents(int length, Error& err) {
   return res;
 }
 
+HeapObject Context::GetScopeInfo(Error& err) {
+  if (v8()->context()->kScopeInfoIndex != -1) {
+    return FixedArray::Get<HeapObject>(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;
   // Not enough postmortem information, return bare minimum
   if (v8()->shared_info()->kScopeInfoOffset == -1 &&
       v8()->shared_info()->kNameOrScopeInfoOffset == -1)
-    return res;
+    return std::string();
 
-  Value previous = Previous(err);
-  if (err.Fail()) return std::string();
+  std::string res = "<Context: {\n";
 
-  JSFunction closure = Closure(err);
-  if (err.Fail()) return std::string();
-
-  SharedFunctionInfo info = closure.Info(err);
+  Value previous = Previous(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);
@@ -1082,11 +1089,14 @@ std::string Context::Inspect(Error& err) {
   if (heap_previous.Check()) {
     char tmp[128];
     snprintf(tmp, sizeof(tmp), "    (previous)=0x%016" PRIx64, previous.raw());
-    res += tmp;
+    res += std::string(tmp) + ":<Context>,";
   }
 
   if (!res.empty()) res += "\n";
-  {
+
+  if (v8()->context()->hasClosure()) {
+    JSFunction closure = Closure(err);
+    if (err.Fail()) return std::string();
     char tmp[128];
     snprintf(tmp, sizeof(tmp), "    (closure)=0x%016" PRIx64 " {",
              closure.raw());
@@ -1095,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) + ":<ScopeInfo";
+
+    Error function_name_error;
+    HeapObject maybe_function_name = scope.MaybeFunctionName(function_name_error);
+
+    if (function_name_error.Success()) {
+      res += ": for function " + String(maybe_function_name).ToString(err);
+    }
+
+    res += ">";
   }
 
   int param_count = param_count_smi.GetValue();
@@ -1116,7 +1141,7 @@ std::string Context::Inspect(Error& err) {
     if (err.Fail()) return std::string();
   }
 
-  return res;
+  return res + " }>";
 }
 
 

diff --git 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 {

diff --git 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);