Skip to content

Loading…

Add patch for stack overflow messages #4252

Closed
wants to merge 1 commit into from

3 participants

@dizzyd

This patch for V8 originates from: http://codereview.chromium.org/11275186/

The original patch from that URL does not apply cleanly to the v0.8 version of V8, so I've resolved the conflicts by hand.

@piscisaureus

Is it possible to upload this to chromium backported patch to chromium code review and ask the v8 guys to land it in v8 v3.11.10 ?

@dizzyd

I have a dialog going with one of the V8 engineers -- I'll see what he thinks about backporting it to that version.

@dizzyd

Nope -- the V8 team will not backport this as it's a new feature and they don't receive real-world crash data anymore for 3.11.x (because of it's age). This patch will land in 3.15, but not sure how prepared node is to make that leap. :)

@isaacs

We have this in 0.10 already. Thanks!

@isaacs isaacs closed this
@smanders smanders pushed a commit to smanders/node that referenced this pull request
@cjihrig cjihrig 2015-12-16, Version 5.3.0 (Stable)
Notable changes:

* buffer:
  - Buffer.prototype.includes() has been added to keep parity
    with TypedArrays. (Alexander Martin) #3567.
* domains:
  - Fix handling of uncaught exceptions.
    (Julien Gilli) #3654.
* https:
  - Added support for disabling session caching.
    (Fedor Indutny) #4252.
* repl:
  - Allow third party modules to be imported using
    require(). This corrects a regression from 5.2.0.
    (Ben Noordhuis) #4215.
* deps:
  - Upgrade libuv to 1.8.0.
    (Saúl Ibarra Corretgé) #4276.

PR-URL: nodejs/node#4281

Conflicts:
	src/node_version.h
e5774c9
@richardlau richardlau pushed a commit to andrewlow/node that referenced this pull request
@cjihrig cjihrig 2015-12-16, Version 5.3.0 (Stable)
Notable changes:

* buffer:
  - Buffer.prototype.includes() has been added to keep parity
    with TypedArrays. (Alexander Martin) #3567.
* domains:
  - Fix handling of uncaught exceptions.
    (Julien Gilli) #3654.
* https:
  - Added support for disabling session caching.
    (Fedor Indutny) #4252.
* repl:
  - Allow third party modules to be imported using
    require(). This corrects a regression from 5.2.0.
    (Ben Noordhuis) #4215.
* deps:
  - Upgrade libuv to 1.8.0.
    (Saúl Ibarra Corretgé) #4276.

PR-URL: nodejs/node#4281
b21110f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 163 additions and 108 deletions.
  1. +112 −6 deps/v8/src/isolate.cc
  2. +4 −1 deps/v8/src/isolate.h
  3. +31 −11 deps/v8/src/messages.js
  4. +15 −90 deps/v8/src/runtime.cc
  5. +1 −0 deps/v8/src/runtime.h
View
118 deps/v8/src/isolate.cc
@@ -527,7 +527,101 @@ Handle<String> Isolate::StackTraceString() {
}
-void Isolate::CaptureAndSetCurrentStackTraceFor(Handle<JSObject> error_object) {
+// Determines whether the given stack frame should be displayed in
+// a stack trace. The caller is the error constructor that asked
+// for the stack trace to be collected. The first time a construct
+// call to this function is encountered it is skipped. The seen_caller
+// in/out parameter is used to remember if the caller has been seen
+// yet.
+bool ShowFrameInStackTrace(StackFrame* raw_frame,
+ Object* caller,
+ bool* seen_caller) {
+ // Only display JS frames.
+ if (!raw_frame->is_java_script()) {
+ return false;
+ }
+ JavaScriptFrame* frame = JavaScriptFrame::cast(raw_frame);
+ Object* raw_fun = frame->function();
+ // Not sure when this can happen but skip it just in case.
+ if (!raw_fun->IsJSFunction()) {
+ return false;
+ }
+ if ((raw_fun == caller) && !(*seen_caller)) {
+ *seen_caller = true;
+ return false;
+ }
+ // Skip all frames until we've seen the caller.
+ if (!(*seen_caller)) return false;
+ // Also, skip non-visible built-in functions and any call with the builtins
+ // object as receiver, so as to not reveal either the builtins object or
+ // an internal function.
+ // The --builtins-in-stack-traces command line flag allows including
+ // internal call sites in the stack trace for debugging purposes.
+ if (!FLAG_builtins_in_stack_traces) {
+ JSFunction* fun = JSFunction::cast(raw_fun);
+ if (frame->receiver()->IsJSBuiltinsObject() ||
+ (fun->IsBuiltin() && !fun->shared()->native())) {
+ return false;
+ }
+ }
+ return true;
+}
+
+
+Handle<JSArray> Isolate::CaptureSimpleStackTrace(Handle<JSObject> error_object,
+ Handle<Object> caller,
+ int limit) {
+ limit = Max(limit, 0); // Ensure that limit is not negative.
+ int initial_size = Min(limit, 10);
+ Handle<FixedArray> elements =
+ factory()->NewFixedArrayWithHoles(initial_size * 4);
+
+ StackFrameIterator iter(this);
+ // If the caller parameter is a function we skip frames until we're
+ // under it before starting to collect.
+ bool seen_caller = !caller->IsJSFunction();
+ int cursor = 0;
+ int frames_seen = 0;
+ while (!iter.done() && frames_seen < limit) {
+ StackFrame* raw_frame = iter.frame();
+ if (ShowFrameInStackTrace(raw_frame, *caller, &seen_caller)) {
+ frames_seen++;
+ JavaScriptFrame* frame = JavaScriptFrame::cast(raw_frame);
+ // Set initial size to the maximum inlining level + 1 for the outermost
+ // function.
+ List<FrameSummary> frames(Compiler::kMaxInliningLevels + 1);
+ frame->Summarize(&frames);
+ for (int i = frames.length() - 1; i >= 0; i--) {
+ if (cursor + 4 > elements->length()) {
+ int new_capacity = JSObject::NewElementsCapacity(elements->length());
+ Handle<FixedArray> new_elements =
+ factory()->NewFixedArrayWithHoles(new_capacity);
+ for (int i = 0; i < cursor; i++) {
+ new_elements->set(i, elements->get(i));
+ }
+ elements = new_elements;
+ }
+ ASSERT(cursor + 4 <= elements->length());
+
+ Handle<Object> recv = frames[i].receiver();
+ Handle<JSFunction> fun = frames[i].function();
+ Handle<Code> code = frames[i].code();
+ Handle<Smi> offset(Smi::FromInt(frames[i].offset()));
+ elements->set(cursor++, *recv);
+ elements->set(cursor++, *fun);
+ elements->set(cursor++, *code);
+ elements->set(cursor++, *offset);
+ }
+ }
+ iter.Advance();
+ }
+ Handle<JSArray> result = factory()->NewJSArrayWithElements(elements);
+ result->set_length(Smi::FromInt(cursor));
+ return result;
+}
+
+
+void Isolate::CaptureAndSetDetailedStackTrace(Handle<JSObject> error_object) {
if (capture_stack_trace_for_uncaught_exceptions_) {
// Capture stack trace for a detailed exception message.
Handle<String> key = factory()->hidden_stack_trace_symbol();
@@ -899,12 +993,24 @@ Failure* Isolate::StackOverflow() {
Handle<String> key = factory()->stack_overflow_symbol();
Handle<JSObject> boilerplate =
Handle<JSObject>::cast(GetProperty(js_builtins_object(), key));
- Handle<Object> exception = Copy(boilerplate);
- // TODO(1240995): To avoid having to call JavaScript code to compute
- // the message for stack overflow exceptions which is very likely to
- // double fault with another stack overflow exception, we use a
- // precomputed message.
+ Handle<JSObject> exception = Copy(boilerplate);
DoThrow(*exception, NULL);
+
+ // Get stack trace limit.
+ Handle<Object> error = GetProperty(js_builtins_object(), "$Error");
+ if (!error->IsJSObject()) return Failure::Exception();
+ Handle<Object> stack_trace_limit =
+ GetProperty(Handle<JSObject>::cast(error), "stackTraceLimit");
+ if (!stack_trace_limit->IsNumber()) return Failure::Exception();
+ int limit = static_cast<int>(stack_trace_limit->Number());
+
+ // Store the raw stack trace as hidden property so that the stack trace
+ // string can be lazily formatted in javascript.
+ Handle<JSArray> stack_trace = CaptureSimpleStackTrace(
+ exception, factory()->undefined_value(), limit);
+ JSObject::SetHiddenProperty(exception,
+ factory()->hidden_stack_trace_symbol(),
+ stack_trace);
return Failure::Exception();
}
View
5 deps/v8/src/isolate.h
@@ -701,7 +701,10 @@ class Isolate {
int frame_limit,
StackTrace::StackTraceOptions options);
- void CaptureAndSetCurrentStackTraceFor(Handle<JSObject> error_object);
+ Handle<JSArray> CaptureSimpleStackTrace(Handle<JSObject> error_object,
+ Handle<Object> caller,
+ int limit);
+ void CaptureAndSetDetailedStackTrace(Handle<JSObject> error_object);
// Returns if the top context may access the given global object. If
// the result is false, the pending exception is guaranteed to be
View
42 deps/v8/src/messages.js
@@ -756,23 +756,17 @@ function GetStackTraceLine(recv, fun, pos, isGlobal) {
// Defines accessors for a property that is calculated the first time
// the property is read.
-function DefineOneShotAccessor(obj, name, fun) {
+function DefineOneShotAccessor(obj, name, value_factory) {
// Note that the accessors consistently operate on 'obj', not 'this'.
// Since the object may occur in someone else's prototype chain we
// can't rely on 'this' being the same as 'obj'.
- var hasBeenSet = false;
- var value;
var getter = function() {
- if (hasBeenSet) {
- return value;
- }
- hasBeenSet = true;
- value = fun(obj);
+ value = value_factory(obj);
+ %DefineOrRedefineDataProperty(obj, name, value, NONE);
return value;
};
var setter = function(v) {
- hasBeenSet = true;
- value = v;
+ %DefineOrRedefineDataProperty(this, name, v, NONE);
};
%DefineOrRedefineAccessorProperty(obj, name, getter, setter, DONT_ENUM);
}
@@ -1254,4 +1248,30 @@ InstallFunctions($Error.prototype, DONT_ENUM, ['toString', ErrorToString]);
// Boilerplate for exceptions for stack overflows. Used from
// Isolate::StackOverflow().
-var kStackOverflowBoilerplate = MakeRangeError('stack_overflow', []);
+function SetUpStackOverflowBoilerplate() {
+ var boilerplate = MakeRangeError('stack_overflow', []);
+
+ function getter() {
+ var holder = this;
+ while (!IS_ERROR(holder)) {
+ holder = %GetPrototype(holder);
+ if (holder == null) return MakeSyntaxError('illegal_access', []);
+ }
+ var raw_stack = %GetOverflowedRawStackTrace(holder);
+ var result = IS_ARRAY(raw_stack) ? FormatRawStackTrace(holder, raw_stack)
+ : void 0;
+ %DefineOrRedefineDataProperty(holder, 'stack', result, NONE);
+ return result;
+ }
+
+ function setter(v) {
+ %DefineOrRedefineDataProperty(this, 'stack', v, NONE);
+ }
+
+ %DefineOrRedefineAccessorProperty(
+ boilerplate, 'stack', getter, setter, DONT_ENUM);
+
+ return boilerplate;
+}
+
+var kStackOverflowBoilerplate = SetUpStackOverflowBoilerplate();
View
105 deps/v8/src/runtime.cc
@@ -13162,47 +13162,6 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetScript) {
}
-// Determines whether the given stack frame should be displayed in
-// a stack trace. The caller is the error constructor that asked
-// for the stack trace to be collected. The first time a construct
-// call to this function is encountered it is skipped. The seen_caller
-// in/out parameter is used to remember if the caller has been seen
-// yet.
-static bool ShowFrameInStackTrace(StackFrame* raw_frame,
- Object* caller,
- bool* seen_caller) {
- // Only display JS frames.
- if (!raw_frame->is_java_script()) {
- return false;
- }
- JavaScriptFrame* frame = JavaScriptFrame::cast(raw_frame);
- Object* raw_fun = frame->function();
- // Not sure when this can happen but skip it just in case.
- if (!raw_fun->IsJSFunction()) {
- return false;
- }
- if ((raw_fun == caller) && !(*seen_caller)) {
- *seen_caller = true;
- return false;
- }
- // Skip all frames until we've seen the caller.
- if (!(*seen_caller)) return false;
- // Also, skip non-visible built-in functions and any call with the builtins
- // object as receiver, so as to not reveal either the builtins object or
- // an internal function.
- // The --builtins-in-stack-traces command line flag allows including
- // internal call sites in the stack trace for debugging purposes.
- if (!FLAG_builtins_in_stack_traces) {
- JSFunction* fun = JSFunction::cast(raw_fun);
- if (frame->receiver()->IsJSBuiltinsObject() ||
- (fun->IsBuiltin() && !fun->shared()->native())) {
- return false;
- }
- }
- return true;
-}
-
-
// Collect the raw data for a stack trace. Returns an array of 4
// element segments each containing a receiver, function, code and
// native code offset.
@@ -13213,57 +13172,23 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CollectStackTrace) {
CONVERT_NUMBER_CHECKED(int32_t, limit, Int32, args[2]);
HandleScope scope(isolate);
- Factory* factory = isolate->factory();
+ // Optionally capture a more detailed stack trace for the message.
+ isolate->CaptureAndSetDetailedStackTrace(error_object);
+ // Capture a simple stack trace for the stack property.
+ return *isolate->CaptureSimpleStackTrace(error_object, caller, limit);
+}
- limit = Max(limit, 0); // Ensure that limit is not negative.
- int initial_size = Min(limit, 10);
- Handle<FixedArray> elements =
- factory->NewFixedArrayWithHoles(initial_size * 4);
- StackFrameIterator iter(isolate);
- // If the caller parameter is a function we skip frames until we're
- // under it before starting to collect.
- bool seen_caller = !caller->IsJSFunction();
- int cursor = 0;
- int frames_seen = 0;
- while (!iter.done() && frames_seen < limit) {
- StackFrame* raw_frame = iter.frame();
- if (ShowFrameInStackTrace(raw_frame, *caller, &seen_caller)) {
- frames_seen++;
- JavaScriptFrame* frame = JavaScriptFrame::cast(raw_frame);
- // Set initial size to the maximum inlining level + 1 for the outermost
- // function.
- List<FrameSummary> frames(Compiler::kMaxInliningLevels + 1);
- frame->Summarize(&frames);
- for (int i = frames.length() - 1; i >= 0; i--) {
- if (cursor + 4 > elements->length()) {
- int new_capacity = JSObject::NewElementsCapacity(elements->length());
- Handle<FixedArray> new_elements =
- factory->NewFixedArrayWithHoles(new_capacity);
- for (int i = 0; i < cursor; i++) {
- new_elements->set(i, elements->get(i));
- }
- elements = new_elements;
- }
- ASSERT(cursor + 4 <= elements->length());
-
- Handle<Object> recv = frames[i].receiver();
- Handle<JSFunction> fun = frames[i].function();
- Handle<Code> code = frames[i].code();
- Handle<Smi> offset(Smi::FromInt(frames[i].offset()));
- elements->set(cursor++, *recv);
- elements->set(cursor++, *fun);
- elements->set(cursor++, *code);
- elements->set(cursor++, *offset);
- }
- }
- iter.Advance();
- }
- Handle<JSArray> result = factory->NewJSArrayWithElements(elements);
- // Capture and attach a more detailed stack trace if necessary.
- isolate->CaptureAndSetCurrentStackTraceFor(error_object);
- result->set_length(Smi::FromInt(cursor));
- return *result;
+// Retrieve the raw stack trace collected on stack overflow and delete
+// it since it is used only once to avoid keeping it alive.
+RUNTIME_FUNCTION(MaybeObject*, Runtime_GetOverflowedRawStackTrace) {
+ ASSERT_EQ(args.length(), 1);
+ CONVERT_ARG_CHECKED(JSObject, error_object, 0);
+ String* key = isolate->heap()->hidden_stack_trace_symbol();
+ Object* result = error_object->GetHiddenProperty(key);
+ RUNTIME_ASSERT(result->IsJSArray() || result->IsUndefined());
+ error_object->DeleteHiddenProperty(key);
+ return result;
}
View
1 deps/v8/src/runtime.h
@@ -232,6 +232,7 @@ namespace internal {
F(FunctionIsBuiltin, 1, 1) \
F(GetScript, 1, 1) \
F(CollectStackTrace, 3, 1) \
+ F(GetOverflowedRawStackTrace, 1, 1) \
F(GetV8Version, 0, 1) \
\
F(ClassOf, 1, 1) \
Something went wrong with that request. Please try again.