Error should compute .stack and friends lazily
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Sep 2017 19:00:03 +0000 (19:00 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Sep 2017 19:00:03 +0000 (19:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176645

Reviewed by Saam Barati.
JSTests:

* ChakraCore.yaml: Skip test that was testing non-standard behavior of these fields.
* microbenchmarks/new-error.js: Added.
* microbenchmarks/throw.js: Added.

Source/JavaScriptCore:

Building the string portion of the stack trace after we walk the stack accounts for most of
the cost of computing the .stack property. So, this patch makes ErrorInstance hold onto the
Vector<StackFrame> so that it can build the string only once it's really needed.

This is an enormous speed-up for programs that allocate and throw exceptions.

It's a 5.6x speed-up for "new Error()" with a stack that is 4 functions deep.

It's a 2.2x speed-up for throwing and catching an Error.

It's a 1.17x speed-up for the WSL test suite (which throws a lot).

It's a significant speed-up on many of our existing try-catch microbenchmarks. For example,
delta-blue-try-catch is 1.16x faster.

* interpreter/Interpreter.cpp:
(JSC::GetStackTraceFunctor::GetStackTraceFunctor):
(JSC::GetStackTraceFunctor::operator() const):
(JSC::Interpreter::getStackTrace):
* interpreter/Interpreter.h:
* runtime/Error.cpp:
(JSC::getStackTrace):
(JSC::getBytecodeOffset):
(JSC::addErrorInfo):
(JSC::addErrorInfoAndGetBytecodeOffset): Deleted.
* runtime/Error.h:
* runtime/ErrorInstance.cpp:
(JSC::ErrorInstance::ErrorInstance):
(JSC::ErrorInstance::finishCreation):
(JSC::ErrorInstance::materializeErrorInfoIfNeeded):
(JSC::ErrorInstance::visitChildren):
(JSC::ErrorInstance::getOwnPropertySlot):
(JSC::ErrorInstance::getOwnNonIndexPropertyNames):
(JSC::ErrorInstance::defineOwnProperty):
(JSC::ErrorInstance::put):
(JSC::ErrorInstance::deleteProperty):
* runtime/ErrorInstance.h:
* runtime/Exception.cpp:
(JSC::Exception::visitChildren):
(JSC::Exception::finishCreation):
* runtime/Exception.h:
* runtime/StackFrame.cpp:
(JSC::StackFrame::visitChildren):
* runtime/StackFrame.h:
(JSC::StackFrame::StackFrame):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221836 268f45cc-cd09-0410-ab3c-d52691b4dbfc

17 files changed:
JSTests/ChakraCore.yaml
JSTests/ChangeLog
JSTests/microbenchmarks/new-error.js [new file with mode: 0644]
JSTests/microbenchmarks/throw.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/inspector/ScriptCallStack.cpp
Source/JavaScriptCore/inspector/ScriptCallStackFactory.cpp
Source/JavaScriptCore/interpreter/Interpreter.cpp
Source/JavaScriptCore/interpreter/Interpreter.h
Source/JavaScriptCore/runtime/Error.cpp
Source/JavaScriptCore/runtime/Error.h
Source/JavaScriptCore/runtime/ErrorInstance.cpp
Source/JavaScriptCore/runtime/ErrorInstance.h
Source/JavaScriptCore/runtime/Exception.cpp
Source/JavaScriptCore/runtime/Exception.h
Source/JavaScriptCore/runtime/StackFrame.cpp
Source/JavaScriptCore/runtime/StackFrame.h

index f0001eb..b248fac 100644 (file)
 - path: ChakraCore/test/StackTrace/ErrorPrototype.js
   cmd: runChakra :baseline, "NoException", "ErrorPrototype.baseline-jsc", ["TrimStackTracePath.js"]
 - path: ChakraCore/test/StackTrace/ErrorDotStackAlreadyExists.js
-  cmd: runChakra :baseline, "NoException", "ErrorDotStackAlreadyExists.baseline-jsc", []
+  # Tests non-standard behavior.
+  cmd: runChakra :skip, "NoException", "ErrorDotStackAlreadyExists.baseline-jsc", []
 - path: ChakraCore/test/StackTrace/FunctionName.js
   cmd: runChakra :baseline, "NoException", "FunctionName.js.baseline-jsc", ["TrimStackTracePath.js"]
 - path: ChakraCore/test/StackTrace/x64StackWalk.js
index 265ba37..69c8f50 100644 (file)
@@ -1,3 +1,14 @@
+2017-09-09  Filip Pizlo  <fpizlo@apple.com>
+
+        Error should compute .stack and friends lazily
+        https://bugs.webkit.org/show_bug.cgi?id=176645
+
+        Reviewed by Saam Barati.
+
+        * ChakraCore.yaml: Skip test that was testing non-standard behavior of these fields.
+        * microbenchmarks/new-error.js: Added.
+        * microbenchmarks/throw.js: Added.
+
 2017-09-09  Mark Lam  <mark.lam@apple.com>
 
         [Re-landing] Use JIT probes for DFG OSR exit.
diff --git a/JSTests/microbenchmarks/new-error.js b/JSTests/microbenchmarks/new-error.js
new file mode 100644 (file)
index 0000000..c2ab76d
--- /dev/null
@@ -0,0 +1,17 @@
+function foo()
+{
+    for (var i = 0; i < 100000; ++i)
+        new Error();
+}
+
+function bar()
+{
+    foo();
+}
+
+function baz()
+{
+    bar();
+}
+
+baz();
diff --git a/JSTests/microbenchmarks/throw.js b/JSTests/microbenchmarks/throw.js
new file mode 100644 (file)
index 0000000..5ce4c5a
--- /dev/null
@@ -0,0 +1,28 @@
+function foo()
+{
+    throw new Error();
+}
+
+function bar()
+{
+    foo();
+}
+
+function baz()
+{
+    bar();
+}
+
+function thingy()
+{
+    try {
+        baz();
+    } catch (e) {
+        if (e.constructor != Error)
+            throw new Error("Bad error: " + e);
+    }
+}
+
+for (var i = 0; i < 10000; ++i)
+    thingy();
+
index 7bd189b..c918c1c 100644 (file)
@@ -1,3 +1,56 @@
+2017-09-08  Filip Pizlo  <fpizlo@apple.com>
+
+        Error should compute .stack and friends lazily
+        https://bugs.webkit.org/show_bug.cgi?id=176645
+
+        Reviewed by Saam Barati.
+        
+        Building the string portion of the stack trace after we walk the stack accounts for most of
+        the cost of computing the .stack property. So, this patch makes ErrorInstance hold onto the
+        Vector<StackFrame> so that it can build the string only once it's really needed.
+        
+        This is an enormous speed-up for programs that allocate and throw exceptions.
+        
+        It's a 5.6x speed-up for "new Error()" with a stack that is 4 functions deep.
+        
+        It's a 2.2x speed-up for throwing and catching an Error.
+        
+        It's a 1.17x speed-up for the WSL test suite (which throws a lot).
+        
+        It's a significant speed-up on many of our existing try-catch microbenchmarks. For example,
+        delta-blue-try-catch is 1.16x faster.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::GetStackTraceFunctor::GetStackTraceFunctor):
+        (JSC::GetStackTraceFunctor::operator() const):
+        (JSC::Interpreter::getStackTrace):
+        * interpreter/Interpreter.h:
+        * runtime/Error.cpp:
+        (JSC::getStackTrace):
+        (JSC::getBytecodeOffset):
+        (JSC::addErrorInfo):
+        (JSC::addErrorInfoAndGetBytecodeOffset): Deleted.
+        * runtime/Error.h:
+        * runtime/ErrorInstance.cpp:
+        (JSC::ErrorInstance::ErrorInstance):
+        (JSC::ErrorInstance::finishCreation):
+        (JSC::ErrorInstance::materializeErrorInfoIfNeeded):
+        (JSC::ErrorInstance::visitChildren):
+        (JSC::ErrorInstance::getOwnPropertySlot):
+        (JSC::ErrorInstance::getOwnNonIndexPropertyNames):
+        (JSC::ErrorInstance::defineOwnProperty):
+        (JSC::ErrorInstance::put):
+        (JSC::ErrorInstance::deleteProperty):
+        * runtime/ErrorInstance.h:
+        * runtime/Exception.cpp:
+        (JSC::Exception::visitChildren):
+        (JSC::Exception::finishCreation):
+        * runtime/Exception.h:
+        * runtime/StackFrame.cpp:
+        (JSC::StackFrame::visitChildren):
+        * runtime/StackFrame.h:
+        (JSC::StackFrame::StackFrame):
+
 2017-09-09  Mark Lam  <mark.lam@apple.com>
 
         [Re-landing] Use JIT probes for DFG OSR exit.
index 612c448..c0a7ae5 100644 (file)
@@ -33,6 +33,7 @@
 #include "ScriptCallStack.h"
 
 #include "InspectorValues.h"
+#include <wtf/DataLog.h>
 
 namespace Inspector {
 
index 2a21a3b..6c77981 100644 (file)
@@ -118,19 +118,36 @@ Ref<ScriptCallStack> createScriptCallStackForConsole(JSC::ExecState* exec, size_
     return ScriptCallStack::create(frames);
 }
 
-static void extractSourceInformationFromException(JSC::ExecState* exec, JSObject* exceptionObject, int* lineNumber, int* columnNumber, String* sourceURL)
+static bool extractSourceInformationFromException(JSC::ExecState* exec, JSObject* exceptionObject, int* lineNumber, int* columnNumber, String* sourceURL)
 {
     VM& vm = exec->vm();
     auto scope = DECLARE_CATCH_SCOPE(vm);
 
     // FIXME: <http://webkit.org/b/115087> Web Inspector: Should not need to evaluate JavaScript handling exceptions
     JSValue lineValue = exceptionObject->getDirect(vm, Identifier::fromString(exec, "line"));
-    *lineNumber = lineValue && lineValue.isNumber() ? int(lineValue.toNumber(exec)) : 0;
     JSValue columnValue = exceptionObject->getDirect(vm, Identifier::fromString(exec, "column"));
-    *columnNumber = columnValue && columnValue.isNumber() ? int(columnValue.toNumber(exec)) : 0;
     JSValue sourceURLValue = exceptionObject->getDirect(vm, Identifier::fromString(exec, "sourceURL"));
-    *sourceURL = sourceURLValue && sourceURLValue.isString() ? sourceURLValue.toWTFString(exec) : ASCIILiteral("undefined");
+    
+    bool result = false;
+    if (lineValue && lineValue.isNumber()
+        && sourceURLValue && sourceURLValue.isString()) {
+        *lineNumber = int(lineValue.toNumber(exec));
+        *columnNumber = columnValue && columnValue.isNumber() ? int(columnValue.toNumber(exec)) : 0;
+        *sourceURL = sourceURLValue.toWTFString(exec);
+        result = true;
+    } else if (ErrorInstance* error = jsDynamicCast<ErrorInstance*>(vm, exceptionObject)) {
+        unsigned unsignedLine;
+        unsigned unsignedColumn;
+        result = getLineColumnAndSource(error->stackTrace(), unsignedLine, unsignedColumn, *sourceURL);
+        *lineNumber = static_cast<int>(unsignedLine);
+        *columnNumber = static_cast<int>(unsignedColumn);
+    }
+    
+    if (sourceURL->isEmpty())
+        *sourceURL = ASCIILiteral("undefined");
+    
     scope.clearException();
+    return result;
 }
 
 Ref<ScriptCallStack> createScriptCallStackFromException(JSC::ExecState* exec, JSC::Exception* exception, size_t maxStackSize)
@@ -154,13 +171,18 @@ Ref<ScriptCallStack> createScriptCallStackFromException(JSC::ExecState* exec, JS
         int columnNumber;
         String exceptionSourceURL;
         if (!frames.size()) {
-            extractSourceInformationFromException(exec, exceptionObject, &lineNumber, &columnNumber, &exceptionSourceURL);
-            frames.append(ScriptCallFrame(String(), exceptionSourceURL, noSourceID, lineNumber, columnNumber));
+            if (extractSourceInformationFromException(exec, exceptionObject, &lineNumber, &columnNumber, &exceptionSourceURL))
+                frames.append(ScriptCallFrame(String(), exceptionSourceURL, noSourceID, lineNumber, columnNumber));
         } else {
+            // FIXME: The typical stack trace will have a native frame at the top, and consumers of
+            // this code already know this (see JSDOMExceptionHandling.cpp's reportException, for
+            // example - it uses firstNonNativeCallFrame). This looks like it splats something else
+            // over it. That something else is probably already at stackTrace[1].
+            // https://bugs.webkit.org/show_bug.cgi?id=176663
             if (!stackTrace[0].hasLineAndColumnInfo() || stackTrace[0].sourceURL().isEmpty()) {
                 const ScriptCallFrame& firstCallFrame = frames.first();
-                extractSourceInformationFromException(exec, exceptionObject, &lineNumber, &columnNumber, &exceptionSourceURL);
-                frames[0] = ScriptCallFrame(firstCallFrame.functionName(), exceptionSourceURL, stackTrace[0].sourceID(), lineNumber, columnNumber);
+                if (extractSourceInformationFromException(exec, exceptionObject, &lineNumber, &columnNumber, &exceptionSourceURL))
+                    frames[0] = ScriptCallFrame(firstCallFrame.functionName(), exceptionSourceURL, stackTrace[0].sourceID(), lineNumber, columnNumber);
             }
         }
     }
index 348a876..9f92db3 100644 (file)
@@ -479,8 +479,9 @@ bool Interpreter::isOpcode(Opcode opcode)
 
 class GetStackTraceFunctor {
 public:
-    GetStackTraceFunctor(VM& vm, Vector<StackFrame>& results, size_t framesToSkip, size_t capacity)
+    GetStackTraceFunctor(VM& vm, JSCell* owner, Vector<StackFrame>& results, size_t framesToSkip, size_t capacity)
         : m_vm(vm)
+        , m_owner(owner)
         , m_results(results)
         , m_framesToSkip(framesToSkip)
         , m_remainingCapacityForFrameCapture(capacity)
@@ -500,10 +501,10 @@ public:
                 m_results.append(StackFrame::wasm(visitor->wasmFunctionIndexOrName()));
             } else if (!!visitor->codeBlock() && !visitor->codeBlock()->unlinkedCodeBlock()->isBuiltinFunction()) {
                 m_results.append(
-                    StackFrame(m_vm, visitor->callee().asCell(), visitor->codeBlock(), visitor->bytecodeOffset()));
+                    StackFrame(m_vm, m_owner, visitor->callee().asCell(), visitor->codeBlock(), visitor->bytecodeOffset()));
             } else {
                 m_results.append(
-                    StackFrame(m_vm, visitor->callee().asCell()));
+                    StackFrame(m_vm, m_owner, visitor->callee().asCell()));
             }
     
             m_remainingCapacityForFrameCapture--;
@@ -514,13 +515,15 @@ public:
 
 private:
     VM& m_vm;
+    JSCell* m_owner;
     Vector<StackFrame>& m_results;
     mutable size_t m_framesToSkip;
     mutable size_t m_remainingCapacityForFrameCapture;
 };
 
-void Interpreter::getStackTrace(Vector<StackFrame>& results, size_t framesToSkip, size_t maxStackSize)
+void Interpreter::getStackTrace(JSCell* owner, Vector<StackFrame>& results, size_t framesToSkip, size_t maxStackSize)
 {
+    DisallowGC disallowGC;
     VM& vm = m_vm;
     CallFrame* callFrame = vm.topCallFrame;
     if (!callFrame)
@@ -537,7 +540,7 @@ void Interpreter::getStackTrace(Vector<StackFrame>& results, size_t framesToSkip
     framesCount -= framesToSkip;
     framesCount = std::min(maxStackSize, framesCount);
 
-    GetStackTraceFunctor functor(vm, results, framesToSkip, framesCount);
+    GetStackTraceFunctor functor(vm, owner, results, framesToSkip, framesCount);
     StackVisitor::visit(callFrame, &vm, functor);
     ASSERT(results.size() == results.capacity());
 }
index 7ab3656..c3f7d81 100644 (file)
@@ -129,7 +129,7 @@ namespace JSC {
 
         JS_EXPORT_PRIVATE void dumpCallFrame(CallFrame*);
 
-        void getStackTrace(Vector<StackFrame>& results, size_t framesToSkip = 0, size_t maxStackSize = std::numeric_limits<size_t>::max());
+        void getStackTrace(JSCell* owner, Vector<StackFrame>& results, size_t framesToSkip = 0, size_t maxStackSize = std::numeric_limits<size_t>::max());
 
     private:
         enum ExecutionFlag { Normal, InitializeAndReturn };
index 18801ed..caa3305 100644 (file)
 #include "NativeErrorConstructor.h"
 #include "SourceCode.h"
 #include "StackFrame.h"
+#include "SuperSampler.h"
 
 namespace JSC {
 
-static const char* linePropertyName = "line";
-static const char* sourceURLPropertyName = "sourceURL";
-
 JSObject* createError(ExecState* exec, const String& message, ErrorInstance::SourceAppender appender)
 {
     ASSERT(!message.isEmpty());
@@ -160,48 +158,69 @@ private:
     mutable unsigned m_index;
 };
 
-bool addErrorInfoAndGetBytecodeOffset(ExecState* exec, VM& vm, JSObject* obj, bool useCurrentFrame, CallFrame*& callFrame, unsigned* bytecodeOffset)
+std::unique_ptr<Vector<StackFrame>> getStackTrace(ExecState* exec, VM& vm, JSObject* obj, bool useCurrentFrame)
 {
     JSGlobalObject* globalObject = obj->globalObject();
     ErrorConstructor* errorConstructor = globalObject->errorConstructor();
     if (!errorConstructor->stackTraceLimit())
-        return false;
+        return nullptr;
 
-    Vector<StackFrame> stackTrace = Vector<StackFrame>();
     size_t framesToSkip = useCurrentFrame ? 0 : 1;
-    vm.interpreter->getStackTrace(stackTrace, framesToSkip, errorConstructor->stackTraceLimit().value());
-    if (!stackTrace.isEmpty()) {
+    std::unique_ptr<Vector<StackFrame>> stackTrace = std::make_unique<Vector<StackFrame>>();
+    vm.interpreter->getStackTrace(obj, *stackTrace, framesToSkip, errorConstructor->stackTraceLimit().value());
+    if (!stackTrace->isEmpty())
+        ASSERT_UNUSED(exec, exec == vm.topCallFrame || exec == exec->lexicalGlobalObject()->globalExec() || exec == exec->vmEntryGlobalObject()->globalExec());
+    return stackTrace;
+}
 
-        ASSERT(exec == vm.topCallFrame || exec == exec->lexicalGlobalObject()->globalExec() || exec == exec->vmEntryGlobalObject()->globalExec());
+void getBytecodeOffset(ExecState* exec, VM& vm, Vector<StackFrame>* stackTrace, CallFrame*& callFrame, unsigned& bytecodeOffset)
+{
+    FindFirstCallerFrameWithCodeblockFunctor functor(exec);
+    StackVisitor::visit(vm.topCallFrame, &vm, functor);
+    callFrame = functor.foundCallFrame();
+    unsigned stackIndex = functor.index();
+    bytecodeOffset = 0;
+    if (stackTrace && stackIndex < stackTrace->size() && stackTrace->at(stackIndex).hasBytecodeOffset())
+        bytecodeOffset = stackTrace->at(stackIndex).bytecodeOffset();
+}
 
-        StackFrame* firstFrameWithLineAndColumnInfo = nullptr;
-        for (unsigned i = 0 ; i < stackTrace.size(); ++i) {
-            firstFrameWithLineAndColumnInfo = &stackTrace.at(i);
-            if (firstFrameWithLineAndColumnInfo->hasLineAndColumnInfo())
-                break;
+bool getLineColumnAndSource(Vector<StackFrame>* stackTrace, unsigned& line, unsigned& column, String& sourceURL)
+{
+    line = 0;
+    column = 0;
+    sourceURL = String();
+    
+    if (!stackTrace)
+        return false;
+    
+    for (unsigned i = 0 ; i < stackTrace->size(); ++i) {
+        StackFrame& frame = stackTrace->at(i);
+        if (frame.hasLineAndColumnInfo()) {
+            frame.computeLineAndColumn(line, column);
+            sourceURL = frame.sourceURL();
+            return true;
         }
+    }
+    
+    return false;
+}
 
-        if (bytecodeOffset) {
-            FindFirstCallerFrameWithCodeblockFunctor functor(exec);
-            StackVisitor::visit(vm.topCallFrame, &vm, functor);
-            callFrame = functor.foundCallFrame();
-            unsigned stackIndex = functor.index();
-            *bytecodeOffset = 0;
-            if (stackIndex < stackTrace.size() && stackTrace.at(stackIndex).hasBytecodeOffset())
-                *bytecodeOffset = stackTrace.at(stackIndex).bytecodeOffset();
-        }
-        
+bool addErrorInfo(VM& vm, Vector<StackFrame>* stackTrace, JSObject* obj)
+{
+    if (!stackTrace)
+        return false;
+    
+    if (!stackTrace->isEmpty()) {
         unsigned line;
         unsigned column;
-        firstFrameWithLineAndColumnInfo->computeLineAndColumn(line, column);
+        String sourceURL;
+        getLineColumnAndSource(stackTrace, line, column, sourceURL);
         obj->putDirect(vm, vm.propertyNames->line, jsNumber(line));
         obj->putDirect(vm, vm.propertyNames->column, jsNumber(column));
+        if (!sourceURL.isEmpty())
+            obj->putDirect(vm, vm.propertyNames->sourceURL, jsString(&vm, sourceURL));
 
-        String frameSourceURL = firstFrameWithLineAndColumnInfo->sourceURL();
-        if (!frameSourceURL.isEmpty())
-            obj->putDirect(vm, vm.propertyNames->sourceURL, jsString(&vm, frameSourceURL));
-
-        obj->putDirect(vm, vm.propertyNames->stack, Interpreter::stackTraceAsString(vm, stackTrace), DontEnum);
+        obj->putDirect(vm, vm.propertyNames->stack, Interpreter::stackTraceAsString(vm, *stackTrace), DontEnum);
 
         return true;
     }
@@ -212,19 +231,33 @@ bool addErrorInfoAndGetBytecodeOffset(ExecState* exec, VM& vm, JSObject* obj, bo
 
 void addErrorInfo(ExecState* exec, JSObject* obj, bool useCurrentFrame)
 {
-    CallFrame* callFrame = nullptr;
-    addErrorInfoAndGetBytecodeOffset(exec, exec->vm(), obj, useCurrentFrame, callFrame);
+    VM& vm = exec->vm();
+    std::unique_ptr<Vector<StackFrame>> stackTrace = getStackTrace(exec, vm, obj, useCurrentFrame);
+    addErrorInfo(vm, stackTrace.get(), obj);
 }
 
 JSObject* addErrorInfo(CallFrame* callFrame, JSObject* error, int line, const SourceCode& source)
 {
-    VM* vm = &callFrame->vm();
+    VM& vm = callFrame->vm();
     const String& sourceURL = source.provider()->url();
-
+    
+    // The putDirect() calls below should really be put() so that they trigger materialization of
+    // the line/sourceURL properties. Otherwise, what we set here will just be overwritten later.
+    // But calling put() would be bad because we'd rather not do effectful things here. Luckily, we
+    // know that this will get called on some kind of error - so we can just directly ask the
+    // ErrorInstance to materialize whatever it needs to. There's a chance that we get passed some
+    // other kind of object, which also has materializable properties. But this code is heuristic-ey
+    // enough that if we're wrong in such corner cases, it's not the end of the world.
+    if (ErrorInstance* errorInstance = jsDynamicCast<ErrorInstance*>(vm, error))
+        errorInstance->materializeErrorInfoIfNeeded(vm);
+    
+    // FIXME: This does not modify the column property, which confusingly continues to reflect
+    // the column at which the exception was thrown.
+    // https://bugs.webkit.org/show_bug.cgi?id=176673
     if (line != -1)
-        error->putDirect(*vm, Identifier::fromString(vm, linePropertyName), jsNumber(line));
+        error->putDirect(vm, vm.propertyNames->line, jsNumber(line));
     if (!sourceURL.isNull())
-        error->putDirect(*vm, Identifier::fromString(vm, sourceURLPropertyName), jsString(vm, sourceURL));
+        error->putDirect(vm, vm.propertyNames->sourceURL, jsString(&vm, sourceURL));
     return error;
 }
 
index 8adc299..90671d5 100644 (file)
@@ -74,10 +74,11 @@ JS_EXPORT_PRIVATE JSObject* createOutOfMemoryError(ExecState*);
 
 JS_EXPORT_PRIVATE JSObject* createError(ExecState*, ErrorType, const String&);
 
-
-bool addErrorInfoAndGetBytecodeOffset(ExecState*, VM&, JSObject*, bool, CallFrame*&, unsigned* = nullptr);
-
-JS_EXPORT_PRIVATE void addErrorInfo(ExecState*, JSObject*, bool); 
+std::unique_ptr<Vector<StackFrame>> getStackTrace(ExecState*, VM&, JSObject*, bool useCurrentFrame);
+void getBytecodeOffset(ExecState*, VM&, Vector<StackFrame>*, CallFrame*&, unsigned& bytecodeOffset);
+bool getLineColumnAndSource(Vector<StackFrame>* stackTrace, unsigned& line, unsigned& column, String& sourceURL);
+bool addErrorInfo(VM&, Vector<StackFrame>*, JSObject*);
+JS_EXPORT_PRIVATE void addErrorInfo(ExecState*, JSObject*, bool);
 JSObject* addErrorInfo(ExecState*, JSObject* error, int line, const SourceCode&);
 
 // Methods to throw Errors.
index fe07eae..5493f8f 100644 (file)
 #include "JSScope.h"
 #include "JSCInlines.h"
 #include "ParseInt.h"
+#include "StackFrame.h"
 #include <wtf/text/StringBuilder.h>
 
 namespace JSC {
 
-STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(ErrorInstance);
-
 const ClassInfo ErrorInstance::s_info = { "Error", &JSNonFinalObject::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(ErrorInstance) };
 
 ErrorInstance::ErrorInstance(VM& vm, Structure* structure)
-    : JSNonFinalObject(vm, structure)
+    : Base(vm, structure)
 {
 }
 
@@ -151,11 +150,11 @@ void ErrorInstance::finishCreation(ExecState* exec, VM& vm, const String& messag
     if (!message.isNull())
         putDirect(vm, vm.propertyNames->message, jsString(&vm, message), DontEnum);
 
-    unsigned bytecodeOffset = 0;
-    CallFrame* callFrame = nullptr;
-    bool hasTrace = addErrorInfoAndGetBytecodeOffset(exec, vm, this, useCurrentFrame, callFrame, hasSourceAppender() ? &bytecodeOffset : nullptr);
-
-    if (hasTrace && callFrame && hasSourceAppender()) {
+    m_stackTrace = getStackTrace(exec, vm, this, useCurrentFrame);
+    if (m_stackTrace && !m_stackTrace->isEmpty() && hasSourceAppender()) {
+        unsigned bytecodeOffset;
+        CallFrame* callFrame;
+        getBytecodeOffset(exec, vm, m_stackTrace.get(), callFrame, bytecodeOffset);
         if (callFrame && callFrame->codeBlock()) {
             ASSERT(!callFrame->callee().isWasm());
             appendSourceToError(callFrame, this, bytecodeOffset);
@@ -227,4 +226,84 @@ String ErrorInstance::sanitizedToString(ExecState* exec)
     return builder.toString();
 }
 
+void ErrorInstance::materializeErrorInfoIfNeeded(VM& vm)
+{
+    if (m_errorInfoMaterialized)
+        return;
+    
+    addErrorInfo(vm, m_stackTrace.get(), this);
+    m_stackTrace = nullptr;
+    
+    m_errorInfoMaterialized = true;
+}
+
+void ErrorInstance::materializeErrorInfoIfNeeded(VM& vm, PropertyName propertyName)
+{
+    if (propertyName == vm.propertyNames->line
+        || propertyName == vm.propertyNames->column
+        || propertyName == vm.propertyNames->sourceURL
+        || propertyName == vm.propertyNames->stack)
+        materializeErrorInfoIfNeeded(vm);
+}
+
+void ErrorInstance::visitChildren(JSCell* cell, SlotVisitor& visitor)
+{
+    ErrorInstance* thisObject = jsCast<ErrorInstance*>(cell);
+    ASSERT_GC_OBJECT_INHERITS(thisObject, info());
+    Base::visitChildren(thisObject, visitor);
+
+    if (thisObject->m_stackTrace) {
+        for (StackFrame& frame : *thisObject->m_stackTrace)
+            frame.visitChildren(visitor);
+    }
+}
+
+bool ErrorInstance::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
+{
+    VM& vm = exec->vm();
+    ErrorInstance* thisObject = jsCast<ErrorInstance*>(object);
+    thisObject->materializeErrorInfoIfNeeded(vm, propertyName);
+    return Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
+}
+
+void ErrorInstance::getOwnNonIndexPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNameArray, EnumerationMode enumerationMode)
+{
+    VM& vm = exec->vm();
+    ErrorInstance* thisObject = jsCast<ErrorInstance*>(object);
+    thisObject->materializeErrorInfoIfNeeded(vm);
+    Base::getOwnNonIndexPropertyNames(thisObject, exec, propertyNameArray, enumerationMode);
+}
+
+void ErrorInstance::getStructurePropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNameArray, EnumerationMode enumerationMode)
+{
+    VM& vm = exec->vm();
+    ErrorInstance* thisObject = jsCast<ErrorInstance*>(object);
+    thisObject->materializeErrorInfoIfNeeded(vm);
+    Base::getStructurePropertyNames(thisObject, exec, propertyNameArray, enumerationMode);
+}
+
+bool ErrorInstance::defineOwnProperty(JSObject* object, ExecState* exec, PropertyName propertyName, const PropertyDescriptor& descriptor, bool shouldThrow)
+{
+    VM& vm = exec->vm();
+    ErrorInstance* thisObject = jsCast<ErrorInstance*>(object);
+    thisObject->materializeErrorInfoIfNeeded(vm, propertyName);
+    return Base::defineOwnProperty(thisObject, exec, propertyName, descriptor, shouldThrow);
+}
+
+bool ErrorInstance::put(JSCell* cell, ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
+{
+    VM& vm = exec->vm();
+    ErrorInstance* thisObject = jsCast<ErrorInstance*>(cell);
+    thisObject->materializeErrorInfoIfNeeded(vm, propertyName);
+    return Base::put(thisObject, exec, propertyName, value, slot);
+}
+
+bool ErrorInstance::deleteProperty(JSCell* cell, ExecState* exec, PropertyName propertyName)
+{
+    VM& vm = exec->vm();
+    ErrorInstance* thisObject = jsCast<ErrorInstance*>(cell);
+    thisObject->materializeErrorInfoIfNeeded(vm, propertyName);
+    return Base::deleteProperty(thisObject, exec, propertyName);
+}
+
 } // namespace JSC
index 56b0f36..7c2b6cc 100644 (file)
@@ -1,6 +1,6 @@
 /*
  *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
- *  Copyright (C) 2008, 2016 Apple Inc. All rights reserved.
+ *  Copyright (C) 2008-2017 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
 
 #pragma once
 
-#include "JSObject.h"
+#include "JSDestructibleObject.h"
 #include "RuntimeType.h"
+#include "StackFrame.h"
 
 namespace JSC {
 
-class ErrorInstance : public JSNonFinalObject {
+class ErrorInstance : public JSDestructibleObject {
 public:
-    typedef JSNonFinalObject Base;
+    typedef JSDestructibleObject Base;
+    const static unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | OverridesGetPropertyNames;
 
     enum SourceTextWhereErrorOccurred { FoundExactSource, FoundApproximateSource };
     typedef String (*SourceAppender) (const String& originalMessage, const String& sourceText, RuntimeType, SourceTextWhereErrorOccurred);
@@ -50,8 +52,6 @@ public:
 
     static ErrorInstance* create(ExecState*, Structure*, JSValue message, SourceAppender = nullptr, RuntimeType = TypeNothing, bool useCurrentFrame = true);
 
-    static void addErrorInfo(ExecState*, VM&, JSObject*, bool = true);
-
     bool hasSourceAppender() const { return !!m_sourceAppender; }
     SourceAppender sourceAppender() const { return m_sourceAppender; }
     void setSourceAppender(SourceAppender appender) { m_sourceAppender = appender; }
@@ -66,16 +66,32 @@ public:
     bool isOutOfMemoryError() const { return m_outOfMemoryError; }
 
     JS_EXPORT_PRIVATE String sanitizedToString(ExecState*);
+    
+    Vector<StackFrame>* stackTrace() { return m_stackTrace.get(); }
+
+    void materializeErrorInfoIfNeeded(VM&);
+    void materializeErrorInfoIfNeeded(VM&, PropertyName);
 
 protected:
     explicit ErrorInstance(VM&, Structure*);
 
     void finishCreation(ExecState*, VM&, const String&, bool useCurrentFrame = true);
 
+    static void visitChildren(JSCell*, SlotVisitor&);
+
+    static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
+    static void getOwnNonIndexPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
+    static void getStructurePropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
+    static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
+    static bool put(JSCell*, ExecState*, PropertyName, JSValue, PutPropertySlot&);
+    static bool deleteProperty(JSCell*, ExecState*, PropertyName);
+    
     SourceAppender m_sourceAppender { nullptr };
     RuntimeType m_runtimeTypeForCause { TypeNothing };
     bool m_stackOverflowError { false };
     bool m_outOfMemoryError { false };
+    bool m_errorInfoMaterialized { false };
+    std::unique_ptr<Vector<StackFrame>> m_stackTrace;
 };
 
 } // namespace JSC
index a9b2d0b..bc989ef 100644 (file)
@@ -58,6 +58,8 @@ void Exception::visitChildren(JSCell* cell, SlotVisitor& visitor)
     Base::visitChildren(thisObject, visitor);
 
     visitor.append(thisObject->m_value);
+    for (StackFrame& frame : thisObject->m_stack)
+        frame.visitChildren(visitor);
 }
 
 Exception::Exception(VM& vm)
@@ -77,7 +79,7 @@ void Exception::finishCreation(VM& vm, JSValue thrownValue, StackCaptureAction a
 
     Vector<StackFrame> stackTrace;
     if (action == StackCaptureAction::CaptureStack)
-        vm.interpreter->getStackTrace(stackTrace, 0, Options::exceptionStackTraceLimit());
+        vm.interpreter->getStackTrace(this, stackTrace, 0, Options::exceptionStackTraceLimit());
     m_stack = WTFMove(stackTrace);
 }
 
index 55b7957..bc0ca3e 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 
 #pragma once
 
-#include "JSObject.h"
+#include "JSDestructibleObject.h"
 #include "StackFrame.h"
 #include <wtf/Vector.h>
 
 namespace JSC {
     
-class Exception : public JSNonFinalObject {
+class Exception : public JSDestructibleObject {
 public:
-    typedef JSNonFinalObject Base;
+    typedef JSDestructibleObject Base;
     static const unsigned StructureFlags = StructureIsImmortal | Base::StructureFlags;
 
     enum StackCaptureAction {
index 836ea0c..8ec875d 100644 (file)
 
 namespace JSC {
 
+StackFrame::StackFrame(VM& vm, JSCell* owner, JSCell* callee)
+    : m_callee(vm, owner, callee)
+    , m_bytecodeOffset(UINT_MAX)
+{
+}
+
+StackFrame::StackFrame(VM& vm, JSCell* owner, JSCell* callee, CodeBlock* codeBlock, unsigned bytecodeOffset)
+    : m_callee(vm, owner, callee)
+    , m_codeBlock(vm, owner, codeBlock)
+    , m_bytecodeOffset(bytecodeOffset)
+{
+}
+
 intptr_t StackFrame::sourceID() const
 {
     if (!m_codeBlock)
@@ -127,5 +140,16 @@ String StackFrame::toString(VM& vm) const
     return traceBuild.toString().impl();
 }
 
+void StackFrame::visitChildren(SlotVisitor& visitor)
+{
+    // FIXME: We should do something here about Wasm::IndexOrName.
+    // https://bugs.webkit.org/show_bug.cgi?id=176644
+    
+    if (m_callee)
+        visitor.append(m_callee);
+    if (m_codeBlock)
+        visitor.append(m_codeBlock);
+}
+
 } // namespace JSC
 
index 4890878..1b5bea6 100644 (file)
 
 #pragma once
 
-#include "Strong.h"
 #include "WasmIndexOrName.h"
+#include "WriteBarrier.h"
 #include <limits.h>
 
 namespace JSC {
 
 class CodeBlock;
 class JSObject;
+class SlotVisitor;
 
 class StackFrame {
 public:
@@ -40,16 +41,9 @@ public:
         : m_bytecodeOffset(UINT_MAX)
     { }
 
-    StackFrame(VM& vm, JSCell* callee)
-        : m_callee(vm, callee)
-        , m_bytecodeOffset(UINT_MAX)
-    { }
+    StackFrame(VM&, JSCell* owner, JSCell* callee);
 
-    StackFrame(VM& vm, JSCell* callee, CodeBlock* codeBlock, unsigned bytecodeOffset)
-        : m_callee(vm, callee)
-        , m_codeBlock(vm, codeBlock)
-        , m_bytecodeOffset(bytecodeOffset)
-    { }
+    StackFrame(VM&, JSCell* owner, JSCell* callee, CodeBlock*, unsigned bytecodeOffset);
 
     static StackFrame wasm(Wasm::IndexOrName indexOrName)
     {
@@ -73,11 +67,12 @@ public:
         ASSERT(hasBytecodeOffset());
         return m_bytecodeOffset;
     }
-
+    
+    void visitChildren(SlotVisitor&);
 
 private:
-    Strong<JSCell> m_callee { };
-    Strong<CodeBlock> m_codeBlock { };
+    WriteBarrier<JSCell> m_callee { };
+    WriteBarrier<CodeBlock> m_codeBlock { };
     union {
         unsigned m_bytecodeOffset;
         Wasm::IndexOrName m_wasmFunctionIndexOrName;