Revision: 13256
Author: [email protected]
Date: Thu Dec 20 08:25:26 2012
Log: Fix several bugs in error stack trace formatting.
GetScriptWrapper can be called recursively:
GetScriptWrapper -> GC -> DeferredFormatStackTrace -> GetScriptWrapper
GC-unsafe code in ErrorObjectList::DeferredFormatStackTrace
Enable overwriting Error.prepareStackTrace by itself while not
causing infinity recursion when it triggers an exception.
[email protected]
BUG=
Review URL: https://chromiumcodereview.appspot.com/11649037
http://code.google.com/p/v8/source/detail?r=13256
Modified:
/branches/bleeding_edge/src/handles.cc
/branches/bleeding_edge/src/heap.cc
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/test/mjsunit/stack-traces.js
=======================================
--- /branches/bleeding_edge/src/handles.cc Mon Dec 17 07:56:16 2012
+++ /branches/bleeding_edge/src/handles.cc Thu Dec 20 08:25:26 2012
@@ -375,6 +375,15 @@
Handle<JSFunction> constructor = isolate->script_function();
Handle<JSValue> result =
Handle<JSValue>::cast(isolate->factory()->NewJSObject(constructor));
+
+ // The allocation might have triggered a GC, which could have called this
+ // function recursively, and a wrapper has already been created and
cached.
+ // In that case, simply return the cached wrapper.
+ if (script->wrapper()->foreign_address() != NULL) {
+ return Handle<JSValue>(
+ reinterpret_cast<JSValue**>(script->wrapper()->foreign_address()));
+ }
+
result->set_value(*script);
// Create a new weak global handle and use it to cache the wrapper
=======================================
--- /branches/bleeding_edge/src/heap.cc Thu Dec 20 01:20:37 2012
+++ /branches/bleeding_edge/src/heap.cc Thu Dec 20 08:25:26 2012
@@ -7293,36 +7293,41 @@
int budget = kBudgetPerGC;
for (int i = 0; i < list_.length(); i++) {
Object* object = list_[i];
- // Skip possible holes in the list.
- if (object->IsTheHole()) continue;
- if (isolate->heap()->InNewSpace(object) || budget == 0) {
- list_[write_index++] = object;
- continue;
+ JSFunction* getter_fun;
+
+ { AssertNoAllocation assert;
+ // Skip possible holes in the list.
+ if (object->IsTheHole()) continue;
+ if (isolate->heap()->InNewSpace(object) || budget == 0) {
+ list_[write_index++] = object;
+ continue;
+ }
+
+ // Check whether the stack property is backed by the original getter.
+ LookupResult lookup(isolate);
+ JSObject::cast(object)->LocalLookupRealNamedProperty(*stack_key,
&lookup);
+ if (!lookup.IsFound() || lookup.type() != CALLBACKS) continue;
+ Object* callback = lookup.GetCallbackObject();
+ if (!callback->IsAccessorPair()) continue;
+ Object* getter_obj = AccessorPair::cast(callback)->getter();
+ if (!getter_obj->IsJSFunction()) continue;
+ getter_fun = JSFunction::cast(getter_obj);
+ String* key = isolate->heap()->hidden_stack_trace_symbol();
+ if (key != getter_fun->GetHiddenProperty(key)) continue;
}
- // Fire the stack property getter, if it is the original marked getter.
- LookupResult lookup(isolate);
- JSObject::cast(object)->LocalLookupRealNamedProperty(*stack_key,
&lookup);
- if (!lookup.IsFound() || lookup.type() != CALLBACKS) continue;
- Object* callback = lookup.GetCallbackObject();
- if (!callback->IsAccessorPair()) continue;
- Object* getter_obj = AccessorPair::cast(callback)->getter();
- if (!getter_obj->IsJSFunction()) continue;
- JSFunction* getter_fun = JSFunction::cast(getter_obj);
- String* key = isolate->heap()->hidden_stack_trace_symbol();
- if (key != getter_fun->GetHiddenProperty(key)) continue;
budget--;
+ HandleScope scope(isolate);
bool has_exception = false;
- Execution::Call(Handle<Object>(getter_fun, isolate),
- Handle<Object>(object, isolate),
- 0,
- NULL,
- &has_exception);
+ Handle<Object> object_handle(object, isolate);
+ Handle<Object> getter_handle(getter_fun, isolate);
+ Execution::Call(getter_handle, object_handle, 0, NULL, &has_exception);
if (has_exception) {
// Hit an exception (most likely a stack overflow).
// Wrap up this pass and retry after another GC.
isolate->clear_pending_exception();
- list_[write_index++] = object;
+ // We use the handle since calling the getter might have caused a GC.
+ list_[write_index++] = *object_handle;
budget = 0;
}
}
=======================================
--- /branches/bleeding_edge/src/messages.js Mon Dec 17 06:00:50 2012
+++ /branches/bleeding_edge/src/messages.js Thu Dec 20 08:25:26 2012
@@ -1080,6 +1080,10 @@
}
return constructorName;
}
+
+
+// Flag to prevent recursive call of Error.prepareStackTrace.
+var formatting_custom_stack_trace = false;
function captureStackTrace(obj, cons_opt) {
@@ -1093,14 +1097,17 @@
stackTraceLimit);
// Don't be lazy if the error stack formatting is custom (observable).
- if (IS_FUNCTION($Error.prepareStackTrace)) {
- var custom_stacktrace_fun = $Error.prepareStackTrace;
- // Use default error formatting for the case that custom formatting
throws.
- $Error.prepareStackTrace = null;
+ if (IS_FUNCTION($Error.prepareStackTrace)
&& !formatting_custom_stack_trace) {
var array = [];
%MoveArrayContents(GetStackFrames(stack), array);
- obj.stack = custom_stacktrace_fun(obj, array);
- $Error.prepareStackTrace = custom_stacktrace_fun;
+ formatting_custom_stack_trace = true;
+ try {
+ obj.stack = $Error.prepareStackTrace(obj, array);
+ } catch (e) {
+ throw e; // The custom formatting function threw. Rethrow.
+ } finally {
+ formatting_custom_stack_trace = false;
+ }
return;
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/stack-traces.js Tue Dec 11
02:14:01 2012
+++ /branches/bleeding_edge/test/mjsunit/stack-traces.js Thu Dec 20
08:25:26 2012
@@ -310,9 +310,9 @@
error.stack;
assertTrue(fired);
-//Check that throwing exception in a custom stack trace formatting function
-//does not lead to recursion.
-Error.prepareStackTrace = function() { throw new Error("abc"); }
+// Check that throwing exception in a custom stack trace formatting
function
+// does not lead to recursion.
+Error.prepareStackTrace = function() { throw new Error("abc"); };
var message;
try {
throw new Error();
@@ -321,3 +321,9 @@
}
assertEquals("abc", message);
+
+// Test that modifying Error.prepareStackTrace by itself works.
+Error.prepareStackTrace = function() { Error.prepareStackTrace = "custom";
};
+new Error();
+
+assertEquals("custom", Error.prepareStackTrace);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
