TypeProfileLog::processLogEntries should stash away any pending exceptions and re...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 04:57:33 +0000 (04:57 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 04:57:33 +0000 (04:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191600

Reviewed by Mark Lam.

JSTests:

* stress/type-profiler-log-should-defer-pending-exceptions.js: Added.
(foo):
(test):
(bar):

Source/JavaScriptCore:

processLogEntries will call into calculatedClassName, which will clear
any exceptions it encounters (it assumes that they're stack overflow exceptions).
However, this code may be called when an exception is already pending on the
VM (e.g, when we throw an exception in the DFG, we compile an OSR exit
offramp, which may compile a baseline codeblock, which will process
the type profiler log). To get around this, processLogEntires should stash
away and re-apply any pending exceptions.

* dfg/DFGDriver.cpp:
(JSC::DFG::compileImpl):
* dfg/DFGOperations.cpp:
* inspector/agents/InspectorRuntimeAgent.cpp:
(Inspector::InspectorRuntimeAgent::getRuntimeTypesForVariablesAtOffsets):
* jit/JIT.cpp:
(JSC::JIT::doMainThreadPreparationBeforeCompile):
* jit/JITOperations.cpp:
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):
* runtime/TypeProfilerLog.cpp:
(JSC::TypeProfilerLog::processLogEntries):
* runtime/TypeProfilerLog.h:
* runtime/VM.cpp:
(JSC::VM::dumpTypeProfilerData):
* runtime/VM.h:
(JSC::VM::DeferExceptionScope::DeferExceptionScope):
* tools/JSDollarVM.cpp:
(JSC::functionFindTypeForExpression):
(JSC::functionReturnTypeFor):

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

14 files changed:
JSTests/ChangeLog
JSTests/stress/type-profiler-log-should-defer-pending-exceptions.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGDriver.cpp
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp
Source/JavaScriptCore/jit/JIT.cpp
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
Source/JavaScriptCore/runtime/TypeProfilerLog.cpp
Source/JavaScriptCore/runtime/TypeProfilerLog.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/JavaScriptCore/tools/JSDollarVM.cpp

index b80b87d..768c100 100644 (file)
@@ -1,3 +1,15 @@
+2018-11-13  Saam Barati  <sbarati@apple.com>
+
+        TypeProfileLog::processLogEntries should stash away any pending exceptions and re-apply them to the VM
+        https://bugs.webkit.org/show_bug.cgi?id=191600
+
+        Reviewed by Mark Lam.
+
+        * stress/type-profiler-log-should-defer-pending-exceptions.js: Added.
+        (foo):
+        (test):
+        (bar):
+
 2018-11-13  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r238132.
diff --git a/JSTests/stress/type-profiler-log-should-defer-pending-exceptions.js b/JSTests/stress/type-profiler-log-should-defer-pending-exceptions.js
new file mode 100644 (file)
index 0000000..0764036
--- /dev/null
@@ -0,0 +1,26 @@
+//@ runDefault("--thresholdForOptimizeAfterWarmUp=0", "--useTypeProfiler=true")
+
+function foo() {
+    try {
+        throw 42;
+    } catch(e) {
+        if (e !== 42)
+            throw new Error("Bad!")
+    }
+}
+
+function test(_a) {
+    if (_a === 0) {
+        foo();
+    } 
+}
+
+function bar() {
+    test(Intl.NumberFormat());
+    test(Intl.NumberFormat());
+    test(0);
+}
+
+for (let i=0; i<200; i++) {
+    bar()
+}
index ffbb099..6c51b9b 100644 (file)
@@ -1,3 +1,39 @@
+2018-11-13  Saam Barati  <sbarati@apple.com>
+
+        TypeProfileLog::processLogEntries should stash away any pending exceptions and re-apply them to the VM
+        https://bugs.webkit.org/show_bug.cgi?id=191600
+
+        Reviewed by Mark Lam.
+
+        processLogEntries will call into calculatedClassName, which will clear
+        any exceptions it encounters (it assumes that they're stack overflow exceptions).
+        However, this code may be called when an exception is already pending on the 
+        VM (e.g, when we throw an exception in the DFG, we compile an OSR exit
+        offramp, which may compile a baseline codeblock, which will process
+        the type profiler log). To get around this, processLogEntires should stash
+        away and re-apply any pending exceptions.
+
+        * dfg/DFGDriver.cpp:
+        (JSC::DFG::compileImpl):
+        * dfg/DFGOperations.cpp:
+        * inspector/agents/InspectorRuntimeAgent.cpp:
+        (Inspector::InspectorRuntimeAgent::getRuntimeTypesForVariablesAtOffsets):
+        * jit/JIT.cpp:
+        (JSC::JIT::doMainThreadPreparationBeforeCompile):
+        * jit/JITOperations.cpp:
+        * runtime/CommonSlowPaths.cpp:
+        (JSC::SLOW_PATH_DECL):
+        * runtime/TypeProfilerLog.cpp:
+        (JSC::TypeProfilerLog::processLogEntries):
+        * runtime/TypeProfilerLog.h:
+        * runtime/VM.cpp:
+        (JSC::VM::dumpTypeProfilerData):
+        * runtime/VM.h:
+        (JSC::VM::DeferExceptionScope::DeferExceptionScope):
+        * tools/JSDollarVM.cpp:
+        (JSC::functionFindTypeForExpression):
+        (JSC::functionReturnTypeFor):
+
 2018-11-13  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r238132.
index e2a3457..b56cf6d 100644 (file)
@@ -97,7 +97,7 @@ static CompilationResult compileImpl(
     vm.getCTIStub(linkPolymorphicCallThunkGenerator);
     
     if (vm.typeProfiler())
-        vm.typeProfilerLog()->processLogEntries("Preparing for DFG compilation."_s);
+        vm.typeProfilerLog()->processLogEntries(vm, "Preparing for DFG compilation."_s);
     
     Ref<Plan> plan = adoptRef(
         *new Plan(codeBlock, profiledDFGCodeBlock, mode, osrEntryBytecodeIndex, mustHandleValues));
index df3fd1e..b123e80 100644 (file)
@@ -2705,7 +2705,7 @@ void JIT_OPERATION operationProcessTypeProfilerLogDFG(ExecState* exec)
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
 
-    vm.typeProfilerLog()->processLogEntries("Log Full, called from inside DFG."_s);
+    vm.typeProfilerLog()->processLogEntries(vm, "Log Full, called from inside DFG."_s);
 }
 
 EncodedJSValue JIT_OPERATION operationResolveScopeForHoistingFuncDeclInEval(ExecState* exec, JSScope* scope, UniquedStringImpl* impl)
index 9f730de..e869a61 100644 (file)
@@ -273,7 +273,7 @@ void InspectorRuntimeAgent::getRuntimeTypesForVariablesAtOffsets(ErrorString& er
     }
 
     MonotonicTime start = MonotonicTime::now();
-    m_vm.typeProfilerLog()->processLogEntries("User Query"_s);
+    m_vm.typeProfilerLog()->processLogEntries(m_vm, "User Query"_s);
 
     for (size_t i = 0; i < locations.length(); i++) {
         RefPtr<JSON::Value> value = locations.get(i);
index 30e99f0..21c9d29 100644 (file)
@@ -948,7 +948,7 @@ void JIT::doMainThreadPreparationBeforeCompile()
 {
     // This ensures that we have the most up to date type information when performing typecheck optimizations for op_profile_type.
     if (m_vm->typeProfiler())
-        m_vm->typeProfilerLog()->processLogEntries("Preparing for JIT compilation."_s);
+        m_vm->typeProfilerLog()->processLogEntries(*m_vm, "Preparing for JIT compilation."_s);
 }
 
 unsigned JIT::frameRegisterCountFor(CodeBlock* codeBlock)
index 1e8fe00..0c67ed4 100644 (file)
@@ -2874,7 +2874,7 @@ void JIT_OPERATION operationProcessTypeProfilerLog(ExecState* exec)
 {
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
-    vm.typeProfilerLog()->processLogEntries("Log Full, called from inside baseline JIT"_s);
+    vm.typeProfilerLog()->processLogEntries(vm, "Log Full, called from inside baseline JIT"_s);
 }
 
 void JIT_OPERATION operationProcessShadowChickenLog(ExecState* exec)
index 5163c33..8bc91b3 100644 (file)
@@ -963,7 +963,7 @@ SLOW_PATH_DECL(slow_path_to_index_string)
 SLOW_PATH_DECL(slow_path_profile_type_clear_log)
 {
     BEGIN();
-    vm.typeProfilerLog()->processLogEntries("LLInt log full."_s);
+    vm.typeProfilerLog()->processLogEntries(vm, "LLInt log full."_s);
     END();
 }
 
index 79c842e..9153ea6 100644 (file)
@@ -55,8 +55,15 @@ TypeProfilerLog::~TypeProfilerLog()
     delete[] m_logStartPtr;
 }
 
-void TypeProfilerLog::processLogEntries(const String& reason)
+void TypeProfilerLog::processLogEntries(VM& vm, const String& reason)
 {
+    // We need to do this because this code will call into calculatedDisplayName.
+    // calculatedDisplayName will clear any exception it sees (because it thinks
+    // it's a stack overflow). We may be called when an exception was already
+    // thrown, so we don't want calcualtedDisplayName to clear that exception that
+    // was thrown before we even got here.
+    VM::DeferExceptionScope deferExceptionScope(vm);
+
     MonotonicTime before { };
     if (TypeProfilerLogInternal::verbose) {
         dataLog("Process caller:'", reason, "'");
index 9387043..42fccd5 100644 (file)
@@ -56,7 +56,7 @@ public:
     TypeProfilerLog(VM&);
     ~TypeProfilerLog();
 
-    JS_EXPORT_PRIVATE void processLogEntries(const String&);
+    JS_EXPORT_PRIVATE void processLogEntries(VM&, const String&);
     LogEntry* logEndPtr() const { return m_logEndPtr; }
 
     void visit(SlotVisitor&);
index 186f7ae..256a583 100644 (file)
@@ -1116,7 +1116,7 @@ void VM::dumpTypeProfilerData()
     if (!typeProfiler())
         return;
 
-    typeProfilerLog()->processLogEntries("VM Dump Types"_s);
+    typeProfilerLog()->processLogEntries(*this, "VM Dump Types"_s);
     typeProfiler()->dumpTypeProfilerData(*this);
 }
 
index c03498e..e1ef468 100644 (file)
@@ -63,6 +63,7 @@
 #include <wtf/Gigacage.h>
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
+#include <wtf/SetForScope.h>
 #include <wtf/StackBounds.h>
 #include <wtf/StackPointer.h>
 #include <wtf/Stopwatch.h>
@@ -893,6 +894,19 @@ public:
     JS_EXPORT_PRIVATE void setRunLoop(CFRunLoopRef);
 #endif // USE(CF)
 
+    class DeferExceptionScope {
+    public:
+        DeferExceptionScope(VM& vm)
+            : m_savedException(vm.m_exception, nullptr)
+            , m_savedLastException(vm.m_lastException, nullptr)
+        {
+        }
+
+    private:
+        SetForScope<Exception*> m_savedException;
+        SetForScope<Exception*> m_savedLastException;
+    };
+
 private:
     friend class LLIntOffsetsExtractor;
 
index add8f18..a6eb1c2 100644 (file)
@@ -1946,7 +1946,7 @@ static EncodedJSValue JSC_HOST_CALL functionFindTypeForExpression(ExecState* exe
 {
     VM& vm = exec->vm();
     RELEASE_ASSERT(vm.typeProfiler());
-    vm.typeProfilerLog()->processLogEntries("jsc Testing API: functionFindTypeForExpression"_s);
+    vm.typeProfilerLog()->processLogEntries(vm, "jsc Testing API: functionFindTypeForExpression"_s);
 
     JSValue functionValue = exec->argument(0);
     RELEASE_ASSERT(functionValue.isFunction(vm));
@@ -1965,7 +1965,7 @@ static EncodedJSValue JSC_HOST_CALL functionReturnTypeFor(ExecState* exec)
 {
     VM& vm = exec->vm();
     RELEASE_ASSERT(vm.typeProfiler());
-    vm.typeProfilerLog()->processLogEntries("jsc Testing API: functionReturnTypeFor"_s);
+    vm.typeProfilerLog()->processLogEntries(vm, "jsc Testing API: functionReturnTypeFor"_s);
 
     JSValue functionValue = exec->argument(0);
     RELEASE_ASSERT(functionValue.isFunction(vm));