- Revision
- 228767
- Author
- carlo...@webkit.org
- Date
- 2018-02-20 02:11:24 -0800 (Tue, 20 Feb 2018)
Log Message
Merge r228488 - Setting a VMTrap shouldn't look at topCallFrame since that may imply we're in C code and holding the malloc lock
https://bugs.webkit.org/show_bug.cgi?id=182801
Reviewed by Keith Miller.
JSTests:
* stress/watchdog-dont-malloc-when-in-c-code.js: Added.
Source/_javascript_Core:
VMTraps would sometimes install traps when it paused the JS thread when it
was in C code. This is wrong, as installing traps mallocs, and the JS thread
may have been holding the malloc lock while in C code. This could lead to a
deadlock when C code was holding the malloc lock.
This patch makes it so that we only install traps when we've proven the PC
is in JIT or LLInt code. If we're in JIT/LLInt code, we are guaranteed that
we're not holding the malloc lock.
* jsc.cpp:
(GlobalObject::finishCreation):
(functionMallocInALoop):
* runtime/VMTraps.cpp:
(JSC::VMTraps::tryInstallTrapBreakpoints):
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog (228766 => 228767)
--- releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog 2018-02-20 10:11:16 UTC (rev 228766)
+++ releases/WebKitGTK/webkit-2.20/JSTests/ChangeLog 2018-02-20 10:11:24 UTC (rev 228767)
@@ -1,3 +1,12 @@
+2018-02-14 Saam Barati <sbar...@apple.com>
+
+ Setting a VMTrap shouldn't look at topCallFrame since that may imply we're in C code and holding the malloc lock
+ https://bugs.webkit.org/show_bug.cgi?id=182801
+
+ Reviewed by Keith Miller.
+
+ * stress/watchdog-dont-malloc-when-in-c-code.js: Added.
+
2018-02-13 Saam Barati <sbar...@apple.com>
putDirectIndexSlowOrBeyondVectorLength needs to convert to dictionary indexing mode always if attributes are present
Added: releases/WebKitGTK/webkit-2.20/JSTests/stress/watchdog-dont-malloc-when-in-c-code.js (0 => 228767)
--- releases/WebKitGTK/webkit-2.20/JSTests/stress/watchdog-dont-malloc-when-in-c-code.js (rev 0)
+++ releases/WebKitGTK/webkit-2.20/JSTests/stress/watchdog-dont-malloc-when-in-c-code.js 2018-02-20 10:11:24 UTC (rev 228767)
@@ -0,0 +1,5 @@
+//@ runFTLEagerWatchdog
+
+for (let i = 0; i < 7000; ++i) {
+ mallocInALoop();
+}
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog (228766 => 228767)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog 2018-02-20 10:11:16 UTC (rev 228766)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog 2018-02-20 10:11:24 UTC (rev 228767)
@@ -1,3 +1,25 @@
+2018-02-14 Saam Barati <sbar...@apple.com>
+
+ Setting a VMTrap shouldn't look at topCallFrame since that may imply we're in C code and holding the malloc lock
+ https://bugs.webkit.org/show_bug.cgi?id=182801
+
+ Reviewed by Keith Miller.
+
+ VMTraps would sometimes install traps when it paused the JS thread when it
+ was in C code. This is wrong, as installing traps mallocs, and the JS thread
+ may have been holding the malloc lock while in C code. This could lead to a
+ deadlock when C code was holding the malloc lock.
+
+ This patch makes it so that we only install traps when we've proven the PC
+ is in JIT or LLInt code. If we're in JIT/LLInt code, we are guaranteed that
+ we're not holding the malloc lock.
+
+ * jsc.cpp:
+ (GlobalObject::finishCreation):
+ (functionMallocInALoop):
+ * runtime/VMTraps.cpp:
+ (JSC::VMTraps::tryInstallTrapBreakpoints):
+
2018-02-14 Michael Saboff <msab...@apple.com>
REGRESSION(225695) : com.apple.WebKit.WebContent at com.apple._javascript_Core: JSC::RegExp::match + 630 :: stack overflow
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/jsc.cpp (228766 => 228767)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/jsc.cpp 2018-02-20 10:11:16 UTC (rev 228766)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/jsc.cpp 2018-02-20 10:11:24 UTC (rev 228767)
@@ -342,6 +342,7 @@
static EncodedJSValue JSC_HOST_CALL functionHeapCapacity(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionFlashHeapAccess(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionDisableRichSourceInfo(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionMallocInALoop(ExecState*);
struct Script {
enum class StrictMode {
@@ -599,6 +600,7 @@
addFunction(vm, "flashHeapAccess", functionFlashHeapAccess, 0);
addFunction(vm, "disableRichSourceInfo", functionDisableRichSourceInfo, 0);
+ addFunction(vm, "mallocInALoop", functionMallocInALoop, 0);
}
void addFunction(VM& vm, JSObject* object, const char* name, NativeFunction function, unsigned arguments)
@@ -1748,6 +1750,16 @@
return JSValue::encode(jsUndefined());
}
+EncodedJSValue JSC_HOST_CALL functionMallocInALoop(ExecState*)
+{
+ Vector<void*> ptrs;
+ for (unsigned i = 0; i < 5000; ++i)
+ ptrs.append(fastMalloc(1024 * 2));
+ for (void* ptr : ptrs)
+ fastFree(ptr);
+ return JSValue::encode(jsUndefined());
+}
+
template<typename ValueType>
typename std::enable_if<!std::is_fundamental<ValueType>::value>::type addOption(VM&, JSObject*, Identifier, ValueType) { }
Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/VMTraps.cpp (228766 => 228767)
--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/VMTraps.cpp 2018-02-20 10:11:16 UTC (rev 228766)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/VMTraps.cpp 2018-02-20 10:11:24 UTC (rev 228767)
@@ -72,23 +72,6 @@
return !vm.entryScope && !vm.ownerThread();
}
-inline 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
- // topCallFrame to be null if not set.
-#if USE(JSVALUE64)
- const uintptr_t badBeefWord = 0xbadbeef0badbeef;
-#else
- const uintptr_t badBeefWord = 0xbadbeef;
-#endif
- if (topCallFrame == reinterpret_cast<CallFrame*>(badBeefWord))
- topCallFrame = nullptr;
-#endif
- return topCallFrame;
-}
-
static bool isSaneFrame(CallFrame* frame, CallFrame* calleeFrame, EntryFrame* entryFrame, StackBounds stackBounds)
{
if (reinterpret_cast<void*>(frame) >= reinterpret_cast<void*>(entryFrame))
@@ -104,22 +87,19 @@
// Let's get the thread to break at invalidation points if needed.
VM& vm = this->vm();
void* trapPC = context.trapPC;
+ // We must ensure we're in JIT/LLint code. If we are, we know a few things:
+ // - The JS thread isn't holding the malloc lock. Therefore, it's safe to malloc below.
+ // - The JS thread isn't holding the CodeBlockSet lock.
+ // If we're not in JIT/LLInt code, we can't run the C++ code below because it
+ // mallocs, and we must prove the JS thread isn't holding the malloc lock
+ // to be able to do that without risking a deadlock.
+ if (!isJITPC(trapPC) && !LLInt::isLLIntPC(trapPC))
+ return;
CallFrame* callFrame = reinterpret_cast<CallFrame*>(context.framePointer);
- 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 codeBlockSetLocker = holdLock(vm.heap.codeBlockSet().getLock());
- 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;
EntryFrame* entryFrame = vm.topEntryFrame;
