VMTraps has some races
https://bugs.webkit.org/show_bug.cgi?id=173941
Reviewed by Michael Saboff.
Source/JavaScriptCore:
This patch refactors much of the VMTraps API.
On the message sending side:
1) No longer uses the Yarr JIT check to determine if we are in
RegExp code. That was unsound because RegExp JIT code can be run
on compilation threads. Instead it looks at the current frame's
code block slot and checks if it is valid, which is the same as
what it did for JIT code previously.
2) Only have one signal sender thread, previously, there could be
many at once, which caused some data races. Additionally, the
signal sender thread is an automatic thread so it will deallocate
itself when not in use.
On the VMTraps breakpoint side:
1) We now have a true mapping of if we hit a breakpoint instead of
a JIT assertion. So the exception handler won't eat JIT assertions
anymore.
2) It jettisons all CodeBlocks that have VMTraps breakpoints on
them instead of every CodeBlock on the stack. This both prevents
us from hitting stale VMTraps breakpoints and also doesn't OSR
codeblocks that otherwise don't need to be jettisoned.
3) The old exception handler could theoretically fail for a couple
of reasons then resume execution with a clobbered instruction
set. This patch will kill the program if the exception handler
would fail.
This patch also refactors some of the jsc.cpp functions to take the
CommandLine options object instead of individual options. Also, there
is a new command line option that makes exceptions due to watchdog
timeouts an acceptable result.
* API/tests/testapi.c:
(main):
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::installVMTrapBreakpoints):
* dfg/DFGCommonData.cpp:
(JSC::DFG::pcCodeBlockMap):
(JSC::DFG::CommonData::invalidate):
(JSC::DFG::CommonData::~CommonData):
(JSC::DFG::CommonData::installVMTrapBreakpoints):
(JSC::DFG::codeBlockForVMTrapPC):
* dfg/DFGCommonData.h:
* jsc.cpp:
(functionDollarAgentStart):
(checkUncaughtException):
(checkException):
(runWithOptions):
(printUsageStatement):
(CommandLine::parseArguments):
(jscmain):
(runWithScripts): Deleted.
* runtime/JSLock.cpp:
(JSC::JSLock::didAcquireLock):
* runtime/VMTraps.cpp:
(JSC::sanitizedTopCallFrame):
(JSC::VMTraps::tryInstallTrapBreakpoints):
(JSC::VMTraps::willDestroyVM):
(JSC::VMTraps::fireTrap):
(JSC::VMTraps::handleTraps):
(JSC::VMTraps::VMTraps):
(JSC::VMTraps::~VMTraps):
(JSC::findActiveVMAndStackBounds): Deleted.
(JSC::installSignalHandler): Deleted.
(JSC::VMTraps::addSignalSender): Deleted.
(JSC::VMTraps::removeSignalSender): Deleted.
(JSC::VMTraps::SignalSender::willDestroyVM): Deleted.
(JSC::VMTraps::SignalSender::send): Deleted.
* runtime/VMTraps.h:
(JSC::VMTraps::~VMTraps): Deleted.
(JSC::VMTraps::SignalSender::SignalSender): Deleted.
Tools:
Add new testing mode for testing the Watchdog with our stress
tests.
* Scripts/run-jsc-stress-tests:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@218936 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/API/tests/testapi.c b/Source/JavaScriptCore/API/tests/testapi.c
index 9d3b456..f12cc2e 100644
--- a/Source/JavaScriptCore/API/tests/testapi.c
+++ b/Source/JavaScriptCore/API/tests/testapi.c
@@ -1348,7 +1348,6 @@
::SetErrorMode(0);
#endif
- testExecutionTimeLimit();
testCompareAndSwap();
startMultithreadedMultiVMExecutionTest();
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 606623b..2ee8371 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,86 @@
+2017-06-28 Keith Miller <keith_miller@apple.com>
+
+ VMTraps has some races
+ https://bugs.webkit.org/show_bug.cgi?id=173941
+
+ Reviewed by Michael Saboff.
+
+ This patch refactors much of the VMTraps API.
+
+ On the message sending side:
+
+ 1) No longer uses the Yarr JIT check to determine if we are in
+ RegExp code. That was unsound because RegExp JIT code can be run
+ on compilation threads. Instead it looks at the current frame's
+ code block slot and checks if it is valid, which is the same as
+ what it did for JIT code previously.
+
+ 2) Only have one signal sender thread, previously, there could be
+ many at once, which caused some data races. Additionally, the
+ signal sender thread is an automatic thread so it will deallocate
+ itself when not in use.
+
+ On the VMTraps breakpoint side:
+
+ 1) We now have a true mapping of if we hit a breakpoint instead of
+ a JIT assertion. So the exception handler won't eat JIT assertions
+ anymore.
+
+ 2) It jettisons all CodeBlocks that have VMTraps breakpoints on
+ them instead of every CodeBlock on the stack. This both prevents
+ us from hitting stale VMTraps breakpoints and also doesn't OSR
+ codeblocks that otherwise don't need to be jettisoned.
+
+ 3) The old exception handler could theoretically fail for a couple
+ of reasons then resume execution with a clobbered instruction
+ set. This patch will kill the program if the exception handler
+ would fail.
+
+ This patch also refactors some of the jsc.cpp functions to take the
+ CommandLine options object instead of individual options. Also, there
+ is a new command line option that makes exceptions due to watchdog
+ timeouts an acceptable result.
+
+ * API/tests/testapi.c:
+ (main):
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::installVMTrapBreakpoints):
+ * dfg/DFGCommonData.cpp:
+ (JSC::DFG::pcCodeBlockMap):
+ (JSC::DFG::CommonData::invalidate):
+ (JSC::DFG::CommonData::~CommonData):
+ (JSC::DFG::CommonData::installVMTrapBreakpoints):
+ (JSC::DFG::codeBlockForVMTrapPC):
+ * dfg/DFGCommonData.h:
+ * jsc.cpp:
+ (functionDollarAgentStart):
+ (checkUncaughtException):
+ (checkException):
+ (runWithOptions):
+ (printUsageStatement):
+ (CommandLine::parseArguments):
+ (jscmain):
+ (runWithScripts): Deleted.
+ * runtime/JSLock.cpp:
+ (JSC::JSLock::didAcquireLock):
+ * runtime/VMTraps.cpp:
+ (JSC::sanitizedTopCallFrame):
+ (JSC::VMTraps::tryInstallTrapBreakpoints):
+ (JSC::VMTraps::willDestroyVM):
+ (JSC::VMTraps::fireTrap):
+ (JSC::VMTraps::handleTraps):
+ (JSC::VMTraps::VMTraps):
+ (JSC::VMTraps::~VMTraps):
+ (JSC::findActiveVMAndStackBounds): Deleted.
+ (JSC::installSignalHandler): Deleted.
+ (JSC::VMTraps::addSignalSender): Deleted.
+ (JSC::VMTraps::removeSignalSender): Deleted.
+ (JSC::VMTraps::SignalSender::willDestroyVM): Deleted.
+ (JSC::VMTraps::SignalSender::send): Deleted.
+ * runtime/VMTraps.h:
+ (JSC::VMTraps::~VMTraps): Deleted.
+ (JSC::VMTraps::SignalSender::SignalSender): Deleted.
+
2017-06-28 Devin Rousso <drousso@apple.com>
Web Inspector: Instrument active pixel memory used by canvases
diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
index 259e64d..2f8e18c 100644
--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
@@ -3036,9 +3036,10 @@
// we should not perturb the refCount of m_jitCode.
if (!JITCode::isOptimizingJIT(jitType()))
return false;
- m_jitCode->dfgCommon()->installVMTrapBreakpoints();
+ m_jitCode->dfgCommon()->installVMTrapBreakpoints(this);
return true;
#else
+ UNREACHABLE_FOR_PLATFORM();
return false;
#endif
}
diff --git a/Source/JavaScriptCore/dfg/DFGCommonData.cpp b/Source/JavaScriptCore/dfg/DFGCommonData.cpp
index cd1e5ef..961475c 100644
--- a/Source/JavaScriptCore/dfg/DFGCommonData.cpp
+++ b/Source/JavaScriptCore/dfg/DFGCommonData.cpp
@@ -36,6 +36,8 @@
#include "TrackedReferences.h"
#include "VM.h"
+#include <wtf/NeverDestroyed.h>
+
namespace JSC { namespace DFG {
void CommonData::notifyCompilingStructureTransition(Plan& plan, CodeBlock* codeBlock, Node* node)
@@ -87,23 +89,76 @@
transitions.shrinkToFit();
}
+static StaticLock pcCodeBlockMapLock;
+inline HashMap<void*, CodeBlock*>& pcCodeBlockMap(AbstractLocker&)
+{
+ static NeverDestroyed<HashMap<void*, CodeBlock*>> pcCodeBlockMap;
+ return pcCodeBlockMap;
+}
+
bool CommonData::invalidate()
{
if (!isStillValid)
return false;
+
+ if (UNLIKELY(hasVMTrapsBreakpointsInstalled)) {
+ LockHolder locker(pcCodeBlockMapLock);
+ auto& map = pcCodeBlockMap(locker);
+ for (auto& jumpReplacement : jumpReplacements)
+ map.remove(jumpReplacement.dataLocation());
+ hasVMTrapsBreakpointsInstalled = false;
+ }
+
for (unsigned i = jumpReplacements.size(); i--;)
jumpReplacements[i].fire();
isStillValid = false;
return true;
}
-void CommonData::installVMTrapBreakpoints()
+CommonData::~CommonData()
{
+ if (UNLIKELY(hasVMTrapsBreakpointsInstalled)) {
+ LockHolder locker(pcCodeBlockMapLock);
+ auto& map = pcCodeBlockMap(locker);
+ for (auto& jumpReplacement : jumpReplacements)
+ map.remove(jumpReplacement.dataLocation());
+ }
+}
+
+void CommonData::installVMTrapBreakpoints(CodeBlock* owner)
+{
+ LockHolder locker(pcCodeBlockMapLock);
if (!isStillValid || hasVMTrapsBreakpointsInstalled)
return;
hasVMTrapsBreakpointsInstalled = true;
- for (unsigned i = jumpReplacements.size(); i--;)
- jumpReplacements[i].installVMTrapBreakpoint();
+
+ auto& map = pcCodeBlockMap(locker);
+#if !defined(NDEBUG)
+ // We need to be able to handle more than one invalidation point at the same pc
+ // but we want to make sure we don't forget to remove a pc from the map.
+ HashSet<void*> newReplacements;
+#endif
+ for (auto& jumpReplacement : jumpReplacements) {
+ jumpReplacement.installVMTrapBreakpoint();
+ void* source = jumpReplacement.dataLocation();
+ auto result = map.add(source, owner);
+ UNUSED_PARAM(result);
+#if !defined(NDEBUG)
+ ASSERT(result.isNewEntry || newReplacements.contains(source));
+ newReplacements.add(source);
+#endif
+ }
+}
+
+CodeBlock* codeBlockForVMTrapPC(void* pc)
+{
+ ASSERT(isJITPC(pc));
+ LockHolder locker(pcCodeBlockMapLock);
+ auto& map = pcCodeBlockMap(locker);
+ auto result = map.find(pc);
+ if (result == map.end())
+ return nullptr;
+ return result->value;
}
bool CommonData::isVMTrapBreakpoint(void* address)
diff --git a/Source/JavaScriptCore/dfg/DFGCommonData.h b/Source/JavaScriptCore/dfg/DFGCommonData.h
index fde0747..294c11f 100644
--- a/Source/JavaScriptCore/dfg/DFGCommonData.h
+++ b/Source/JavaScriptCore/dfg/DFGCommonData.h
@@ -75,6 +75,7 @@
, frameRegisterCount(std::numeric_limits<unsigned>::max())
, requiredRegisterCountForExit(std::numeric_limits<unsigned>::max())
{ }
+ ~CommonData();
void notifyCompilingStructureTransition(Plan&, CodeBlock*, Node*);
CallSiteIndex addCodeOrigin(CodeOrigin);
@@ -86,7 +87,7 @@
bool invalidate(); // Returns true if we did invalidate, or false if the code block was already invalidated.
bool hasInstalledVMTrapsBreakpoints() const { return isStillValid && hasVMTrapsBreakpointsInstalled; }
- void installVMTrapBreakpoints();
+ void installVMTrapBreakpoints(CodeBlock* owner);
bool isVMTrapBreakpoint(void* address);
unsigned requiredRegisterCountForExecutionAndExit() const
@@ -128,6 +129,8 @@
};
+CodeBlock* codeBlockForVMTrapPC(void* pc);
+
} } // namespace JSC::DFG
#endif // ENABLE(DFG_JIT)
diff --git a/Source/JavaScriptCore/jsc.cpp b/Source/JavaScriptCore/jsc.cpp
index a71fe76..aadc142 100644
--- a/Source/JavaScriptCore/jsc.cpp
+++ b/Source/JavaScriptCore/jsc.cpp
@@ -986,7 +986,7 @@
template<typename Func>
int runJSC(CommandLine, bool isWorker, const Func&);
-static void checkException(GlobalObject*, bool isLastFile, bool hasException, JSValue, const String& uncaughtExceptionName, bool alwaysDumpUncaughtException, bool dump, bool& success);
+static void checkException(GlobalObject*, bool isLastFile, bool hasException, JSValue, CommandLine&, bool& success);
class Message : public ThreadSafeRefCounted<Message> {
public:
@@ -1210,6 +1210,7 @@
bool m_profile { false };
String m_profilerOutput;
String m_uncaughtExceptionName;
+ bool m_treatWatchdogExceptionAsSuccess { false };
bool m_alwaysDumpUncaughtException { false };
bool m_dumpSamplingProfilerData { false };
bool m_enableRemoteDebugging { false };
@@ -2617,7 +2618,7 @@
result = evaluate(globalObject->globalExec(), makeSource(sourceCode, SourceOrigin(ASCIILiteral("worker"))), JSValue(), evaluationException);
if (evaluationException)
result = evaluationException->value();
- checkException(globalObject, true, evaluationException, result, String(), false, false, success);
+ checkException(globalObject, true, evaluationException, result, commandLine, success);
if (!success)
exit(1);
return success;
@@ -3292,8 +3293,9 @@
#undef CHECK_EXCEPTION
}
-static bool checkUncaughtException(VM& vm, GlobalObject* globalObject, JSValue exception, const String& expectedExceptionName, bool alwaysDumpException)
+static bool checkUncaughtException(VM& vm, GlobalObject* globalObject, JSValue exception, CommandLine& options)
{
+ const String& expectedExceptionName = options.m_uncaughtExceptionName;
auto scope = DECLARE_CATCH_SCOPE(vm);
scope.clearException();
if (!exception) {
@@ -3314,7 +3316,7 @@
return false;
}
if (isInstanceOfExpectedException) {
- if (alwaysDumpException)
+ if (options.m_alwaysDumpUncaughtException)
dumpException(globalObject, exception);
return true;
}
@@ -3324,25 +3326,32 @@
return false;
}
-static void checkException(GlobalObject* globalObject, bool isLastFile, bool hasException, JSValue value, const String& uncaughtExceptionName, bool alwaysDumpUncaughtException, bool dump, bool& success)
+static void checkException(GlobalObject* globalObject, bool isLastFile, bool hasException, JSValue value, CommandLine& options, bool& success)
{
VM& vm = globalObject->vm();
- if (!uncaughtExceptionName || !isLastFile) {
+
+ if (options.m_treatWatchdogExceptionAsSuccess && value.inherits(vm, TerminatedExecutionError::info())) {
+ ASSERT(hasException);
+ return;
+ }
+
+ if (!options.m_uncaughtExceptionName || !isLastFile) {
success = success && !hasException;
- if (dump && !hasException)
+ if (options.m_dump && !hasException)
printf("End: %s\n", value.toWTFString(globalObject->globalExec()).utf8().data());
if (hasException)
dumpException(globalObject, value);
} else
- success = success && checkUncaughtException(vm, globalObject, (hasException) ? value : JSValue(), uncaughtExceptionName, alwaysDumpUncaughtException);
+ success = success && checkUncaughtException(vm, globalObject, (hasException) ? value : JSValue(), options);
}
-static bool runWithScripts(GlobalObject* globalObject, const Vector<Script>& scripts, const String& uncaughtExceptionName, bool alwaysDumpUncaughtException, bool dump, bool module)
+static bool runWithOptions(GlobalObject* globalObject, CommandLine& options)
{
+ Vector<Script>& scripts = options.m_scripts;
String fileName;
Vector<char> scriptBuffer;
- if (dump)
+ if (options.m_dump)
JSC::Options::dumpGeneratedBytecodes() = true;
VM& vm = globalObject->vm();
@@ -3355,7 +3364,7 @@
for (size_t i = 0; i < scripts.size(); i++) {
JSInternalPromise* promise = nullptr;
- bool isModule = module || scripts[i].scriptType == Script::ScriptType::Module;
+ bool isModule = options.m_module || scripts[i].scriptType == Script::ScriptType::Module;
if (scripts[i].codeSource == Script::CodeSource::File) {
fileName = scripts[i].argument;
if (scripts[i].strictMode == Script::StrictMode::Strict)
@@ -3382,12 +3391,12 @@
scope.clearException();
JSFunction* fulfillHandler = JSNativeStdFunction::create(vm, globalObject, 1, String(), [&, isLastFile](ExecState* exec) {
- checkException(globalObject, isLastFile, false, exec->argument(0), uncaughtExceptionName, alwaysDumpUncaughtException, dump, success);
+ checkException(globalObject, isLastFile, false, exec->argument(0), options, success);
return JSValue::encode(jsUndefined());
});
JSFunction* rejectHandler = JSNativeStdFunction::create(vm, globalObject, 1, String(), [&, isLastFile](ExecState* exec) {
- checkException(globalObject, isLastFile, true, exec->argument(0), uncaughtExceptionName, alwaysDumpUncaughtException, dump, success);
+ checkException(globalObject, isLastFile, true, exec->argument(0), options, success);
return JSValue::encode(jsUndefined());
});
@@ -3400,7 +3409,7 @@
scope.assertNoException();
if (evaluationException)
returnValue = evaluationException->value();
- checkException(globalObject, isLastFile, evaluationException, returnValue, uncaughtExceptionName, alwaysDumpUncaughtException, dump, success);
+ checkException(globalObject, isLastFile, evaluationException, returnValue, options, success);
}
scriptBuffer.clear();
@@ -3502,6 +3511,7 @@
fprintf(stderr, " --strict-file=<file> Parse the given file as if it were in strict mode (this option may be passed more than once)\n");
fprintf(stderr, " --module-file=<file> Parse and evaluate the given file as module (this option may be passed more than once)\n");
fprintf(stderr, " --exception=<name> Check the last script exits with an uncaught exception with the specified name\n");
+ fprintf(stderr, " --watchdog-exception-ok Uncaught watchdog exceptions exit with success\n");
fprintf(stderr, " --dumpException Dump uncaught exception text\n");
fprintf(stderr, " --options Dumps all JSC VM options and exits\n");
fprintf(stderr, " --dumpOptions Dumps all non-default JSC VM options before continuing\n");
@@ -3631,6 +3641,11 @@
continue;
}
+ if (!strcmp(arg, "--watchdog-exception-ok")) {
+ m_treatWatchdogExceptionAsSuccess = true;
+ continue;
+ }
+
// See if the -- option is a JSC VM option.
if (strstr(arg, "--") == arg) {
if (!JSC::Options::setOption(&arg[2])) {
@@ -3778,7 +3793,7 @@
result = runJSC(
options, false,
[&] (VM&, GlobalObject* globalObject) {
- return runWithScripts(globalObject, options.m_scripts, options.m_uncaughtExceptionName, options.m_alwaysDumpUncaughtException, options.m_dump, options.m_module);
+ return runWithOptions(globalObject, options);
});
printSuperSamplerState();
diff --git a/Source/JavaScriptCore/runtime/JSLock.cpp b/Source/JavaScriptCore/runtime/JSLock.cpp
index 31e2db4..f83512f 100644
--- a/Source/JavaScriptCore/runtime/JSLock.cpp
+++ b/Source/JavaScriptCore/runtime/JSLock.cpp
@@ -154,10 +154,10 @@
registerThreadForMachExceptionHandling(&Thread::current());
#endif
+ // Note: everything below must come after addCurrentThread().
m_vm->traps().notifyGrabAllLocks();
#if ENABLE(SAMPLING_PROFILER)
- // Note: this must come after addCurrentThread().
if (SamplingProfiler* samplingProfiler = m_vm->samplingProfiler())
samplingProfiler->noticeJSLockAcquisition();
#endif
diff --git a/Source/JavaScriptCore/runtime/VMTraps.cpp b/Source/JavaScriptCore/runtime/VMTraps.cpp
index 44d53ea..02fa4fd 100644
--- a/Source/JavaScriptCore/runtime/VMTraps.cpp
+++ b/Source/JavaScriptCore/runtime/VMTraps.cpp
@@ -72,83 +72,7 @@
return !vm.entryScope && !vm.ownerThread();
}
-struct VMAndStackBounds {
- VM* vm;
- StackBounds stackBounds;
-};
-
-static Expected<VMAndStackBounds, VMTraps::Error> findActiveVMAndStackBounds(SignalContext& context)
-{
- VMInspector& inspector = VMInspector::instance();
- auto locker = tryHoldLock(inspector.getLock());
- if (UNLIKELY(!locker))
- return makeUnexpected(VMTraps::Error::LockUnavailable);
-
- VM* activeVM = nullptr;
- StackBounds stackBounds = StackBounds::emptyBounds();
- void* stackPointer = context.stackPointer;
- bool unableToAcquireMachineThreadsLock = false;
- inspector.iterate(locker, [&] (VM& vm) {
- if (vmIsInactive(vm))
- return VMInspector::FunctorStatus::Continue;
-
- auto& machineThreads = vm.heap.machineThreads();
- auto machineThreadsLocker = tryHoldLock(machineThreads.getLock());
- if (UNLIKELY(!machineThreadsLocker)) {
- unableToAcquireMachineThreadsLock = true;
- return VMInspector::FunctorStatus::Continue; // Try next VM.
- }
-
- const auto& threadList = machineThreads.threadsListHead(machineThreadsLocker);
- for (MachineThreads::MachineThread* thread = threadList.head(); thread; thread = thread->next()) {
- RELEASE_ASSERT(thread->stackBase());
- RELEASE_ASSERT(thread->stackEnd());
- if (stackPointer <= thread->stackBase() && stackPointer >= thread->stackEnd()) {
- activeVM = &vm;
- stackBounds = StackBounds(thread->stackBase(), thread->stackEnd());
- return VMInspector::FunctorStatus::Done;
- }
- }
- return VMInspector::FunctorStatus::Continue;
- });
-
- if (!activeVM && unableToAcquireMachineThreadsLock)
- return makeUnexpected(VMTraps::Error::LockUnavailable);
- return VMAndStackBounds { activeVM, stackBounds };
-}
-
-static void installSignalHandler()
-{
- installSignalHandler(Signal::BadAccess, [] (Signal, SigInfo&, PlatformRegisters& registers) -> SignalAction {
- SignalContext context(registers);
-
- if (!isJITPC(context.trapPC))
- return SignalAction::NotHandled;
-
- // FIXME: This currently eats all traps including jit asserts we should make sure this
- // always works. https://bugs.webkit.org/show_bug.cgi?id=171039
- auto activeVMAndStackBounds = findActiveVMAndStackBounds(context);
- if (!activeVMAndStackBounds)
- return SignalAction::Handled; // Let the SignalSender try again later.
-
- VM* vm = activeVMAndStackBounds.value().vm;
- if (vm) {
- VMTraps& traps = vm->traps();
- if (!traps.needTrapHandling())
- return SignalAction::Handled; // The polling code beat us to handling the trap already.
-
- auto expectedSuccess = traps.tryJettisonCodeBlocksOnStack(context);
- if (!expectedSuccess)
- return SignalAction::Handled; // Let the SignalSender try again later.
- if (expectedSuccess.value())
- return SignalAction::Handled; // We've success jettison the codeBlocks.
- }
-
- return SignalAction::Handled;
- });
-}
-
-ALWAYS_INLINE static CallFrame* sanitizedTopCallFrame(CallFrame* topCallFrame)
+inline CallFrame* sanitizedTopCallFrame(CallFrame* topCallFrame)
{
#if !defined(NDEBUG) && !CPU(ARM) && !CPU(MIPS)
// prepareForExternalCall() in DFGSpeculativeJIT.h may set topCallFrame to a bad word
@@ -183,28 +107,17 @@
CallFrame* callFrame = reinterpret_cast<CallFrame*>(context.framePointer);
- auto codeBlockSetLocker = tryHoldLock(vm.heap.codeBlockSet().getLock());
+ auto& lock = vm.heap.codeBlockSet().getLock();
+ // If the target thread is in C++ code it might be holding the codeBlockSet lock.
+ // if it's in JIT code then it cannot be holding that lock but the GC might be.
+ auto codeBlockSetLocker = isJITPC(trapPC) ? holdLock(lock) : tryHoldLock(lock);
if (!codeBlockSetLocker)
return; // Let the SignalSender try again later.
- {
- auto& allocator = ExecutableAllocator::singleton();
- auto allocatorLocker = tryHoldLock(allocator.getLock());
- if (!allocatorLocker)
- return; // Let the SignalSender try again later.
-
- if (allocator.isValidExecutableMemory(allocatorLocker, trapPC)) {
- if (vm.isExecutingInRegExpJIT) {
- // We need to do this because a regExpJIT frame isn't a JS frame.
- callFrame = sanitizedTopCallFrame(vm.topCallFrame);
- }
- } else if (LLInt::isLLIntPC(trapPC)) {
- // The framePointer probably has the callFrame. We're good to go.
- } else {
- // We resort to topCallFrame to see if we can get anything
- // useful. We usually get here when we're executing C code.
- callFrame = sanitizedTopCallFrame(vm.topCallFrame);
- }
+ if (!isJITPC(trapPC) && !LLInt::isLLIntPC(trapPC)) {
+ // We resort to topCallFrame to see if we can get anything
+ // useful. We usually get here when we're executing C code.
+ callFrame = sanitizedTopCallFrame(vm.topCallFrame);
}
CodeBlock* foundCodeBlock = nullptr;
@@ -240,7 +153,7 @@
}
if (JITCode::isOptimizingJIT(foundCodeBlock->jitType())) {
- auto locker = tryHoldLock(m_lock);
+ auto locker = tryHoldLock(*m_lock);
if (!locker)
return; // Let the SignalSender try again later.
@@ -250,45 +163,6 @@
}
}
-auto VMTraps::tryJettisonCodeBlocksOnStack(SignalContext& context) -> Expected<bool, Error>
-{
- VM& vm = this->vm();
- auto codeBlockSetLocker = tryHoldLock(vm.heap.codeBlockSet().getLock());
- if (!codeBlockSetLocker)
- return makeUnexpected(Error::LockUnavailable);
-
- CallFrame* topCallFrame = reinterpret_cast<CallFrame*>(context.framePointer);
- void* trapPC = context.trapPC;
- bool trapPCIsVMTrap = false;
-
- vm.heap.forEachCodeBlockIgnoringJITPlans(codeBlockSetLocker, [&] (CodeBlock* codeBlock) {
- if (!codeBlock->hasInstalledVMTrapBreakpoints())
- return false; // Not found yet.
-
- JITCode* jitCode = codeBlock->jitCode().get();
- ASSERT(JITCode::isOptimizingJIT(jitCode->jitType()));
- if (jitCode->dfgCommon()->isVMTrapBreakpoint(trapPC)) {
- trapPCIsVMTrap = true;
- // At the codeBlock trap point, we're guaranteed that:
- // 1. the pc is not in the middle of any range of JIT code which invalidation points
- // may write over. Hence, it's now safe to patch those invalidation points and
- // jettison the codeBlocks.
- // 2. The top frame must be an optimized JS frame.
- ASSERT(codeBlock == topCallFrame->codeBlock());
- codeBlock->jettison(Profiler::JettisonDueToVMTraps);
- return true;
- }
-
- return false; // Not found yet.
- });
-
- if (!trapPCIsVMTrap)
- return false;
-
- invalidateCodeBlocksOnStack(codeBlockSetLocker, topCallFrame);
- return true;
-}
-
void VMTraps::invalidateCodeBlocksOnStack()
{
invalidateCodeBlocksOnStack(vm().topCallFrame);
@@ -321,6 +195,111 @@
}
}
+class VMTraps::SignalSender final : public AutomaticThread {
+public:
+ using Base = AutomaticThread;
+ SignalSender(const AbstractLocker& locker, VM& vm)
+ : Base(locker, vm.traps().m_lock, vm.traps().m_trapSet)
+ , m_vm(vm)
+ {
+ static std::once_flag once;
+ std::call_once(once, [] {
+ installSignalHandler(Signal::BadAccess, [] (Signal, SigInfo&, PlatformRegisters& registers) -> SignalAction {
+ SignalContext context(registers);
+
+ if (!isJITPC(context.trapPC))
+ return SignalAction::NotHandled;
+
+ CodeBlock* currentCodeBlock = DFG::codeBlockForVMTrapPC(context.trapPC);
+ ASSERT(currentCodeBlock->hasInstalledVMTrapBreakpoints());
+ VM& vm = *currentCodeBlock->vm();
+ ASSERT(vm.traps().needTrapHandling()); // We should have already jettisoned this code block when we handled the trap.
+
+ // We are in JIT code so it's safe to aquire this lock.
+ auto codeBlockSetLocker = holdLock(vm.heap.codeBlockSet().getLock());
+ bool sawCurrentCodeBlock = false;
+ vm.heap.forEachCodeBlockIgnoringJITPlans(codeBlockSetLocker, [&] (CodeBlock* codeBlock) {
+ // We want to jettison all code blocks that have vm traps breakpoints, otherwise we could hit them later.
+ if (codeBlock->hasInstalledVMTrapBreakpoints()) {
+ if (currentCodeBlock == codeBlock)
+ sawCurrentCodeBlock = true;
+
+ codeBlock->jettison(Profiler::JettisonDueToVMTraps);
+ }
+ return false;
+ });
+ RELEASE_ASSERT(sawCurrentCodeBlock);
+
+ return SignalAction::Handled; // We've successfully jettisoned the codeBlocks.
+ });
+ });
+ }
+
+ VMTraps& traps() { return m_vm.traps(); }
+
+protected:
+ PollResult poll(const AbstractLocker&) override
+ {
+ if (traps().m_isShuttingDown)
+ return PollResult::Stop;
+
+ if (!traps().needTrapHandling())
+ return PollResult::Wait;
+
+ // We know that no trap could have been processed and re-added because we are holding the lock.
+ if (vmIsInactive(m_vm))
+ return PollResult::Wait;
+ return PollResult::Work;
+ }
+
+ WorkResult work() override
+ {
+
+ // We need a nested scope so that we'll release the lock before we sleep below.
+ VM& vm = m_vm;
+
+ auto optionalOwnerThread = vm.ownerThread();
+ if (optionalOwnerThread) {
+ sendMessage(*optionalOwnerThread.value().get(), [&] (PlatformRegisters& registers) -> void {
+ SignalContext context(registers);
+
+ auto ownerThread = vm.apiLock().ownerThread();
+ // We can't mess with a thread unless it's the one we suspended.
+ if (!ownerThread || ownerThread != optionalOwnerThread)
+ return;
+
+ Thread& thread = *ownerThread->get();
+ StackBounds stackBounds = StackBounds::emptyBounds();
+ {
+ // FIXME: We need to use the machine threads because it is the only non-TLS source
+ // for the stack bounds of this thread. We should keep in on the WTF::Thread instead.
+ // see: https://bugs.webkit.org/show_bug.cgi?id=173975
+ MachineThreads& machineThreads = vm.heap.machineThreads();
+ auto machineThreadsLock = tryHoldLock(machineThreads.getLock());
+ if (!machineThreadsLock)
+ return; // Try again later.
+
+ auto& threadList = machineThreads.threadsListHead(machineThreadsLock);
+ for (MachineThreads::MachineThread* machineThread = threadList.head(); machineThread; machineThread = machineThread->next()) {
+ if (machineThread->m_thread.get() == thread)
+ stackBounds = StackBounds(machineThread->stackBase(), machineThread->stackEnd());
+ }
+ RELEASE_ASSERT(!stackBounds.isEmpty());
+ }
+
+ vm.traps().tryInstallTrapBreakpoints(context, stackBounds);
+ });
+ }
+
+ sleepMS(1);
+ return WorkResult::Continue;
+ }
+
+private:
+
+ VM& m_vm;
+};
+
#endif // ENABLE(SIGNAL_BASED_VM_TRAPS)
void VMTraps::willDestroyVM()
@@ -328,98 +307,24 @@
m_isShuttingDown = true;
WTF::storeStoreFence();
#if ENABLE(SIGNAL_BASED_VM_TRAPS)
- while (!m_signalSenders.isEmpty()) {
- RefPtr<SignalSender> sender;
+ if (m_signalSender) {
{
- // We don't want to be holding the VMTraps lock when calling
- // SignalSender::willDestroyVM() because SignalSender::willDestroyVM()
- // will acquire the SignalSender lock, and SignalSender::send() needs
- // to acquire these locks in the opposite order.
- auto locker = holdLock(m_lock);
- sender = m_signalSenders.takeAny();
- if (!sender)
- break;
+ auto locker = holdLock(*m_lock);
+ if (!m_signalSender->tryStop(locker))
+ m_trapSet->notifyAll(locker);
}
- sender->willDestroyVM();
+ if (!ASSERT_DISABLED)
+ m_signalSender->join();
+ m_signalSender = nullptr;
}
- ASSERT(m_signalSenders.isEmpty());
#endif
}
-#if ENABLE(SIGNAL_BASED_VM_TRAPS)
-void VMTraps::addSignalSender(VMTraps::SignalSender* sender)
-{
- auto locker = holdLock(m_lock);
- m_signalSenders.add(sender);
-}
-
-void VMTraps::removeSignalSender(VMTraps::SignalSender* sender)
-{
- auto locker = holdLock(m_lock);
- m_signalSenders.remove(sender);
-}
-
-void VMTraps::SignalSender::willDestroyVM()
-{
- auto locker = holdLock(m_lock);
- m_vm = nullptr;
-}
-
-void VMTraps::SignalSender::send()
-{
- static std::once_flag once;
- std::call_once(once, [] {
- installSignalHandler();
- });
-
- while (true) {
- // We need a nested scope so that we'll release the lock before we sleep below.
- {
- auto locker = holdLock(m_lock);
- if (!m_vm)
- break;
-
- VM& vm = *m_vm;
- auto optionalOwnerThread = vm.ownerThread();
- if (optionalOwnerThread) {
- sendMessage(*optionalOwnerThread.value().get(), [] (PlatformRegisters& registers) -> void {
- SignalContext context(registers);
- auto activeVMAndStackBounds = findActiveVMAndStackBounds(context);
- if (activeVMAndStackBounds) {
- VM* vm = activeVMAndStackBounds.value().vm;
- if (vm) {
- StackBounds stackBounds = activeVMAndStackBounds.value().stackBounds;
- VMTraps& traps = vm->traps();
- if (traps.needTrapHandling())
- traps.tryInstallTrapBreakpoints(context, stackBounds);
- }
- }
- });
- break;
- }
-
- if (vmIsInactive(vm))
- break;
-
- VMTraps::Mask mask(m_eventType);
- if (!vm.needTrapHandling(mask))
- break;
- }
-
- sleepMS(1);
- }
-
- auto locker = holdLock(m_lock);
- if (m_vm)
- m_vm->traps().removeSignalSender(this);
-}
-#endif // ENABLE(SIGNAL_BASED_VM_TRAPS)
-
void VMTraps::fireTrap(VMTraps::EventType eventType)
{
ASSERT(!vm().currentThreadIsHoldingAPILock());
{
- auto locker = holdLock(m_lock);
+ auto locker = holdLock(*m_lock);
ASSERT(!m_isShuttingDown);
setTrapForEvent(locker, eventType);
m_needToInvalidatedCodeBlocks = true;
@@ -430,11 +335,10 @@
// sendSignal() can loop until it has confirmation that the mutator thread
// has received the trap request. We'll call it from another trap so that
// fireTrap() does not block.
- RefPtr<SignalSender> sender = adoptRef(new SignalSender(vm(), eventType));
- addSignalSender(sender.get());
- Thread::create("jsc.vmtraps.signalling.thread", [sender] {
- sender->send();
- });
+ auto locker = holdLock(*m_lock);
+ if (!m_signalSender)
+ m_signalSender = adoptRef(new SignalSender(locker, vm()));
+ m_trapSet->notifyAll(locker);
}
#endif
}
@@ -444,6 +348,16 @@
VM& vm = this->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
+ {
+ auto codeBlockSetLocker = holdLock(vm.heap.codeBlockSet().getLock());
+ vm.heap.forEachCodeBlockIgnoringJITPlans(codeBlockSetLocker, [&] (CodeBlock* codeBlock) {
+ // We want to jettison all code blocks that have vm traps breakpoints, otherwise we could hit them later.
+ if (codeBlock->hasInstalledVMTrapBreakpoints())
+ codeBlock->jettison(Profiler::JettisonDueToVMTraps);
+ return false;
+ });
+ }
+
ASSERT(needTrapHandling(mask));
while (needTrapHandling(mask)) {
auto eventType = takeTopPriorityTrap(mask);
@@ -460,7 +374,6 @@
FALLTHROUGH;
case NeedTermination:
- invalidateCodeBlocksOnStack(exec);
throwException(exec, scope, createTerminatedExecutionException(&vm));
return;
@@ -472,7 +385,7 @@
auto VMTraps::takeTopPriorityTrap(VMTraps::Mask mask) -> EventType
{
- auto locker = holdLock(m_lock);
+ auto locker = holdLock(*m_lock);
for (int i = 0; i < NumberOfEventTypes; ++i) {
EventType eventType = static_cast<EventType>(i);
if (hasTrapForEvent(locker, eventType, mask)) {
@@ -483,4 +396,17 @@
return Invalid;
}
+VMTraps::VMTraps()
+ : m_lock(Box<Lock>::create())
+ , m_trapSet(AutomaticThreadCondition::create())
+{
+}
+
+VMTraps::~VMTraps()
+{
+#if ENABLE(SIGNAL_BASED_VM_TRAPS)
+ ASSERT(!m_signalSender);
+#endif
+}
+
} // namespace JSC
diff --git a/Source/JavaScriptCore/runtime/VMTraps.h b/Source/JavaScriptCore/runtime/VMTraps.h
index 9b72647..9b06cdd 100644
--- a/Source/JavaScriptCore/runtime/VMTraps.h
+++ b/Source/JavaScriptCore/runtime/VMTraps.h
@@ -25,6 +25,8 @@
#pragma once
+#include <wtf/AutomaticThread.h>
+#include <wtf/Box.h>
#include <wtf/Expected.h>
#include <wtf/HashSet.h>
#include <wtf/Lock.h>
@@ -86,12 +88,8 @@
BitField m_mask;
};
- ~VMTraps()
- {
-#if ENABLE(SIGNAL_BASED_VM_TRAPS)
- ASSERT(m_signalSenders.isEmpty());
-#endif
- }
+ ~VMTraps();
+ VMTraps();
void willDestroyVM();
@@ -110,7 +108,6 @@
void handleTraps(ExecState*, VMTraps::Mask);
void tryInstallTrapBreakpoints(struct SignalContext&, StackBounds);
- Expected<bool, Error> tryJettisonCodeBlocksOnStack(struct SignalContext&);
private:
VM& vm() const;
@@ -134,21 +131,8 @@
EventType takeTopPriorityTrap(Mask);
#if ENABLE(SIGNAL_BASED_VM_TRAPS)
- class SignalSender : public ThreadSafeRefCounted<SignalSender> {
- public:
- SignalSender(VM& vm, EventType eventType)
- : m_vm(&vm)
- , m_eventType(eventType)
- { }
-
- void willDestroyVM();
- void send();
-
- private:
- Lock m_lock;
- VM* m_vm;
- EventType m_eventType;
- };
+ class SignalSender;
+ friend class SignalSender;
void invalidateCodeBlocksOnStack();
void invalidateCodeBlocksOnStack(ExecState* topCallFrame);
@@ -161,7 +145,8 @@
void invalidateCodeBlocksOnStack(ExecState*) { }
#endif
- Lock m_lock;
+ Box<Lock> m_lock;
+ RefPtr<AutomaticThreadCondition> m_trapSet;
union {
BitField m_needTrapHandling { 0 };
BitField m_trapsBitField;
@@ -170,7 +155,7 @@
bool m_isShuttingDown { false };
#if ENABLE(SIGNAL_BASED_VM_TRAPS)
- HashSet<RefPtr<SignalSender>> m_signalSenders;
+ RefPtr<SignalSender> m_signalSender;
#endif
friend class LLIntOffsetsExtractor;
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 5e7a7fe..c6744bf 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,15 @@
+2017-06-28 Keith Miller <keith_miller@apple.com>
+
+ VMTraps has some races
+ https://bugs.webkit.org/show_bug.cgi?id=173941
+
+ Reviewed by Michael Saboff.
+
+ Add new testing mode for testing the Watchdog with our stress
+ tests.
+
+ * Scripts/run-jsc-stress-tests:
+
2017-06-29 Carlos Garcia Campos <cgarcia@igalia.com>
[GTK][WPE] Implement API::IconLoadingClient and rework WebKitFaviconDatabase to use IconDatabase directly
diff --git a/Tools/Scripts/run-jsc-stress-tests b/Tools/Scripts/run-jsc-stress-tests
index d142d3d..8ef2ddc 100755
--- a/Tools/Scripts/run-jsc-stress-tests
+++ b/Tools/Scripts/run-jsc-stress-tests
@@ -890,6 +890,11 @@
run("ftl-eager", "--airForceBriggsAllocator=true", *(FTL_OPTIONS + EAGER_OPTIONS + COLLECT_CONTINUOUSLY_OPTIONS + optionalTestSpecificOptions))
end
+def runFTLEagerWatchdog(*optionalTestSpecificOptions)
+ timeout = rand(100)
+ run("ftl-eager-watchdog-#{timeout}", "--watchdog=#{timeout}", "--watchdog-exception-ok", *(FTL_OPTIONS + EAGER_OPTIONS + COLLECT_CONTINUOUSLY_OPTIONS + optionalTestSpecificOptions))
+end
+
def runFTLEagerNoCJITValidate(*optionalTestSpecificOptions)
run("ftl-eager-no-cjit", "--validateGraph=true", "--airForceIRCAllocator=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + EAGER_OPTIONS + COLLECT_CONTINUOUSLY_OPTIONS + optionalTestSpecificOptions))
end
