Diff
Modified: trunk/Source/_javascript_Core/API/tests/testapi.c (218935 => 218936)
--- trunk/Source/_javascript_Core/API/tests/testapi.c 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/API/tests/testapi.c 2017-06-29 17:34:57 UTC (rev 218936)
@@ -1348,7 +1348,6 @@
::SetErrorMode(0);
#endif
- testExecutionTimeLimit();
testCompareAndSwap();
startMultithreadedMultiVMExecutionTest();
Modified: trunk/Source/_javascript_Core/ChangeLog (218935 => 218936)
--- trunk/Source/_javascript_Core/ChangeLog 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-06-29 17:34:57 UTC (rev 218936)
@@ -1,3 +1,86 @@
+2017-06-28 Keith Miller <[email protected]>
+
+ 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 <[email protected]>
Web Inspector: Instrument active pixel memory used by canvases
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (218935 => 218936)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2017-06-29 17:34:57 UTC (rev 218936)
@@ -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
}
Modified: trunk/Source/_javascript_Core/dfg/DFGCommonData.cpp (218935 => 218936)
--- trunk/Source/_javascript_Core/dfg/DFGCommonData.cpp 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/dfg/DFGCommonData.cpp 2017-06-29 17:34:57 UTC (rev 218936)
@@ -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,10 +89,26 @@
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;
@@ -97,15 +115,52 @@
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)
{
if (!isStillValid)
Modified: trunk/Source/_javascript_Core/dfg/DFGCommonData.h (218935 => 218936)
--- trunk/Source/_javascript_Core/dfg/DFGCommonData.h 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/dfg/DFGCommonData.h 2017-06-29 17:34:57 UTC (rev 218936)
@@ -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)
Modified: trunk/Source/_javascript_Core/jsc.cpp (218935 => 218936)
--- trunk/Source/_javascript_Core/jsc.cpp 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/jsc.cpp 2017-06-29 17:34:57 UTC (rev 218936)
@@ -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();
Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (218935 => 218936)
--- trunk/Source/_javascript_Core/runtime/JSLock.cpp 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp 2017-06-29 17:34:57 UTC (rev 218936)
@@ -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
Modified: trunk/Source/_javascript_Core/runtime/VMTraps.cpp (218935 => 218936)
--- trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2017-06-29 17:34:57 UTC (rev 218936)
@@ -72,84 +72,8 @@
return !vm.entryScope && !vm.ownerThread();
}
-struct VMAndStackBounds {
- VM* vm;
- StackBounds stackBounds;
-};
-
-static Expected<VMAndStackBounds, VMTraps::Error> findActiveVMAndStackBounds(SignalContext& context)
+inline CallFrame* sanitizedTopCallFrame(CallFrame* topCallFrame)
{
- 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)
-{
#if !defined(NDEBUG) && !CPU(ARM) && !CPU(MIPS)
// prepareForExternalCall() in DFGSpeculativeJIT.h may set topCallFrame to a bad word
// before calling native functions, but tryInstallTrapBreakpoints() below expects
@@ -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,105 +195,136 @@
}
}
-#endif // ENABLE(SIGNAL_BASED_VM_TRAPS)
+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);
-void VMTraps::willDestroyVM()
-{
- m_isShuttingDown = true;
- WTF::storeStoreFence();
-#if ENABLE(SIGNAL_BASED_VM_TRAPS)
- while (!m_signalSenders.isEmpty()) {
- RefPtr<SignalSender> sender;
- {
- // 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;
- }
- sender->willDestroyVM();
+ 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.
+ });
+ });
}
- 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);
-}
+ VMTraps& traps() { return m_vm.traps(); }
-void VMTraps::removeSignalSender(VMTraps::SignalSender* sender)
-{
- auto locker = holdLock(m_lock);
- m_signalSenders.remove(sender);
-}
+protected:
+ PollResult poll(const AbstractLocker&) override
+ {
+ if (traps().m_isShuttingDown)
+ return PollResult::Stop;
-void VMTraps::SignalSender::willDestroyVM()
-{
- auto locker = holdLock(m_lock);
- m_vm = nullptr;
-}
+ if (!traps().needTrapHandling())
+ return PollResult::Wait;
-void VMTraps::SignalSender::send()
-{
- static std::once_flag once;
- std::call_once(once, [] {
- installSignalHandler();
- });
+ // 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;
+ }
- while (true) {
+ WorkResult work() override
+ {
+
// 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;
- 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);
- }
+ 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());
}
- });
- break;
- }
+ RELEASE_ASSERT(!stackBounds.isEmpty());
+ }
- if (vmIsInactive(vm))
- break;
-
- VMTraps::Mask mask(m_eventType);
- if (!vm.needTrapHandling(mask))
- break;
+ vm.traps().tryInstallTrapBreakpoints(context, stackBounds);
+ });
}
sleepMS(1);
+ return WorkResult::Continue;
}
+
+private:
- auto locker = holdLock(m_lock);
- if (m_vm)
- m_vm->traps().removeSignalSender(this);
-}
+ VM& m_vm;
+};
+
#endif // ENABLE(SIGNAL_BASED_VM_TRAPS)
+void VMTraps::willDestroyVM()
+{
+ m_isShuttingDown = true;
+ WTF::storeStoreFence();
+#if ENABLE(SIGNAL_BASED_VM_TRAPS)
+ if (m_signalSender) {
+ {
+ auto locker = holdLock(*m_lock);
+ if (!m_signalSender->tryStop(locker))
+ m_trapSet->notifyAll(locker);
+ }
+ if (!ASSERT_DISABLED)
+ m_signalSender->join();
+ m_signalSender = nullptr;
+ }
+#endif
+}
+
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
Modified: trunk/Source/_javascript_Core/runtime/VMTraps.h (218935 => 218936)
--- trunk/Source/_javascript_Core/runtime/VMTraps.h 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Source/_javascript_Core/runtime/VMTraps.h 2017-06-29 17:34:57 UTC (rev 218936)
@@ -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,22 +131,9 @@
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)
- { }
+ class SignalSender;
+ friend class SignalSender;
- void willDestroyVM();
- void send();
-
- private:
- Lock m_lock;
- VM* m_vm;
- EventType m_eventType;
- };
-
void invalidateCodeBlocksOnStack();
void invalidateCodeBlocksOnStack(ExecState* topCallFrame);
void invalidateCodeBlocksOnStack(Locker<Lock>& codeBlockSetLocker, 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;
Modified: trunk/Tools/ChangeLog (218935 => 218936)
--- trunk/Tools/ChangeLog 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Tools/ChangeLog 2017-06-29 17:34:57 UTC (rev 218936)
@@ -1,3 +1,15 @@
+2017-06-28 Keith Miller <[email protected]>
+
+ 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 <[email protected]>
[GTK][WPE] Implement API::IconLoadingClient and rework WebKitFaviconDatabase to use IconDatabase directly
Modified: trunk/Tools/Scripts/run-jsc-stress-tests (218935 => 218936)
--- trunk/Tools/Scripts/run-jsc-stress-tests 2017-06-29 17:21:45 UTC (rev 218935)
+++ trunk/Tools/Scripts/run-jsc-stress-tests 2017-06-29 17:34:57 UTC (rev 218936)
@@ -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
