Revert of Remove stack overflow boilerplate (patchset #3 id:40001 of...

Revert of Remove stack overflow boilerplate (patchset #3 id:40001 of https://codereview.chromium.org/2161953003/ )

Reason for revert:
Clusterfuzz failures in parent CL https://codereview.chromium.org/2142933003/

Original issue's description:
> Remove stack overflow boilerplate
>
> We no longer need to prepare the stack overflow error in advance now that
> Errors are constructed in C++.
>
> R=yangguo@chromium.org
> BUG=
>
> Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
> Cr-Commit-Position: refs/heads/master@{#37923}

TBR=yangguo@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

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

src/builtins/builtins-error.cc

@@ -7,7 +7,6 @@
 
 #include "src/accessors.h"
 #include "src/bootstrapper.h"
-#include "src/messages.h"
 #include "src/property-descriptor.h"
 #include "src/string-builder.h"
 
@@ -17,11 +16,53 @@ namespace internal {
 // ES6 section 19.5.1.1 Error ( message )
 BUILTIN(ErrorConstructor) {
   HandleScope scope(isolate);
-  RETURN_RESULT_OR_FAILURE(
-      isolate,
-      ConstructError(isolate, args.target<JSFunction>(),
-                     Handle<Object>::cast(args.new_target()),
-                     args.atOrUndefined(isolate, 1), SKIP_FIRST, false));
+
+  // 1. If NewTarget is undefined, let newTarget be the active function object,
+  // else let newTarget be NewTarget.
+
+  Handle<JSFunction> target = args.target<JSFunction>();
+  Handle<JSReceiver> new_target;
+  if (args.new_target()->IsJSReceiver()) {
+    new_target = Handle<JSReceiver>::cast(args.new_target());
+  } else {
+    new_target = target;
+  }
+
+  // 2. Let O be ? OrdinaryCreateFromConstructor(newTarget, "%ErrorPrototype%",
+  //    « [[ErrorData]] »).
+  Handle<JSObject> err;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, err,
+                                     JSObject::New(target, new_target));
+
+  // 3. If message is not undefined, then
+  //  a. Let msg be ? ToString(message).
+  //  b. Let msgDesc be the PropertyDescriptor{[[Value]]: msg, [[Writable]]:
+  //     true, [[Enumerable]]: false, [[Configurable]]: true}.
+  //  c. Perform ! DefinePropertyOrThrow(O, "message", msgDesc).
+  // 4. Return O.
+
+  Handle<Object> msg = args.atOrUndefined(isolate, 1);
+  if (!msg->IsUndefined(isolate)) {
+    Handle<String> msg_string;
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, msg_string,
+                                       Object::ToString(isolate, msg));
+    RETURN_FAILURE_ON_EXCEPTION(
+        isolate,
+        JSObject::SetOwnPropertyIgnoreAttributes(
+            err, isolate->factory()->message_string(), msg_string, DONT_ENUM));
+  }
+
+  // Capture the stack trace unless we're setting up.
+  if (!isolate->bootstrapper()->IsActive()) {
+    // Optionally capture a more detailed stack trace for the message.
+    RETURN_FAILURE_ON_EXCEPTION(isolate,
+                                isolate->CaptureAndSetDetailedStackTrace(err));
+    // Capture a simple stack trace for the stack property.
+    RETURN_FAILURE_ON_EXCEPTION(isolate,
+                                isolate->CaptureAndSetSimpleStackTrace(err));
+  }
+
+  return *err;
 }
 
 // static
@@ -34,7 +75,6 @@ BUILTIN(ErrorCaptureStackTrace) {
   }
   Handle<JSObject> object = Handle<JSObject>::cast(object_obj);
   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.
@@ -96,7 +136,7 @@ BUILTIN(ErrorCaptureStackTrace) {
   RETURN_FAILURE_ON_EXCEPTION(isolate,
                               isolate->CaptureAndSetDetailedStackTrace(object));
   RETURN_FAILURE_ON_EXCEPTION(
-      isolate, isolate->CaptureAndSetSimpleStackTrace(object, mode, caller));
+      isolate, isolate->CaptureAndSetSimpleStackTrace(object, caller));
 
   return *isolate->factory()->undefined_value();
 }

src/contexts.h

@@ -133,6 +133,7 @@ enum BindingFlags {
   V(SET_ADD_METHOD_INDEX, JSFunction, set_add)                              \
   V(SET_DELETE_METHOD_INDEX, JSFunction, set_delete)                        \
   V(SET_HAS_METHOD_INDEX, JSFunction, set_has)                              \
+  V(STACK_OVERFLOW_BOILERPLATE_INDEX, JSObject, stack_overflow_boilerplate) \
   V(SYNTAX_ERROR_FUNCTION_INDEX, JSFunction, syntax_error_function)         \
   V(TYPE_ERROR_FUNCTION_INDEX, JSFunction, type_error_function)             \
   V(URI_ERROR_FUNCTION_INDEX, JSFunction, uri_error_function)

src/isolate.cc

@@ -329,9 +329,16 @@ static Handle<FixedArray> MaybeGrow(Isolate* isolate,
 }
 
 class StackTraceHelper {
+ private:
+  enum FrameSkipMode {
+    SKIP_FIRST,
+    SKIP_UNTIL_SEEN,
+    SKIP_NONE,
+  };
+
  public:
-  StackTraceHelper(Isolate* isolate, FrameSkipMode mode, Handle<Object> caller)
-      : isolate_(isolate), mode_(mode), caller_(caller) {
+  StackTraceHelper(Isolate* isolate, Handle<Object> caller)
+      : isolate_(isolate), caller_(caller) {
     // The caller parameter can be used to skip a specific set of frames in the
     // stack trace. It can be:
     // * null, when called from a standard error constructor. We unconditionally
@@ -340,18 +347,15 @@ class StackTraceHelper {
     // * a JSFunction, when called by the user from Error.captureStackTrace().
     //   We skip each frame until encountering the caller function.
     // * For any other value, all frames are included in the trace.
-    switch (mode_) {
-      case SKIP_FIRST:
-        DCHECK(caller_.is_null());
-        skip_next_frame_ = true;
-        break;
-      case SKIP_UNTIL_SEEN:
-        DCHECK(caller_->IsJSFunction());
-        skip_next_frame_ = true;
-        break;
-      case SKIP_NONE:
-        skip_next_frame_ = false;
-        break;
+    if (caller_.is_null()) {
+      mode_ = SKIP_FIRST;
+      skip_next_frame_ = true;
+    } else if (caller_->IsJSFunction()) {
+      mode_ = SKIP_UNTIL_SEEN;
+      skip_next_frame_ = true;
+    } else {
+      mode_ = SKIP_NONE;
+      skip_next_frame_ = false;
     }
     encountered_strict_function_ = false;
     sloppy_frames_ = 0;
@@ -421,8 +425,8 @@ class StackTraceHelper {
 
   Isolate* isolate_;
 
-  const FrameSkipMode mode_;
-  const Handle<Object> caller_;
+  FrameSkipMode mode_;
+  Handle<Object> caller_;
   bool skip_next_frame_;
 
   int sloppy_frames_;
@@ -430,7 +434,6 @@ class StackTraceHelper {
 };
 
 Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
-                                                FrameSkipMode mode,
                                                 Handle<Object> caller) {
   DisallowJavascriptExecution no_js(this);
 
@@ -449,7 +452,7 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
   Handle<FixedArray> elements =
       factory()->NewFixedArrayWithHoles(initial_size * 4 + 1);
 
-  StackTraceHelper helper(this, mode, caller);
+  StackTraceHelper helper(this, caller);
 
   // First element is reserved to store the number of sloppy frames.
   int cursor = 1;
@@ -569,12 +572,10 @@ MaybeHandle<JSReceiver> Isolate::CaptureAndSetDetailedStackTrace(
 }
 
 MaybeHandle<JSReceiver> Isolate::CaptureAndSetSimpleStackTrace(
-    Handle<JSReceiver> error_object, FrameSkipMode mode,
-    Handle<Object> caller) {
+    Handle<JSReceiver> error_object, Handle<Object> caller) {
   // Capture stack trace for simple stack trace string formatting.
   Handle<Name> key = factory()->stack_trace_symbol();
-  Handle<Object> stack_trace =
-      CaptureSimpleStackTrace(error_object, mode, caller);
+  Handle<Object> stack_trace = CaptureSimpleStackTrace(error_object, 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),
@@ -959,14 +960,20 @@ bool Isolate::MayAccess(Handle<Context> accessing_context,
 Object* Isolate::StackOverflow() {
   DisallowJavascriptExecution no_js(this);
   HandleScope scope(this);
-
-  Handle<JSFunction> fun = range_error_function();
-  Handle<Object> msg = factory()->NewStringFromAsciiChecked(
-      MessageTemplate::TemplateString(MessageTemplate::kStackOverflow));
+  // At this point we cannot create an Error object using its javascript
+  // constructor.  Instead, we copy the pre-constructed boilerplate and
+  // attach the stack trace as a hidden property.
   Handle<Object> exception;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      this, exception, ConstructError(this, fun, fun, msg, SKIP_NONE, true));
-
+  if (bootstrapper()->IsActive()) {
+    // There is no boilerplate to use during bootstrapping.
+    exception = factory()->NewStringFromAsciiChecked(
+        MessageTemplate::TemplateString(MessageTemplate::kStackOverflow));
+  } else {
+    Handle<JSObject> boilerplate = stack_overflow_boilerplate();
+    Handle<JSObject> copy = factory()->CopyJSObject(boilerplate);
+    CaptureAndSetSimpleStackTrace(copy, factory()->undefined_value());
+    exception = copy;
+  }
   Throw(*exception, nullptr);
 
 #ifdef VERIFY_HEAP

src/isolate.h

@@ -696,13 +696,12 @@ class Isolate {
       int frame_limit,
       StackTrace::StackTraceOptions options);
   Handle<Object> CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
-                                         FrameSkipMode mode,
                                          Handle<Object> caller);
   MaybeHandle<JSReceiver> CaptureAndSetDetailedStackTrace(
       Handle<JSReceiver> error_object);
   MaybeHandle<JSReceiver> CaptureAndSetSimpleStackTrace(
-      Handle<JSReceiver> error_object, FrameSkipMode mode,
-      Handle<Object> caller);
+      Handle<JSReceiver> error_object,
+      Handle<Object> caller = Handle<Object>());
   Handle<JSArray> GetDetailedStackTrace(Handle<JSObject> error_object);
 
   // Returns if the given context may access the given global object. If

src/js/messages.js

@@ -637,6 +637,10 @@ function MakeURIError() {
   return MakeGenericError(GlobalURIError, kURIMalformed);
 }
 
+// Boilerplate for exceptions for stack overflows. Used from
+// Isolate::StackOverflow().
+var StackOverflowBoilerplate = MakeRangeError(kStackOverflow);
+
 %InstallToContext([
   "error_format_stack_trace", FormatStackTrace,
   "get_stack_trace_line_fun", GetStackTraceLine,
@@ -647,6 +651,7 @@ function MakeURIError() {
   "message_get_line_number", GetLineNumber,
   "message_get_source_line", GetSourceLine,
   "no_side_effects_to_string_fun", NoSideEffectsToString,
+  "stack_overflow_boilerplate", StackOverflowBoilerplate,
 ]);
 
 utils.Export(function(to) {

src/messages.cc

@@ -5,7 +5,6 @@
 #include "src/messages.h"
 
 #include "src/api.h"
-#include "src/bootstrapper.h"
 #include "src/execution.h"
 #include "src/isolate-inl.h"
 #include "src/keys.h"
@@ -446,52 +445,6 @@ MaybeHandle<String> MessageTemplate::FormatMessage(int template_index,
   return builder.Finish();
 }
 
-MaybeHandle<Object> ConstructError(Isolate* isolate, Handle<JSFunction> target,
-                                   Handle<Object> new_target,
-                                   Handle<Object> message, FrameSkipMode mode,
-                                   bool suppress_detailed_trace) {
-  // 1. If NewTarget is undefined, let newTarget be the active function object,
-  // else let newTarget be NewTarget.
-
-  Handle<JSReceiver> new_target_recv =
-      new_target->IsJSReceiver() ? Handle<JSReceiver>::cast(new_target)
-                                 : Handle<JSReceiver>::cast(target);
-
-  // 2. Let O be ? OrdinaryCreateFromConstructor(newTarget, "%ErrorPrototype%",
-  //    « [[ErrorData]] »).
-  Handle<JSObject> err;
-  ASSIGN_RETURN_ON_EXCEPTION(isolate, err,
-                             JSObject::New(target, new_target_recv), Object);
-
-  // 3. If message is not undefined, then
-  //  a. Let msg be ? ToString(message).
-  //  b. Let msgDesc be the PropertyDescriptor{[[Value]]: msg, [[Writable]]:
-  //     true, [[Enumerable]]: false, [[Configurable]]: true}.
-  //  c. Perform ! DefinePropertyOrThrow(O, "message", msgDesc).
-  // 4. Return O.
-
-  if (!message->IsUndefined(isolate)) {
-    Handle<String> msg_string;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, msg_string,
-                               Object::ToString(isolate, message), Object);
-    RETURN_ON_EXCEPTION(isolate, JSObject::SetOwnPropertyIgnoreAttributes(
-                                     err, isolate->factory()->message_string(),
-                                     msg_string, DONT_ENUM),
-                        Object);
-  }
-
-  // Optionally capture a more detailed stack trace for the message.
-  if (!suppress_detailed_trace) {
-    RETURN_ON_EXCEPTION(isolate, isolate->CaptureAndSetDetailedStackTrace(err),
-                        Object);
-  }
-  // Capture a simple stack trace for the stack property.
-  RETURN_ON_EXCEPTION(isolate, isolate->CaptureAndSetSimpleStackTrace(
-                                   err, mode, Handle<Object>()),
-                      Object);
-
-  return err;
-}
 
 }  // namespace internal
 }  // namespace v8

src/messages.h

@@ -71,21 +71,6 @@ class CallSite {
   uint32_t wasm_func_index_ = static_cast<uint32_t>(-1);
 };
 
-// Determines how stack trace collection skips frames.
-enum FrameSkipMode {
-  // Unconditionally skips the first frame. Used e.g. when the Error constructor
-  // is called, in which case the first frame is always a BUILTIN_EXIT frame.
-  SKIP_FIRST,
-  // Skip all frames until a specified caller function is seen.
-  SKIP_UNTIL_SEEN,
-  SKIP_NONE,
-};
-
-MaybeHandle<Object> ConstructError(Isolate* isolate, Handle<JSFunction> target,
-                                   Handle<Object> new_target,
-                                   Handle<Object> message, FrameSkipMode mode,
-                                   bool suppress_detailed_trace);
-
 #define MESSAGE_TEMPLATES(T)                                                   \
   /* Error */                                                                  \
   T(None, "")                                                                  \