src: fix JSError inspection with stringified stack

When the `stack` property of an Error object is accessed in V8, a
stringified version of the stack is generated, and then the "raw stack"
(with FP and arguments) is removed from memory. Our current inspection
code couldn't handle this because it was not checking if the raw stack
was a valid object. This commit makes that code more robust and thus
fixes inspection of error objects with stringified stacks.

Author: mmarchini

src/llv8-inl.h

@@ -46,7 +46,8 @@ inline int64_t Smi::GetValue() const {
 
 
 inline bool HeapObject::Check() const {
-  return (raw() & v8()->heap_obj()->kTagMask) == v8()->heap_obj()->kTag;
+  return raw() != -1 &&
+         (raw() & v8()->heap_obj()->kTagMask) == v8()->heap_obj()->kTag;
 }
 
 

src/llv8.cc

@@ -1268,6 +1268,13 @@ StackTrace::Iterator StackTrace::end() {
 
 StackTrace::StackTrace(JSArray frame_array, Error& err)
     : frame_array_(frame_array) {
+  if (!frame_array.Check()) {
+    Error::PrintInDebugMode("JS Array is not a valid object");
+    len_ = -1;
+    multiplier_ = -1;
+    return;
+  }
+
   v8::Value maybe_stack_len = frame_array.GetArrayElement(0, err);
 
   if (err.Fail()) {

src/llv8.h

@@ -67,7 +67,7 @@ class Value {
 
  protected:
   LLV8* v8_;
-  int64_t raw_ = 0x0;
+  int64_t raw_ = -1;
 };
 
 class Smi : public Value {

src/printer.cc

@@ -526,27 +526,31 @@ std::string Printer::Stringify(v8::JSError js_error, Error& err) {
   if (options_.detailed) {
     output << StringifyJSObjectFields(js_error, err);
 
-    v8::StackTrace stack_trace = js_error.GetStackTrace(err);
+    if (js_error.HasStackTrace(err)) {
+      v8::StackTrace stack_trace = js_error.GetStackTrace(err);
 
-    std::stringstream error_stack;
-    error_stack << std::endl
-                << rang::fg::red << "  error stack" << rang::fg::reset << " {"
-                << std::endl;
+      std::stringstream error_stack;
+      error_stack << std::endl
+                  << rang::fg::red << "  error stack" << rang::fg::reset << " {"
+                  << std::endl;
 
-    Printer printer(llv8_);
-    for (v8::StackFrame frame : stack_trace) {
-      v8::JSFunction js_function = frame.GetFunction(err);
-      if (err.Fail()) {
-        error_stack << rang::fg::gray << "    <unknown>" << std::endl;
-        continue;
+
+      Printer printer(llv8_);
+      for (v8::StackFrame frame : stack_trace) {
+        v8::JSFunction js_function = frame.GetFunction(err);
+        if (err.Fail()) {
+          error_stack << rang::fg::gray << "    <unknown>" << std::endl;
+          continue;
+        }
+
+        error_stack << "    "
+                    << printer.Stringify<v8::HeapObject>(js_function, err)
+                    << std::endl;
       }
 
-      error_stack << "    "
-                  << printer.Stringify<v8::HeapObject>(js_function, err)
-                  << std::endl;
+      error_stack << "  }";
+      output << error_stack.str();
     }
-    error_stack << "  }";
-    output << error_stack.str();
   }
 
   output << rang::fg::yellow << ">" << rang::fg::reset;

test/fixtures/inspect-scenario.js

@@ -64,6 +64,9 @@ function closure() {
   c.hashmap['error'].code = 'ERR_TEST';
   c.hashmap['error'].errno = 1;
 
+  c.hashmap['stringifiedError'] = new Error('test');
+  c.hashmap['stringifiedErrorStack'] = c.hashmap['stringifiedError'].stack;
+
   c.hashmap[0] = null;
   c.hashmap[4] = undefined;
   c.hashmap[23] = /regexp/;

test/plugin/inspect-test.js

@@ -161,6 +161,28 @@ const hashMapTests = {
       });
     }
   },
+  // .stringifiedError=0x0000392d5d661119:<Object: Error>
+  'error': {
+    re: /.stringifiedError=(0x[0-9a-f]+):<Object: Error>/,
+    desc: '.stringifiedError Error property with stringified stack',
+    validator(t, sess, addresses, name, cb) {
+      const address = addresses[name];
+      sess.send(`v8 inspect ${address}`);
+
+      sess.linesUntil(/}>/, (err, lines) => {
+        if (err) return cb(err);
+        lines = lines.join('\n');
+
+        let strStackMatch = lines.match(/stack=(0x[0-9a-f]+):<String: /i);
+        t.ok(strStackMatch, 'hashmap.stringifiedError should have stringified stack');
+
+        let stackMatch  = lines.match(/error stack {/i);
+        t.notOk(stackMatch, 'Error object with stringified stack should not have an error stack');
+
+        cb(null);
+      });
+    }
+  },
   // .array=0x000003df9cbe7919:<Array: length=6>,
   'array': {
     re: /.array=(0x[0-9a-f]+):<Array: length=6>/,