src: teach llnode how to print Symbol properties names

Closes: #156 PR-URL: #214 Fixes: #156 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
nodejs · Dec 21, 2018 · e30beba · e30beba
1 parent 388605f
commit e30beba
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 2 deletions.
diff --git a/src/llv8-constants.cc b/src/llv8-constants.cc
@@ -482,6 +482,11 @@ void Frame::Load() {
 }
 
 
+void Symbol::Load() {
+  kNameOffset = LoadConstant("class_Symbol__name__Object");
+}
+
+
 void Types::Load() {
   kFirstNonstringType = LoadConstant("FirstNonstringType");
   kFirstJSObjectType =
@@ -513,6 +518,7 @@ void Types::Load() {
       LoadConstant("type_SharedFunctionInfo__SHARED_FUNCTION_INFO_TYPE");
   kScriptType = LoadConstant("type_Script__SCRIPT_TYPE");
   kScopeInfoType = LoadConstant("type_ScopeInfo__SCOPE_INFO_TYPE");
+  kSymbolType = LoadConstant("type_Symbol__SYMBOL_TYPE");
 
   if (kJSAPIObjectType == -1) {
     common_->Load();

diff --git a/src/llv8-constants.h b/src/llv8-constants.h
@@ -468,6 +468,18 @@ class Frame : public Module {
   void Load();
 };
 
+
+class Symbol : public Module {
+ public:
+  CONSTANTS_DEFAULT_METHODS(Symbol);
+
+  int64_t kNameOffset;
+
+ protected:
+  void Load();
+};
+
+
 class Types : public Module {
  public:
   CONSTANTS_DEFAULT_METHODS(Types);
@@ -498,6 +510,7 @@ class Types : public Module {
   int64_t kSharedFunctionInfoType;
   int64_t kScriptType;
   int64_t kScopeInfoType;
+  int64_t kSymbolType;
 
  protected:
   void Load();

diff --git a/src/llv8-inl.h b/src/llv8-inl.h
@@ -171,6 +171,8 @@ ACCESSOR(Map, MaybeConstructor, map()->kMaybeConstructorOffset, HeapObject)
 ACCESSOR(Map, InstanceDescriptors, map()->kInstanceDescriptorsOffset,
          HeapObject)
 
+ACCESSOR(Symbol, Name, symbol()->kNameOffset, HeapObject)
+
 inline int64_t Map::BitField3(Error& err) {
   return v8()->LoadUnsigned(LeaField(v8()->map()->kBitField3Offset), 4, err);
 }

diff --git a/src/llv8.cc b/src/llv8.cc
@@ -60,6 +60,7 @@ void LLV8::Load(SBTarget target) {
   descriptor_array.Assign(target, &common);
   name_dictionary.Assign(target, &common);
   frame.Assign(target, &common);
+  symbol.Assign(target, &common);
   types.Assign(target, &common);
 }
 
@@ -566,6 +567,11 @@ std::string HeapObject::ToString(Error& err) {
     return str.ToString(err);
   }
 
+  if (type == v8()->types()->kSymbolType) {
+    Symbol symbol(this);
+    return symbol.ToString(err);
+  }
+
   return "<non-string>";
 }
 
@@ -674,6 +680,15 @@ std::string HeapNumber::ToString(bool whole, Error& err) {
 }
 
 
+std::string Symbol::ToString(Error& err) {
+  if (!String::IsString(v8(), Name(err), err)) {
+    return "Symbol()";
+  }
+  HeapObject name = Name(err);
+  return "Symbol('" + String(name).ToString(err) + "')";
+}
+
+
 std::string String::ToString(Error& err) {
   int64_t repr = Representation(err);
   if (err.Fail()) return std::string();

diff --git a/src/llv8.h b/src/llv8.h
@@ -112,6 +112,15 @@ class Map : public HeapObject {
   HeapObject Constructor(Error& err);
 };
 
+class Symbol : public HeapObject {
+ public:
+  V8_VALUE_DEFAULT_METHODS(Symbol, HeapObject)
+
+  HeapObject Name(Error& err);
+
+  std::string ToString(Error& err);
+};
+
 class String : public HeapObject {
  public:
   V8_VALUE_DEFAULT_METHODS(String, HeapObject)
@@ -513,6 +522,7 @@ class LLV8 {
   constants::DescriptorArray descriptor_array;
   constants::NameDictionary name_dictionary;
   constants::Frame frame;
+  constants::Symbol symbol;
   constants::Types types;
 
   friend class Value;
@@ -547,6 +557,7 @@ class LLV8 {
   friend class JSRegExp;
   friend class JSDate;
   friend class CodeMap;
+  friend class Symbol;
   friend class llnode::Printer;
   friend class llnode::FindJSObjectsVisitor;
   friend class llnode::FindObjectsCmd;

diff --git a/src/printer.cc b/src/printer.cc
@@ -567,9 +567,10 @@ std::string Printer::Stringify(v8::JSError js_error, Error& err) {
     // In the future we can add postmortem metadata on V8 regarding existing
     // symbols, but for now we'll use an heuristic to find the stack in the
     // error object.
-    v8::Value maybe_stack = js_error.GetProperty("<non-string>", err);
+    std::string stack_property = llv8_->types()->kSymbolType != -1 ? "Symbol()" : "<non-string>";
+    v8::Value maybe_stack = js_error.GetProperty(stack_property, err);
 
-    if (err.Fail()) {
+    if (err.Fail() || maybe_stack.raw() == -1) {
       Error::PrintInDebugMode(
           "Couldn't find a symbol property in the Error object.");
       output << rang::fg::yellow << ">" << rang::fg::reset;

diff --git a/test/fixtures/inspect-scenario.js b/test/fixtures/inspect-scenario.js
@@ -6,6 +6,8 @@ const zlib = require('zlib');
 
 let outerVar = 'outer variable';
 
+const oneSymbol = Symbol("oneSymbol");
+
 exports.holder = {};
 
 function makeThin(a, b) {
@@ -66,6 +68,7 @@ function closure() {
   c.hashmap[4] = undefined;
   c.hashmap[23] = /regexp/;
   c.hashmap[25] = (a,b)=>{a+b};
+  c.hashmap[oneSymbol] = 42;
 
   let scopedVar = 'scoped value';
   let scopedAPI = zlib.createDeflate()._handle;

diff --git a/test/plugin/inspect-test.js b/test/plugin/inspect-test.js
@@ -328,6 +328,23 @@ const hashMapTests = {
         cb(null);
       });
     }]
+  },
+  // .@@oneSymbol=<Smi: 42>
+  'symbol': {
+    re: /\.(<non-string>|Symbol\('oneSymbol'\))=<Smi: 42>/,
+    desc: ".Symbol('oneSymbol') Symbol property",
+    validator(t, sess, addresses, name, cb) {
+      sess.hasSymbol('v8dbg_type_Symbol__SYMBOL_TYPE', (err, hasSymbol) => {
+        if (err) return cb(err);
+
+        if(!hasSymbol)
+          t.skip("no metadata for Symbol type");
+        else
+          t.ok(/Symbol\('oneSymbol'\)/.test(addresses[name]), 'Symbol should exist');
+
+        cb(null);
+      });
+    }
   }
 };