Changeset 153457 in webkit for trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

Timestamp:

Jul 29, 2013, 9:33:35 PM (12 years ago)

Author:

[email protected]

Message:

Eager stack trace for error objects.
​https://bugs.webkit.org/show_bug.cgi?id=118918

Source/JavaScriptCore:

Patch by Chris Curtis <​[email protected]> on 2013-07-29
Reviewed by Geoffrey Garen.

Chrome and Firefox give error objects the stack property and we wanted to match
that functionality. This allows developers to see the stack without throwing an object.

• runtime/ErrorInstance.cpp:

(JSC::ErrorInstance::finishCreation):

For error objects that are not thrown as an exception, we pass the stackTrace in
as a parameter. This allows the error object to have the stack property.

• interpreter/Interpreter.cpp:

(JSC::stackTraceAsString):
Helper function used to eliminate duplicate code.

(JSC::Interpreter::addStackTraceIfNecessary):
When an error object is created by the user the vm->exceptionStack is not set.
If the user throws this error object later the stack that is in the error object
may not be the correct stack for the throw, so when we set the vm->exception stack,
the stack property on the error object is set as well.

• runtime/ErrorConstructor.cpp:

(JSC::constructWithErrorConstructor):
(JSC::callErrorConstructor):

• runtime/NativeErrorConstructor.cpp:

(JSC::constructWithNativeErrorConstructor):
(JSC::callNativeErrorConstructor):
These functions indicate that the user created an error object. For all error objects
that the user explicitly creates, the topCallFrame is at a new frame created to
handle the user's call. In this case though, the error object needs the caller's
frame to create the stack trace correctly.

• interpreter/Interpreter.h:
• runtime/ErrorInstance.h:

(JSC::ErrorInstance::create):

LayoutTests:

Patch by Chris Curtis <​[email protected]> on 2013-07-29
Reviewed by Geoffrey Garen.

Added tests to ensure that the stack property was present at creation for all
error Objects. This test will fail without this patch.

• fast/js/script-tests/stack-at-creation-for-error-objects.js: Added.

(checkStack):

• fast/js/stack-at-creation-for-error-objects-expected.txt: Added.
• fast/js/stack-at-creation-for-error-objects.html: Added.

• inspector/console/console-format-expected.txt:
• inspector/console/console-format.html:

This test was modified by removing the error object from being evaluated. Prior to this patch
error objects did not have the stack property, so the stack information was not being
displayed. The stack trace includes a file path specific to the machine that is running
the test. The results would have differed from one computer to the next. There
is not an easy way to capture the error object to treat it differently. By removing
the error object there is no need to add extra code to treat it differently.
Also there are other tests inside inspector/console that test the stack trace,
so the testing suite does not lose error testing by removing it.

The .stack property was added to the error objects at creation time.

• fast/js/exception-properties-expected.txt:
• fast/js/script-tests/exception-properties.js:

The column numbers are modified in the following test. When error objects are explicitly
invoked, the column number points to the beginning "(" instead of end ")".
Functionality between browsers do not match either. Firefox does not output column
numbers. Chrome points columns numbers to the beginning of the "new" call.

• fast/js/line-column-numbers-expected.txt:
• fast/js/stack-trace-expected.txt:

File:

• trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -558 +558 @@
     }
 }
-
+JSString* Interpreter:: stackTraceAsString(ExecState* exec, Vector<StackFrame> stackTrace)
+{
+    // FIXME: JSStringJoiner could be more efficient than StringBuilder here.
+    StringBuilder builder;
+    for (unsigned i = 0; i < stackTrace.size(); i++) {
+        builder.append(String(stackTrace[i].toString(exec)));
+        if (i != stackTrace.size() - 1)
+            builder.append('\n');
+    }
+    return jsString(&exec->vm(), builder.toString());
+}
+    
 void Interpreter::addStackTraceIfNecessary(CallFrame* callFrame, JSValue error)
 {
@@ -564 +575 @@
     ASSERT(callFrame == vm->topCallFrame || callFrame == callFrame->lexicalGlobalObject()->globalExec() || callFrame == callFrame->dynamicGlobalObject()->globalExec());
 
-    if (error.isObject()) {
-        if (asObject(error)->hasProperty(callFrame, vm->propertyNames->stack))
+    if (vm->exceptionStack().size()) {
+        if (!error.isObject() || asObject(error)->hasProperty(callFrame, vm->propertyNames->stack))
             return;
     }
@@ -575 +586 @@
         return;
 
-    JSObject* errorObject = asObject(error);
-    JSGlobalObject* globalObject = 0;
-    if (isTerminatedExecutionException(error))
-        globalObject = vm->dynamicGlobalObject;
-    else
-        globalObject = errorObject->globalObject();
-
-    // FIXME: JSStringJoiner could be more efficient than StringBuilder here.
-    StringBuilder builder;
-    for (unsigned i = 0; i < stackTrace.size(); i++) {
-        builder.append(String(stackTrace[i].toString(globalObject->globalExec()).impl()));
-        if (i != stackTrace.size() - 1)
-            builder.append('\n');
-    }
-
-    errorObject->putDirect(*vm, vm->propertyNames->stack, jsString(vm, builder.toString()), ReadOnly | DontDelete);
+    // Note: 'error' might already have a stack property if it was created by the user (e.g. "new Error"). The stack
+    // now, as the error is thrown, might be different from the stack when it was created, so we overwrite it with
+    // the current stack unconditionally.
+    asObject(error)->putDirect(*vm, vm->propertyNames->stack, vm->interpreter->stackTraceAsString(vm->topCallFrame, stackTrace), ReadOnly | DontDelete);
+
 }