[error] Lazy stack trace formatting for Error.captureStackTrace

This reinstates the old behavior of Error.captureStackTrace prior to
4feafee9.  Like the builtin Error constructors, captureStackTrace now formats
the stack trace lazily once it is accessed.

Bug: v8:5962
Change-Id: I03821b73d26b7b40809a1fea98f9c820bfa05d6b
Reviewed-on: https://chromium-review.googlesource.com/574530Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46727}

src/builtins/builtins-error.cc

@@ -5,6 +5,7 @@
 #include "src/builtins/builtins.h"
 #include "src/builtins/builtins-utils.h"
 
+#include "src/accessors.h"
 #include "src/counters.h"
 #include "src/messages.h"
 #include "src/objects-inl.h"
@@ -40,10 +41,12 @@ BUILTIN(ErrorConstructor) {
 BUILTIN(ErrorCaptureStackTrace) {
   HandleScope scope(isolate);
   Handle<Object> object_obj = args.atOrUndefined(isolate, 1);
+
   if (!object_obj->IsJSObject()) {
     THROW_NEW_ERROR_RETURN_FAILURE(
         isolate, NewTypeError(MessageTemplate::kInvalidArgument, object_obj));
   }
+
   Handle<JSObject> object = Handle<JSObject>::cast(object_obj);
   Handle<Object> caller = args.atOrUndefined(isolate, 2);
   FrameSkipMode mode = caller->IsJSFunction() ? SKIP_UNTIL_SEEN : SKIP_FIRST;
@@ -52,27 +55,24 @@ BUILTIN(ErrorCaptureStackTrace) {
 
   RETURN_FAILURE_ON_EXCEPTION(isolate,
                               isolate->CaptureAndSetDetailedStackTrace(object));
+  RETURN_FAILURE_ON_EXCEPTION(
+      isolate, isolate->CaptureAndSetSimpleStackTrace(object, mode, caller));
+
+  // Add the stack accessors.
+
+  Handle<AccessorInfo> error_stack =
+      Accessors::ErrorStackInfo(isolate, DONT_ENUM);
 
-  // Eagerly format the stack trace and set the stack property.
-
-  Handle<Object> stack_trace =
-      isolate->CaptureSimpleStackTrace(object, mode, caller);
-  if (!stack_trace->IsJSArray()) return isolate->heap()->undefined_value();
-
-  Handle<Object> formatted_stack_trace;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, formatted_stack_trace,
-      ErrorUtils::FormatStackTrace(isolate, object, stack_trace));
-
-  PropertyDescriptor desc;
-  desc.set_configurable(true);
-  desc.set_writable(true);
-  desc.set_value(formatted_stack_trace);
-  Maybe<bool> status = JSReceiver::DefineOwnProperty(
-      isolate, object, isolate->factory()->stack_string(), &desc,
-      Object::THROW_ON_ERROR);
-  if (!status.IsJust()) return isolate->heap()->exception();
-  CHECK(status.FromJust());
+  // Explicitly check for frozen objects. Other access checks are performed by
+  // the LookupIterator in SetAccessor below.
+  if (!JSObject::IsExtensible(object)) {
+    return isolate->Throw(*isolate->factory()->NewTypeError(
+        MessageTemplate::kDefineDisallowed,
+        handle(error_stack->name(), isolate)));
+  }
+
+  RETURN_FAILURE_ON_EXCEPTION(isolate,
+                              JSObject::SetAccessor(object, error_stack));
   return isolate->heap()->undefined_value();
 }
 

test/mjsunit/stack-traces.js

@@ -391,9 +391,7 @@ assertTrue(desc.writable);
 
 // Check that exceptions thrown within prepareStackTrace throws an exception.
 Error.prepareStackTrace = function(e, frames) { throw 42; }
-
-var x = {}
-assertThrows(() => Error.captureStackTrace(x));
+assertThrows(() => new Error().stack);
 
 // Check that we don't crash when CaptureSimpleStackTrace returns undefined.
 var o = {};