Eagerly format traces in captureStackTrace

This allows us to skip complicated logic for setting the accessors.

BUG=

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

src/accessors.cc

@@ -1132,24 +1132,6 @@ MaybeHandle<JSReceiver> ClearInternalStackTrace(Isolate* isolate,
   return error;
 }
 
-MaybeHandle<Object> FormatStackTrace(Isolate* isolate, Handle<JSObject> error,
-                                     Handle<Object> stack_trace) {
-  // TODO(jgruber): Port FormatStackTrace from JS.
-  Handle<JSFunction> fun = isolate->error_format_stack_trace();
-
-  int argc = 2;
-  ScopedVector<Handle<Object>> argv(argc);
-  argv[0] = error;
-  argv[1] = stack_trace;
-
-  Handle<Object> formatted_stack_trace;
-  ASSIGN_RETURN_ON_EXCEPTION(
-      isolate, formatted_stack_trace,
-      Execution::Call(isolate, fun, error, argc, argv.start()), Object);
-
-  return formatted_stack_trace;
-}
-
 bool IsAccessor(Handle<Object> receiver, Handle<Name> name,
                 Handle<JSObject> holder) {
   LookupIterator it(receiver, name, holder,

src/builtins/builtins-error.cc

@@ -5,7 +5,6 @@
 #include "src/builtins/builtins.h"
 #include "src/builtins/builtins-utils.h"
 
-#include "src/accessors.h"
 #include "src/bootstrapper.h"
 #include "src/messages.h"
 #include "src/property-descriptor.h"
@@ -36,67 +35,24 @@ BUILTIN(ErrorCaptureStackTrace) {
   Handle<Object> caller = args.atOrUndefined(isolate, 2);
   FrameSkipMode mode = caller->IsJSFunction() ? SKIP_UNTIL_SEEN : SKIP_NONE;
 
-  // TODO(jgruber): Eagerly format the stack trace and remove accessors.h
-  // include.
-
-  // Handle writes to the global object.
-
-  if (object->IsJSGlobalProxy()) {
-    Map* map = object->map();
-    if (map->has_hidden_prototype()) {
-      object = handle(JSGlobalObject::cast(map->prototype()), isolate);
-    }
-  }
-
-  // Check if the stack property is read-only.
-
-  bool is_extensible = true;
-  if (!JSObject::IsExtensible(object)) {
-    is_extensible = false;
-  }
-
-  PropertyDescriptor desc;
-  Maybe<bool> owned = JSReceiver::GetOwnPropertyDescriptor(
-      isolate, object, isolate->factory()->stack_string(), &desc);
-  if (owned.FromMaybe(false)) {
-    if (!desc.configurable() || !desc.writable()) {
-      is_extensible = false;
-    }
-  }
-
-  if (!is_extensible) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate, NewTypeError(MessageTemplate::kDefineDisallowed,
-                              isolate->factory()->stack_string(), object));
-  }
-
-  // Add stack accessors to the given object
-
-  Handle<Map> map(object->map());
-  PropertyAttributes attribs = DONT_ENUM;
-  Handle<AccessorInfo> error_stack =
-      Accessors::ErrorStackInfo(isolate, attribs);
-  {
-    AccessorConstantDescriptor d(Handle<Name>(Name::cast(error_stack->name())),
-                                 error_stack, attribs);
-    Handle<DescriptorArray> old_descriptors(map->instance_descriptors());
-    int index = old_descriptors->SearchWithCache(isolate, *d.GetKey(), *map);
-    if (index == DescriptorArray::kNotFound) {
-      // TODO(jgruber): This ensures we do not crash when CaptureStackTrace is
-      // called on an object with an existing "stack" property. This will be
-      // removed as soon as we move to eager trace formatting.
-      Handle<Map> new_map =
-          Map::CopyInsertDescriptor(map, &d, INSERT_TRANSITION);
-      JSObject::MigrateToMap(object, new_map, 1);
-    }
-  }
-
   // Collect the stack trace.
 
   RETURN_FAILURE_ON_EXCEPTION(isolate,
                               isolate->CaptureAndSetDetailedStackTrace(object));
+
+  // Eagerly format the stack trace and set the stack property.
+
+  Handle<Object> stack_trace =
+      isolate->CaptureSimpleStackTrace(object, mode, caller);
+
+  Handle<Object> formatted_stack_trace;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, formatted_stack_trace,
+      FormatStackTrace(isolate, object, stack_trace));
+
   RETURN_FAILURE_ON_EXCEPTION(
-      isolate, isolate->CaptureAndSetSimpleStackTrace(object, mode, caller));
+      isolate, JSObject::SetProperty(object, isolate->factory()->stack_string(),
+                                     formatted_stack_trace, STRICT));
 
   return *isolate->factory()->undefined_value();
 }

src/isolate.cc

@@ -563,9 +563,8 @@ MaybeHandle<JSReceiver> Isolate::CaptureAndSetDetailedStackTrace(
     Handle<JSArray> stack_trace = CaptureCurrentStackTrace(
         stack_trace_for_uncaught_exceptions_frame_limit_,
         stack_trace_for_uncaught_exceptions_options_);
-    // TODO(jgruber): Set back to STRICT once we have eagerly formatted traces.
     RETURN_ON_EXCEPTION(
-        this, JSReceiver::SetProperty(error_object, key, stack_trace, SLOPPY),
+        this, JSReceiver::SetProperty(error_object, key, stack_trace, STRICT),
         JSReceiver);
   }
   return error_object;
@@ -578,9 +577,8 @@ MaybeHandle<JSReceiver> Isolate::CaptureAndSetSimpleStackTrace(
   Handle<Name> key = factory()->stack_trace_symbol();
   Handle<Object> stack_trace =
       CaptureSimpleStackTrace(error_object, mode, caller);
-  // TODO(jgruber): Set back to STRICT once we have eagerly formatted traces.
   RETURN_ON_EXCEPTION(
-      this, JSReceiver::SetProperty(error_object, key, stack_trace, SLOPPY),
+      this, JSReceiver::SetProperty(error_object, key, stack_trace, STRICT),
       JSReceiver);
   return error_object;
 }

src/messages.cc

@@ -364,6 +364,23 @@ bool CallSite::IsConstructor() {
   return constructor.is_identical_to(fun_);
 }
 
+MaybeHandle<Object> FormatStackTrace(Isolate* isolate, Handle<JSObject> error,
+                                     Handle<Object> stack_trace) {
+  // TODO(jgruber): Port FormatStackTrace from JS.
+  Handle<JSFunction> fun = isolate->error_format_stack_trace();
+
+  int argc = 2;
+  ScopedVector<Handle<Object>> argv(argc);
+  argv[0] = error;
+  argv[1] = stack_trace;
+
+  Handle<Object> formatted_stack_trace;
+  ASSIGN_RETURN_ON_EXCEPTION(
+      isolate, formatted_stack_trace,
+      Execution::Call(isolate, fun, error, argc, argv.start()), Object);
+
+  return formatted_stack_trace;
+}
 
 Handle<String> MessageTemplate::FormatMessage(Isolate* isolate,
                                               int template_index,

src/messages.h

@@ -71,6 +71,11 @@ class CallSite {
   uint32_t wasm_func_index_ = static_cast<uint32_t>(-1);
 };
 
+// Formats a textual stack trace from the given structured stack trace.
+// Note that this can call arbitrary JS code through Error.prepareStackTrace.
+MaybeHandle<Object> FormatStackTrace(Isolate* isolate, Handle<JSObject> error,
+                                     Handle<Object> stack_trace);
+
 // Determines how stack trace collection skips frames.
 enum FrameSkipMode {
   // Unconditionally skips the first frame. Used e.g. when the Error constructor

test/mjsunit/stack-traces.js

@@ -380,3 +380,9 @@ assertEquals(undefined, error.stack);
 // Check that repeated trace collection does not crash.
 error = new Error();
 Error.captureStackTrace(error);
+
+// Check that exceptions thrown within prepareStackTrace throws an exception.
+Error.prepareStackTrace = function(e, frames) { throw 42; }
+
+var x = {}
+assertThrows(() => Error.captureStackTrace(x));