Crash in JavaScriptCore GC when using JSC on dispatch queues (thread_get_state return...

https://bugs.webkit.org/show_bug.cgi?id=160337
<rdar://problem/27611733>

Reviewed by Filip Pizlo and Geoffrey Garen.

This is a workaround for <rdar://problem/27607384>. During thread initialization,
for some target platforms, thread state is momentarily set to 0 before being
filled in with the target thread's real register values. As a result, there's
a race condition that may result in us getting a null stackPointer during a GC scan.
This issue may manifest with workqueue threads where the OS may choose to recycle
a thread for an expired task.

The workaround is simply to indicate that there's nothing to copy and return.
This is correct because we will only ever observe a null pointer during thread
initialization. Hence, by definition, there's nothing there that we need to scan
yet, and therefore, nothing that needs to be copied.

* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::tryCopyOtherThreadStack):

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 46f130f..c0bba6c 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,26 @@
+2017-05-10  Mark Lam  <mark.lam@apple.com>
+
+        Crash in JavaScriptCore GC when using JSC on dispatch queues (thread_get_state returns NULL stack pointer).
+        https://bugs.webkit.org/show_bug.cgi?id=160337
+        <rdar://problem/27611733>
+
+        Reviewed by Filip Pizlo and Geoffrey Garen.
+
+        This is a workaround for <rdar://problem/27607384>. During thread initialization,
+        for some target platforms, thread state is momentarily set to 0 before being
+        filled in with the target thread's real register values. As a result, there's
+        a race condition that may result in us getting a null stackPointer during a GC scan.
+        This issue may manifest with workqueue threads where the OS may choose to recycle
+        a thread for an expired task.
+
+        The workaround is simply to indicate that there's nothing to copy and return.
+        This is correct because we will only ever observe a null pointer during thread
+        initialization. Hence, by definition, there's nothing there that we need to scan
+        yet, and therefore, nothing that needs to be copied.
+
+        * heap/MachineStackMarker.cpp:
+        (JSC::MachineThreads::tryCopyOtherThreadStack):
+
 2017-05-10  JF Bastien  <jfbastien@apple.com>
 
         WebAssembly: support name section

diff --git a/Source/JavaScriptCore/heap/MachineStackMarker.cpp b/Source/JavaScriptCore/heap/MachineStackMarker.cpp
index 68b5b50..8c7ac54 100644 (file)
--- a/Source/JavaScriptCore/heap/MachineStackMarker.cpp
+++ b/Source/JavaScriptCore/heap/MachineStackMarker.cpp
@@ -311,6 +311,23 @@ void MachineThreads::tryCopyOtherThreadStack(MachineThread* thread, void* buffer
 {
     MachineThread::Registers registers;
     size_t registersSize = thread->getRegisters(registers);
+
+    // This is a workaround for <rdar://problem/27607384>. During thread initialization,
+    // for some target platforms, thread state is momentarily set to 0 before being
+    // filled in with the target thread's real register values. As a result, there's
+    // a race condition that may result in us getting a null stackPointer.
+    // This issue may manifest with workqueue threads where the OS may choose to recycle
+    // a thread for an expired task.
+    //
+    // The workaround is simply to indicate that there's nothing to copy and return.
+    // This is correct because we will only ever observe a null pointer during thread
+    // initialization. Hence, by definition, there's nothing there that we need to scan
+    // yet, and therefore, nothing that needs to be copied.
+    if (UNLIKELY(!registers.stackPointer())) {
+        *size = 0;
+        return;
+    }
+
     std::pair<void*, size_t> stack = thread->captureStack(registers.stackPointer());
 
     bool canCopy = *size + registersSize + stack.second <= capacity;