Add new FrameArray type

src/isolate.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/isolate.h"
 
 #include <stdlib.h>
 
 #include <fstream>  // NOLINT(readability/streams)
 #include <sstream>
@@ skipping 297 matching lines @@
   uint8_t buffer[kMaxStackTraceSize];
   int length = Min(kMaxStackTraceSize - 1, trace->length());
   String::WriteToFlat(*trace, buffer, 0, length);
   buffer[length] = '\0';
   // TODO(dcarney): convert buffer to utf8?
   base::OS::PrintError("Stacktrace (%x-%x) %p %p: %s\n", magic, magic2, ptr1,
                        ptr2, reinterpret_cast<char*>(buffer));
   base::OS::Abort();
 }
 
-static Handle<FixedArray> MaybeGrow(Isolate* isolate,
-                                    Handle<FixedArray> elements,
-                                    int cur_position, int new_size) {
-  if (new_size > elements->length()) {
-    int new_capacity = JSObject::NewElementsCapacity(elements->length());
-    Handle<FixedArray> new_elements =
-        isolate->factory()->NewFixedArrayWithHoles(new_capacity);
-    for (int i = 0; i < cur_position; i++) {
-      new_elements->set(i, elements->get(i));
-    }
-    elements = new_elements;
-  }
-  DCHECK(new_size <= elements->length());
-  return elements;
-}
+namespace {
 
 class StackTraceHelper {
  public:
   StackTraceHelper(Isolate* isolate, FrameSkipMode mode, Handle<Object> caller)
       : isolate_(isolate),
         mode_(mode),
         caller_(caller),
         skip_next_frame_(true) {
     // The caller parameter can be used to skip a specific set of frames in the
     // stack trace. It can be:
@@ skipping 85 matching lines @@
   Isolate* isolate_;
 
   const FrameSkipMode mode_;
   const Handle<Object> caller_;
   bool skip_next_frame_;
 
   int sloppy_frames_;
   bool encountered_strict_function_;
 };
 
-namespace {
-
 // TODO(jgruber): Fix all cases in which frames give us a hole value (e.g. the
 // receiver in RegExp constructor frames.
 Handle<Object> TheHoleToUndefined(Isolate* isolate, Handle<Object> in) {
   return (in->IsTheHole(isolate))
              ? Handle<Object>::cast(isolate->factory()->undefined_value())
              : in;
 }
-}
+
+}  // namespace
 
 Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
                                                 FrameSkipMode mode,
                                                 Handle<Object> caller) {
   DisallowJavascriptExecution no_js(this);
 
   // Get stack trace limit.
   Handle<JSObject> error = error_function();
   Handle<String> stackTraceLimit =
       factory()->InternalizeUtf8String("stackTraceLimit");
   DCHECK(!stackTraceLimit.is_null());
   Handle<Object> stack_trace_limit =
       JSReceiver::GetDataProperty(error, stackTraceLimit);
   if (!stack_trace_limit->IsNumber()) return factory()->undefined_value();
   int limit = FastD2IChecked(stack_trace_limit->Number());
   limit = Max(limit, 0);  // Ensure that limit is not negative.
 
   int initial_size = Min(limit, 10);
-  Handle<FixedArray> elements =
-      factory()->NewFixedArrayWithHoles(initial_size * 4 + 1);
+  Handle<FrameArray> elements = FrameArray::Allocate(this, initial_size);
 
   StackTraceHelper helper(this, mode, caller);
 
-  // First element is reserved to store the number of sloppy frames.
-  int cursor = 1;
   int frames_seen = 0;

[Toon Verwaest, 2016/08/23 13:14:08]

Perhaps we should just encapsulate frames_seen as an additional first integer in
the fixed array, basically use fixed-array-length as "capacity" and have an
additional "length" field?

[jgruber, 2016/08/23 15:32:13]

Done.

   for (StackFrameIterator iter(this); !iter.done() && frames_seen < limit;
        iter.Advance()) {
     StackFrame* frame = iter.frame();
 
     switch (frame->type()) {
       case StackFrame::JAVA_SCRIPT:
       case StackFrame::OPTIMIZED:
       case StackFrame::INTERPRETED:
       case StackFrame::BUILTIN: {
         JavaScriptFrame* js_frame = JavaScriptFrame::cast(frame);
@@ skipping 11 matching lines @@
           Handle<Object> recv = frames[i].receiver();
           Handle<AbstractCode> abstract_code = frames[i].abstract_code();
           if (frame->type() == StackFrame::BUILTIN) {
             // Help CallSite::IsConstructor correctly detect hand-written
             // construct stubs.
             Code* code = Code::cast(*abstract_code);
             if (code->is_construct_stub()) {
               recv = handle(heap()->call_site_constructor_symbol(), this);
             }
           }
-          Handle<Smi> offset(Smi::FromInt(frames[i].code_offset()), this);
+          const int offset = frames[i].code_offset();
 
-          elements = MaybeGrow(this, elements, cursor, cursor + 4);
-          elements->set(cursor++, *TheHoleToUndefined(this, recv));
-          elements->set(cursor++, *fun);
-          elements->set(cursor++, *abstract_code);
-          elements->set(cursor++, *offset);
+          elements = FrameArray::AppendJSFrame(elements, frames_seen,
+                                               TheHoleToUndefined(this, recv),
+                                               fun, abstract_code, offset);
           frames_seen++;
         }
       } break;
 
       case StackFrame::BUILTIN_EXIT: {
         BuiltinExitFrame* exit_frame = BuiltinExitFrame::cast(frame);
         Handle<JSFunction> fun = handle(exit_frame->function(), this);
 
         // Filter out internal frames that we do not want to show.
         if (!helper.IsVisibleInStackTrace(*fun)) continue;
         helper.CountSloppyFrames(*fun);
 
         Handle<Code> code = handle(exit_frame->LookupCode(), this);
         int offset =
             static_cast<int>(exit_frame->pc() - code->instruction_start());
 
         // In order to help CallSite::IsConstructor detect builtin constructors,
         // we reuse the receiver field to pass along a special symbol.
         Handle<Object> recv;
         if (exit_frame->IsConstructor()) {
           recv = factory()->call_site_constructor_symbol();
         } else {
           recv = handle(exit_frame->receiver(), this);
         }
 
-        elements = MaybeGrow(this, elements, cursor, cursor + 4);
-        elements->set(cursor++, *recv);
-        elements->set(cursor++, *fun);
-        elements->set(cursor++, *code);
-        elements->set(cursor++, Smi::FromInt(offset));
+        elements =
+            FrameArray::AppendJSFrame(elements, frames_seen, recv, fun,
+                                      Handle<AbstractCode>::cast(code), offset);
         frames_seen++;
       } break;
 
       case StackFrame::WASM: {
         WasmFrame* wasm_frame = WasmFrame::cast(frame);
+        Handle<Object> wasm_object = handle(wasm_frame->wasm_obj(), this);
+        const int wasm_function_index = wasm_frame->function_index();
         Code* code = wasm_frame->unchecked_code();
         Handle<AbstractCode> abstract_code =
             Handle<AbstractCode>(AbstractCode::cast(code), this);
-        int offset =
+        const int offset =
             static_cast<int>(wasm_frame->pc() - code->instruction_start());
-        elements = MaybeGrow(this, elements, cursor, cursor + 4);
-        elements->set(cursor++, wasm_frame->wasm_obj());
-        elements->set(cursor++, Smi::FromInt(wasm_frame->function_index()));
-        elements->set(cursor++, *abstract_code);
-        elements->set(cursor++, Smi::FromInt(offset));
+
+        // TODO(wasm): The wasm object returned by the WasmFrame should always
+        //             be a wasm object.
+        DCHECK(wasm::IsWasmObject(*wasm_object) ||
+               wasm_object->IsUndefined(this));
+
+        elements = FrameArray::AppendWasmFrame(elements, frames_seen,
+                                               wasm_object, wasm_function_index,
+                                               abstract_code, offset);
         frames_seen++;
       } break;
 
       default:
         break;
     }
   }
   elements->set(0, Smi::FromInt(helper.sloppy_frames()));
-  elements->Shrink(cursor);
+
+  const int elements_length = FrameArray::LengthFor(frames_seen);

[Toon Verwaest, 2016/08/23 13:14:08]

What about elements->Shrink(frames_seen) to avoid leaking here that it's a fixed
array? We don't need to pass in frames-seen even if we store it in the fixed
array. Slightly less efficient but it probably doesn't matter.

[jgruber, 2016/08/23 15:32:13]

Done.

+  elements->Shrink(elements_length);
+
   Handle<JSArray> result = factory()->NewJSArrayWithElements(elements);
-  result->set_length(Smi::FromInt(cursor));
+  result->set_length(Smi::FromInt(elements_length));
+
   // TODO(yangguo): Queue this structured stack trace for preprocessing on GC.
   return result;
 }
 
 MaybeHandle<JSReceiver> Isolate::CaptureAndSetDetailedStackTrace(
     Handle<JSReceiver> error_object) {
   if (capture_stack_trace_for_uncaught_exceptions_) {
     // Capture stack trace for a detailed exception message.
     Handle<Name> key = factory()->detailed_stack_trace_symbol();
     Handle<JSArray> stack_trace = CaptureCurrentStackTrace(
@@ skipping 188 matching lines @@
   Handle<String> column_key_;
   Handle<String> line_key_;
   Handle<String> script_id_key_;
   Handle<String> script_name_key_;
   Handle<String> script_name_or_source_url_key_;
   Handle<String> function_key_;
   Handle<String> eval_key_;
   Handle<String> constructor_key_;
 };
 
-
-int PositionFromStackTrace(Handle<FixedArray> elements, int index) {
-  DisallowHeapAllocation no_gc;
-  Object* maybe_code = elements->get(index + 2);
-  if (maybe_code->IsSmi()) {
-    return Smi::cast(maybe_code)->value();
-  } else {
-    AbstractCode* abstract_code = AbstractCode::cast(maybe_code);
-    int code_offset = Smi::cast(elements->get(index + 3))->value();
-    return abstract_code->SourcePosition(code_offset);
-  }
-}
-
 Handle<JSArray> Isolate::CaptureCurrentStackTrace(
     int frame_limit, StackTrace::StackTraceOptions options) {
   DisallowJavascriptExecution no_js(this);
   CaptureStackTraceHelper helper(this, options);
 
   // Ensure no negative values.
   int limit = Max(frame_limit, 0);
   Handle<JSArray> stack_trace = factory()->NewJSArray(frame_limit);
   Handle<FixedArray> stack_trace_elems(
       FixedArray::cast(stack_trace->elements()), this);
@@ skipping 725 matching lines @@
 
 bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
                                             Handle<Object> exception) {
   if (!exception->IsJSObject()) return false;
   Handle<Name> key = factory()->stack_trace_symbol();
   Handle<Object> property =
       JSReceiver::GetDataProperty(Handle<JSObject>::cast(exception), key);
   if (!property->IsJSArray()) return false;
   Handle<JSArray> simple_stack_trace = Handle<JSArray>::cast(property);
 
-  Handle<FixedArray> elements(FixedArray::cast(simple_stack_trace->elements()));
-  int elements_limit = Smi::cast(simple_stack_trace->length())->value();
+  Handle<FrameArray> elements(FrameArray::cast(simple_stack_trace->elements()));
 
-  for (int i = 1; i < elements_limit; i += 4) {
-    Handle<Object> fun_obj = handle(elements->get(i + 1), this);
-    if (fun_obj->IsSmi()) {
+  const int frame_count = elements->FrameCount();
+  for (int i = 0; i < frame_count; i++) {
+    if (elements->IsWasmFrame(i)) {
       // TODO(clemensh): handle wasm frames
       return false;
     }
-    Handle<JSFunction> fun = Handle<JSFunction>::cast(fun_obj);
+
+    Handle<JSFunction> fun = handle(elements->Function(i), this);
     if (!fun->shared()->IsSubjectToDebugging()) continue;
 
     Object* script = fun->shared()->script();
     if (script->IsScript() &&
         !(Script::cast(script)->source()->IsUndefined(this))) {
-      int pos = PositionFromStackTrace(elements, i);
+      AbstractCode* abstract_code = elements->Code(i);
+      const int code_offset = elements->Offset(i)->value();
+      const int pos = abstract_code->SourcePosition(code_offset);
+
       Handle<Script> casted_script(Script::cast(script));
       *target = MessageLocation(casted_script, pos, pos + 1);
       return true;
     }
   }
   return false;
 }
 
 
 Handle<JSMessageObject> Isolate::CreateMessage(Handle<Object> exception,
@@ skipping 1651 matching lines @@
   // Then check whether this scope intercepts.
   if ((flag & intercept_mask_)) {
     intercepted_flags_ |= flag;
     return true;
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace v8