From c4ef8a8d6e52fc2f434654ab4934c2dc73411047 Mon Sep 17 00:00:00 2001 From: jgruber Date: Thu, 21 Jul 2016 02:07:30 -0700 Subject: [PATCH] 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 | 56 ++++++++++++++++++++++++----- src/contexts.h | 1 + src/isolate.cc | 65 +++++++++++++++++++--------------- src/isolate.h | 5 ++- src/js/messages.js | 5 +++ src/messages.cc | 47 ------------------------ src/messages.h | 15 -------- 7 files changed, 92 insertions(+), 102 deletions(-) diff --git a/src/builtins/builtins-error.cc b/src/builtins/builtins-error.cc index 0311f8c682..458d15c0a0 100644 --- a/src/builtins/builtins-error.cc +++ b/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(), - Handle::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 target = args.target(); + Handle new_target; + if (args.new_target()->IsJSReceiver()) { + new_target = Handle::cast(args.new_target()); + } else { + new_target = target; + } + + // 2. Let O be ? OrdinaryCreateFromConstructor(newTarget, "%ErrorPrototype%", + // « [[ErrorData]] »). + Handle 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 msg = args.atOrUndefined(isolate, 1); + if (!msg->IsUndefined(isolate)) { + Handle 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 object = Handle::cast(object_obj); Handle 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(); } diff --git a/src/contexts.h b/src/contexts.h index 6640854d51..6aa80f2907 100644 --- a/src/contexts.h +++ b/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) diff --git a/src/isolate.cc b/src/isolate.cc index 9a6e6bb33a..5c7423288f 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -329,9 +329,16 @@ static Handle MaybeGrow(Isolate* isolate, } class StackTraceHelper { + private: + enum FrameSkipMode { + SKIP_FIRST, + SKIP_UNTIL_SEEN, + SKIP_NONE, + }; + public: - StackTraceHelper(Isolate* isolate, FrameSkipMode mode, Handle caller) - : isolate_(isolate), mode_(mode), caller_(caller) { + StackTraceHelper(Isolate* isolate, Handle 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 caller_; + FrameSkipMode mode_; + Handle caller_; bool skip_next_frame_; int sloppy_frames_; @@ -430,7 +434,6 @@ class StackTraceHelper { }; Handle Isolate::CaptureSimpleStackTrace(Handle error_object, - FrameSkipMode mode, Handle caller) { DisallowJavascriptExecution no_js(this); @@ -449,7 +452,7 @@ Handle Isolate::CaptureSimpleStackTrace(Handle error_object, Handle 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 Isolate::CaptureAndSetDetailedStackTrace( } MaybeHandle Isolate::CaptureAndSetSimpleStackTrace( - Handle error_object, FrameSkipMode mode, - Handle caller) { + Handle error_object, Handle caller) { // Capture stack trace for simple stack trace string formatting. Handle key = factory()->stack_trace_symbol(); - Handle stack_trace = - CaptureSimpleStackTrace(error_object, mode, caller); + Handle 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 accessing_context, Object* Isolate::StackOverflow() { DisallowJavascriptExecution no_js(this); HandleScope scope(this); - - Handle fun = range_error_function(); - Handle 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 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 boilerplate = stack_overflow_boilerplate(); + Handle copy = factory()->CopyJSObject(boilerplate); + CaptureAndSetSimpleStackTrace(copy, factory()->undefined_value()); + exception = copy; + } Throw(*exception, nullptr); #ifdef VERIFY_HEAP diff --git a/src/isolate.h b/src/isolate.h index a4e8129f1c..918b14fb08 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -696,13 +696,12 @@ class Isolate { int frame_limit, StackTrace::StackTraceOptions options); Handle CaptureSimpleStackTrace(Handle error_object, - FrameSkipMode mode, Handle caller); MaybeHandle CaptureAndSetDetailedStackTrace( Handle error_object); MaybeHandle CaptureAndSetSimpleStackTrace( - Handle error_object, FrameSkipMode mode, - Handle caller); + Handle error_object, + Handle caller = Handle()); Handle GetDetailedStackTrace(Handle error_object); // Returns if the given context may access the given global object. If diff --git a/src/js/messages.js b/src/js/messages.js index e833522d4f..f327fee18d 100644 --- a/src/js/messages.js +++ b/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) { diff --git a/src/messages.cc b/src/messages.cc index 3213d2cbb9..c007f069dd 100644 --- a/src/messages.cc +++ b/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 MessageTemplate::FormatMessage(int template_index, return builder.Finish(); } -MaybeHandle ConstructError(Isolate* isolate, Handle target, - Handle new_target, - Handle 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 new_target_recv = - new_target->IsJSReceiver() ? Handle::cast(new_target) - : Handle::cast(target); - - // 2. Let O be ? OrdinaryCreateFromConstructor(newTarget, "%ErrorPrototype%", - // « [[ErrorData]] »). - Handle 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 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); - - return err; -} } // namespace internal } // namespace v8 diff --git a/src/messages.h b/src/messages.h index 164ede114e..aeba83ce72 100644 --- a/src/messages.h +++ b/src/messages.h @@ -71,21 +71,6 @@ class CallSite { uint32_t wasm_func_index_ = static_cast(-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 ConstructError(Isolate* isolate, Handle target, - Handle new_target, - Handle message, FrameSkipMode mode, - bool suppress_detailed_trace); - #define MESSAGE_TEMPLATES(T) \ /* Error */ \ T(None, "") \