Refactor call site handling for stack formatting

This commit introduces several new types:

* JSStackFrame and WasmStackFrame are wrapper classes around a single frame
  in a FrameArray.
* They both inherit from StackFrameBase, which uses virtual dispatch to call
  the correct implementation.
* FrameArrayIterator contains a static instance of JSStackFrame and
  WasmStackFrame and returns a pointer to the corresponding type for each
  frame.
* The JS callsite object now contains the frame array and frame index
  as internal fields.

Internal stack formatting now relies completely on FrameArrayIterator and the
{JS,Wasm}StackFrame types. JS callsite instances are allocated only for custom
user formatting through Error.prepareStackTrace.

BUG=

Review-Url: https://codereview.chromium.org/2275233002
Cr-Commit-Position: refs/heads/master@{#39015}

src/builtins/builtins-callsite.cc

@@ -14,7 +14,7 @@ namespace internal {
 #define CHECK_CALLSITE(recv, method)                                          \
   CHECK_RECEIVER(JSObject, recv, method);                                     \
   if (!JSReceiver::HasOwnProperty(                                            \
-           recv, isolate->factory()->call_site_position_symbol())             \
+           recv, isolate->factory()->call_site_frame_array_symbol())          \
            .FromMaybe(false)) {                                               \
     THROW_NEW_ERROR_RETURN_FAILURE(                                           \
         isolate,                                                              \
@@ -29,172 +29,152 @@ Object* PositiveNumberOrNull(int value, Isolate* isolate) {
   return isolate->heap()->null_value();
 }
 
+Handle<FrameArray> GetFrameArray(Isolate* isolate, Handle<JSObject> object) {
+  Handle<Object> frame_array_obj = JSObject::GetDataProperty(
+      object, isolate->factory()->call_site_frame_array_symbol());
+  return Handle<FrameArray>::cast(frame_array_obj);
+}
+
+int GetFrameIndex(Isolate* isolate, Handle<JSObject> object) {
+  Handle<Object> frame_index_obj = JSObject::GetDataProperty(
+      object, isolate->factory()->call_site_frame_index_symbol());
+  return Smi::cast(*frame_index_obj)->value();
+}
+
 }  // namespace
 
 BUILTIN(CallSitePrototypeGetColumnNumber) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getColumnNumber");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return PositiveNumberOrNull(call_site.GetColumnNumber(), isolate);
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return PositiveNumberOrNull(it.Frame()->GetColumnNumber(), isolate);
 }
 
 BUILTIN(CallSitePrototypeGetEvalOrigin) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getEvalOrigin");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return *call_site.GetEvalOrigin();
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return *it.Frame()->GetEvalOrigin();
 }
 
 BUILTIN(CallSitePrototypeGetFileName) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getFileName");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return *call_site.GetFileName();
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return *it.Frame()->GetFileName();
 }
 
-namespace {
-
-bool CallSiteIsStrict(Isolate* isolate, Handle<JSObject> receiver) {
-  Handle<Object> strict;
-  Handle<Symbol> symbol = isolate->factory()->call_site_strict_symbol();
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, strict,
-                                     JSObject::GetProperty(receiver, symbol));
-  return strict->BooleanValue();
-}
-
-}  // namespace
-
 BUILTIN(CallSitePrototypeGetFunction) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getFunction");
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
 
-  if (CallSiteIsStrict(isolate, recv))
-    return *isolate->factory()->undefined_value();
-
-  Handle<Symbol> symbol = isolate->factory()->call_site_function_symbol();
-  RETURN_RESULT_OR_FAILURE(isolate, JSObject::GetProperty(recv, symbol));
+  StackFrameBase* frame = it.Frame();
+  if (frame->IsStrict()) return isolate->heap()->undefined_value();
+  return *frame->GetFunction();
 }
 
 BUILTIN(CallSitePrototypeGetFunctionName) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getFunctionName");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return *call_site.GetFunctionName();
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return *it.Frame()->GetFunctionName();
 }
 
 BUILTIN(CallSitePrototypeGetLineNumber) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getLineNumber");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-
-  int line_number = call_site.IsWasm() ? call_site.wasm_func_index()
-                                       : call_site.GetLineNumber();
-  return PositiveNumberOrNull(line_number, isolate);
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return PositiveNumberOrNull(it.Frame()->GetLineNumber(), isolate);
 }
 
 BUILTIN(CallSitePrototypeGetMethodName) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getMethodName");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return *call_site.GetMethodName();
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return *it.Frame()->GetMethodName();
 }
 
 BUILTIN(CallSitePrototypeGetPosition) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getPosition");
-
-  Handle<Symbol> symbol = isolate->factory()->call_site_position_symbol();
-  RETURN_RESULT_OR_FAILURE(isolate, JSObject::GetProperty(recv, symbol));
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return Smi::FromInt(it.Frame()->GetPosition());
 }
 
 BUILTIN(CallSitePrototypeGetScriptNameOrSourceURL) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getScriptNameOrSourceUrl");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return *call_site.GetScriptNameOrSourceUrl();
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return *it.Frame()->GetScriptNameOrSourceUrl();
 }
 
 BUILTIN(CallSitePrototypeGetThis) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getThis");
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
 
-  if (CallSiteIsStrict(isolate, recv))
-    return *isolate->factory()->undefined_value();
-
-  Handle<Object> receiver;
-  Handle<Symbol> symbol = isolate->factory()->call_site_receiver_symbol();
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
-                                     JSObject::GetProperty(recv, symbol));
-
-  if (*receiver == isolate->heap()->call_site_constructor_symbol())
-    return *isolate->factory()->undefined_value();
-
-  return *receiver;
+  StackFrameBase* frame = it.Frame();
+  if (frame->IsStrict()) return isolate->heap()->undefined_value();
+  return *frame->GetReceiver();
 }
 
 BUILTIN(CallSitePrototypeGetTypeName) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "getTypeName");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return *call_site.GetTypeName();
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return *it.Frame()->GetTypeName();
 }
 
 BUILTIN(CallSitePrototypeIsConstructor) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "isConstructor");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return isolate->heap()->ToBoolean(call_site.IsConstructor());
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return isolate->heap()->ToBoolean(it.Frame()->IsConstructor());
 }
 
 BUILTIN(CallSitePrototypeIsEval) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "isEval");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return isolate->heap()->ToBoolean(call_site.IsEval());
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return isolate->heap()->ToBoolean(it.Frame()->IsEval());
 }
 
 BUILTIN(CallSitePrototypeIsNative) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "isNative");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return isolate->heap()->ToBoolean(call_site.IsNative());
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return isolate->heap()->ToBoolean(it.Frame()->IsNative());
 }
 
 BUILTIN(CallSitePrototypeIsToplevel) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "isToplevel");
-
-  CallSite call_site(isolate, recv);
-  CHECK(call_site.IsJavaScript() || call_site.IsWasm());
-  return isolate->heap()->ToBoolean(call_site.IsToplevel());
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return isolate->heap()->ToBoolean(it.Frame()->IsToplevel());
 }
 
 BUILTIN(CallSitePrototypeToString) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "toString");
-  RETURN_RESULT_OR_FAILURE(isolate, CallSiteUtils::ToString(isolate, recv));
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  RETURN_RESULT_OR_FAILURE(isolate, it.Frame()->ToString());
 }
 
 #undef CHECK_CALLSITE

src/execution.cc

@@ -436,31 +436,6 @@ void StackGuard::InitThread(const ExecutionAccess& lock) {
 // --- C a l l s   t o   n a t i v e s ---
 
 
-Handle<String> Execution::GetStackTraceLine(Handle<Object> recv,
-                                            Handle<JSFunction> fun,
-                                            Handle<Object> pos,
-                                            Handle<Object> is_global) {
-  Isolate* isolate = fun->GetIsolate();
-  Handle<Object> strict_mode = isolate->factory()->ToBoolean(false);
-
-  MaybeHandle<Object> maybe_callsite =
-      CallSiteUtils::Construct(isolate, recv, fun, pos, strict_mode);
-  if (maybe_callsite.is_null()) {
-    isolate->clear_pending_exception();
-    return isolate->factory()->empty_string();
-  }
-
-  MaybeHandle<String> maybe_to_string =
-      CallSiteUtils::ToString(isolate, maybe_callsite.ToHandleChecked());
-  if (maybe_to_string.is_null()) {
-    isolate->clear_pending_exception();
-    return isolate->factory()->empty_string();
-  }
-
-  return maybe_to_string.ToHandleChecked();
-}
-
-
 void StackGuard::HandleGCInterrupt() {
   if (CheckAndClearInterrupt(GC_REQUEST)) {
     isolate_->heap()->HandleGCRequest();

src/execution.h

@@ -48,11 +48,6 @@ class Execution final : public AllStatic {
                                      Handle<Object> receiver, int argc,
                                      Handle<Object> argv[],
                                      MaybeHandle<Object>* exception_out = NULL);
-
-  static Handle<String> GetStackTraceLine(Handle<Object> recv,
-                                          Handle<JSFunction> fun,
-                                          Handle<Object> pos,
-                                          Handle<Object> is_global);
 };
 
 

src/heap-symbols.h

@@ -162,13 +162,8 @@
   V(array_iteration_kind_symbol)            \
   V(array_iterator_next_symbol)             \
   V(array_iterator_object_symbol)           \
-  V(call_site_constructor_symbol)           \
-  V(call_site_function_symbol)              \
-  V(call_site_position_symbol)              \
-  V(call_site_receiver_symbol)              \
-  V(call_site_strict_symbol)                \
-  V(call_site_wasm_obj_symbol)              \
-  V(call_site_wasm_func_index_symbol)       \
+  V(call_site_frame_array_symbol)           \
+  V(call_site_frame_index_symbol)           \
   V(class_end_position_symbol)              \
   V(class_start_position_symbol)            \
   V(detailed_stack_trace_symbol)            \

src/isolate.cc

@@ -501,7 +501,7 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
 
         Handle<Object> recv(exit_frame->receiver(), this);
         Handle<Code> code(exit_frame->LookupCode(), this);
-        int offset =
+        const int offset =
             static_cast<int>(exit_frame->pc() - code->instruction_start());
 
         int flags = 0;
@@ -1395,36 +1395,20 @@ Object* Isolate::PromoteScheduledException() {
 
 
 void Isolate::PrintCurrentStackTrace(FILE* out) {
-  StackTraceFrameIterator it(this);
-  while (!it.done()) {
+  for (StackTraceFrameIterator it(this); !it.done(); it.Advance()) {
+    if (!it.is_javascript()) continue;
+
     HandleScope scope(this);
-    // Find code position if recorded in relocation info.
-    StandardFrame* frame = it.frame();
-    AbstractCode* abstract_code;
-    int code_offset;
-    if (frame->is_interpreted()) {
-      InterpretedFrame* iframe = reinterpret_cast<InterpretedFrame*>(frame);
-      abstract_code = AbstractCode::cast(iframe->GetBytecodeArray());
-      code_offset = iframe->GetBytecodeOffset();
-    } else {
-      DCHECK(frame->is_java_script() || frame->is_wasm());
-      Code* code = frame->LookupCode();
-      abstract_code = AbstractCode::cast(code);
-      code_offset = static_cast<int>(frame->pc() - code->instruction_start());
-    }
-    int pos = abstract_code->SourcePosition(code_offset);
-    JavaScriptFrame* js_frame = JavaScriptFrame::cast(frame);
-    Handle<Object> pos_obj(Smi::FromInt(pos), this);
-    // Fetch function and receiver.
-    Handle<JSFunction> fun(js_frame->function(), this);
-    Handle<Object> recv(js_frame->receiver(), this);
-    // Advance to the next JavaScript frame and determine if the
-    // current frame is the top-level frame.
-    it.Advance();
-    Handle<Object> is_top_level = factory()->ToBoolean(it.done());
-    // Generate and print stack trace line.
-    Handle<String> line =
-        Execution::GetStackTraceLine(recv, fun, pos_obj, is_top_level);
+    JavaScriptFrame* frame = it.javascript_frame();
+
+    Handle<Object> receiver(frame->receiver(), this);
+    Handle<JSFunction> function(frame->function(), this);
+    Handle<AbstractCode> code(AbstractCode::cast(frame->LookupCode()), this);
+    const int offset =
+        static_cast<int>(frame->pc() - code->instruction_start());
+
+    JSStackFrame site(this, receiver, function, code, offset);
+    Handle<String> line = site.ToString().ToHandleChecked();
     if (line->length() > 0) {
       line->PrintOn(out);
       PrintF(out, "\n");

src/messages.h

@@ -19,6 +19,8 @@ namespace v8 {
 namespace internal {
 
 // Forward declarations.
+class AbstractCode;
+class FrameArray;
 class JSMessageObject;
 class LookupIterator;
 class SourceInfo;
@@ -42,38 +44,142 @@ class MessageLocation {
   Handle<JSFunction> function_;
 };
 
-
-class CallSite {
+class StackFrameBase {
  public:
-  CallSite(Isolate* isolate, Handle<JSObject> call_site_obj);
-
-  Handle<Object> GetFileName();
-  Handle<Object> GetFunctionName();
-  Handle<Object> GetScriptNameOrSourceUrl();
-  Handle<Object> GetMethodName();
-  Handle<Object> GetTypeName();
-  Handle<Object> GetEvalOrigin();
+  virtual ~StackFrameBase() {}
+
+  virtual Handle<Object> GetReceiver() const = 0;
+  virtual Handle<Object> GetFunction() const = 0;
+
+  virtual Handle<Object> GetFileName() = 0;
+  virtual Handle<Object> GetFunctionName() = 0;
+  virtual Handle<Object> GetScriptNameOrSourceUrl() = 0;
+  virtual Handle<Object> GetMethodName() = 0;
+  virtual Handle<Object> GetTypeName() = 0;
+  virtual Handle<Object> GetEvalOrigin() = 0;
+
+  virtual int GetPosition() const = 0;
   // Return 1-based line number, including line offset.
-  int GetLineNumber();
+  virtual int GetLineNumber() = 0;
   // Return 1-based column number, including column offset if first line.
-  int GetColumnNumber();
-  bool IsNative();
-  bool IsToplevel();
-  bool IsEval();
-  bool IsConstructor();
+  virtual int GetColumnNumber() = 0;
+
+  virtual bool IsNative() = 0;
+  virtual bool IsToplevel() = 0;
+  virtual bool IsEval() = 0;
+  virtual bool IsConstructor() = 0;
+  virtual bool IsStrict() const = 0;
+
+  virtual MaybeHandle<String> ToString() = 0;
+};
+
+class JSStackFrame : public StackFrameBase {
+ public:
+  JSStackFrame(Isolate* isolate, Handle<Object> receiver,
+               Handle<JSFunction> function, Handle<AbstractCode> code,
+               int offset);
+  virtual ~JSStackFrame() {}
+
+  Handle<Object> GetReceiver() const override { return receiver_; }
+  Handle<Object> GetFunction() const override;
+
+  Handle<Object> GetFileName() override;
+  Handle<Object> GetFunctionName() override;
+  Handle<Object> GetScriptNameOrSourceUrl() override;
+  Handle<Object> GetMethodName() override;
+  Handle<Object> GetTypeName() override;
+  Handle<Object> GetEvalOrigin() override;
+
+  int GetPosition() const override;
+  int GetLineNumber() override;
+  int GetColumnNumber() override;
 
-  bool IsJavaScript() { return !fun_.is_null(); }
-  bool IsWasm() { return !wasm_obj_.is_null(); }
+  bool IsNative() override;
+  bool IsToplevel() override;
+  bool IsEval() override;
+  bool IsConstructor() override;
+  bool IsStrict() const override { return is_strict_; }
 
-  int wasm_func_index() const { return wasm_func_index_; }
+  MaybeHandle<String> ToString() override;
 
  private:
+  JSStackFrame();
+  void FromFrameArray(Isolate* isolate, Handle<FrameArray> array, int frame_ix);
+
+  bool HasScript() const;
+  Handle<Script> GetScript() const;
+
   Isolate* isolate_;
+
   Handle<Object> receiver_;
-  Handle<JSFunction> fun_;
-  int32_t pos_ = -1;
-  Handle<JSObject> wasm_obj_;
-  uint32_t wasm_func_index_ = static_cast<uint32_t>(-1);
+  Handle<JSFunction> function_;
+  Handle<AbstractCode> code_;
+  int offset_;
+
+  bool force_constructor_;
+  bool is_strict_;
+
+  friend class FrameArrayIterator;
+};
+
+class WasmStackFrame : public StackFrameBase {
+ public:
+  virtual ~WasmStackFrame() {}
+
+  Handle<Object> GetReceiver() const override { return wasm_obj_; }
+  Handle<Object> GetFunction() const override;
+
+  Handle<Object> GetFileName() override { return Null(); }
+  Handle<Object> GetFunctionName() override;
+  Handle<Object> GetScriptNameOrSourceUrl() override { return Null(); }
+  Handle<Object> GetMethodName() override { return Null(); }
+  Handle<Object> GetTypeName() override { return Null(); }
+  Handle<Object> GetEvalOrigin() override { return Null(); }
+
+  int GetPosition() const override;
+  int GetLineNumber() override { return wasm_func_index_; }
+  int GetColumnNumber() override { return -1; }
+
+  bool IsNative() override { return false; }
+  bool IsToplevel() override { return false; }
+  bool IsEval() override { return false; }
+  bool IsConstructor() override { return false; }
+  bool IsStrict() const override { return false; }
+
+  MaybeHandle<String> ToString() override;
+
+ private:
+  void FromFrameArray(Isolate* isolate, Handle<FrameArray> array, int frame_ix);
+  Handle<Object> Null() const;
+
+  Isolate* isolate_;
+
+  Handle<Object> wasm_obj_;
+  uint32_t wasm_func_index_;
+  Handle<AbstractCode> code_;
+  int offset_;
+
+  friend class FrameArrayIterator;
+};
+
+class FrameArrayIterator {
+ public:
+  FrameArrayIterator(Isolate* isolate, Handle<FrameArray> array,
+                     int frame_ix = 0);
+
+  StackFrameBase* Frame();
+
+  bool HasNext() const;
+  void Next();
+
+ private:
+  Isolate* isolate_;
+
+  Handle<FrameArray> array_;
+  int next_frame_ix_;
+
+  WasmStackFrame wasm_frame_;
+  JSStackFrame js_frame_;
 };
 
 // Determines how stack trace collection skips frames.
@@ -107,16 +213,6 @@ class ErrorUtils : public AllStatic {
                                               Handle<Object> stack_trace);
 };
 
-class CallSiteUtils : public AllStatic {
- public:
-  static MaybeHandle<Object> Construct(Isolate* isolate,
-                                       Handle<Object> receiver,
-                                       Handle<Object> fun, Handle<Object> pos,
-                                       Handle<Object> strict_mode);
-
-  static MaybeHandle<String> ToString(Isolate* isolate, Handle<Object> recv);
-};
-
 #define MESSAGE_TEMPLATES(T)                                                   \
   /* Error */                                                                  \
   T(None, "")                                                                  \