[webkit-changes] [233613] trunk/Source/WTF

Sat, 07 Jul 2018 00:11:07 -0700

Title: [233613] trunk/Source/WTF
Revision
233613
Author
[email protected]
Date
2018-07-07 00:10:20 -0700 (Sat, 07 Jul 2018)

Log Message

[linux] ASSERT: Using an alternative signal stack is not supported. Consider disabling the concurrent GC.
https://bugs.webkit.org/show_bug.cgi?id=187297

Reviewed by Mark Lam.

This patch relaxes the JSC's limitation: accepting an alternative signal stack mechanism.

* wtf/ThreadingPthreads.cpp:
(WTF::getApproximateStackPointer):
Fix approximate stack pointer function to make it valid.

(WTF::Thread::signalHandlerSuspendResume):
Use StackBounds::contains to check whether the given stack pointer is in range of StackBounds.
If it is out of range, it seems that this stack pointer is pointing an alternative signal stack.

(WTF::Thread::suspend):
Repeatedly retry suspension by using Thread::yield().

(WTF::isOnAlternativeSignalStack): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (233612 => 233613)


--- trunk/Source/WTF/ChangeLog	2018-07-07 03:43:04 UTC (rev 233612)
+++ trunk/Source/WTF/ChangeLog	2018-07-07 07:10:20 UTC (rev 233613)
@@ -1,3 +1,25 @@
+2018-07-07  Yusuke Suzuki  <[email protected]>
+
+        [linux] ASSERT: Using an alternative signal stack is not supported. Consider disabling the concurrent GC.
+        https://bugs.webkit.org/show_bug.cgi?id=187297
+
+        Reviewed by Mark Lam.
+
+        This patch relaxes the JSC's limitation: accepting an alternative signal stack mechanism.
+
+        * wtf/ThreadingPthreads.cpp:
+        (WTF::getApproximateStackPointer):
+        Fix approximate stack pointer function to make it valid.
+
+        (WTF::Thread::signalHandlerSuspendResume):
+        Use StackBounds::contains to check whether the given stack pointer is in range of StackBounds.
+        If it is out of range, it seems that this stack pointer is pointing an alternative signal stack.
+
+        (WTF::Thread::suspend):
+        Repeatedly retry suspension by using Thread::yield().
+
+        (WTF::isOnAlternativeSignalStack): Deleted.
+
 2018-07-06  Frederic Wang  <[email protected]>
 
         WTF's internal std::optional implementation should abort() on bad optional access

Modified: trunk/Source/WTF/wtf/ThreadingPthreads.cpp (233612 => 233613)


--- trunk/Source/WTF/wtf/ThreadingPthreads.cpp	2018-07-07 03:43:04 UTC (rev 233612)
+++ trunk/Source/WTF/wtf/ThreadingPthreads.cpp	2018-07-07 07:10:20 UTC (rev 233613)
@@ -119,10 +119,11 @@
 #pragma clang diagnostic ignored "-Wreturn-stack-address"
 #endif // COMPILER(CLANG)
 
-static UNUSED_FUNCTION NEVER_INLINE void* getApproximateStackPointer()
+static NEVER_INLINE void* getApproximateStackPointer()
 {
-    volatile void* stackLocation = nullptr;
-    return &stackLocation;
+    volatile uintptr_t stackLocation;
+    stackLocation = bitwise_cast<uintptr_t>(&stackLocation);
+    return bitwise_cast<void*>(stackLocation);
 }
 
 #if COMPILER(GCC)
@@ -133,14 +134,6 @@
 #pragma clang diagnostic pop
 #endif // COMPILER(CLANG)
 
-static UNUSED_FUNCTION bool isOnAlternativeSignalStack()
-{
-    stack_t stack { };
-    int ret = sigaltstack(nullptr, &stack);
-    RELEASE_ASSERT(!ret);
-    return stack.ss_flags == SS_ONSTACK;
-}
-
 void Thread::signalHandlerSuspendResume(int, siginfo_t*, void* ucontext)
 {
     // Touching a global variable atomic types from signal handlers is allowed.
@@ -156,7 +149,18 @@
         return;
     }
 
-    ASSERT_WITH_MESSAGE(!isOnAlternativeSignalStack(), "Using an alternative signal stack is not supported. Consider disabling the concurrent GC.");
+    void* approximateStackPointer = getApproximateStackPointer();
+    if (!thread->m_stack.contains(approximateStackPointer)) {
+        // This happens if we use an alternative signal stack.
+        // 1. A user-defined signal handler is invoked with an alternative signal stack.
+        // 2. In the middle of the execution of the handler, we attempt to suspend the target thread.
+        // 3. A nested signal handler is executed.
+        // 4. The stack pointer saved in the machine context will be pointing to the alternative signal stack.
+        // In this case, we back off the suspension and retry a bit later.
+        thread->m_platformRegisters = nullptr;
+        globalSemaphoreForSuspendResume->post();
+        return;
+    }
 
 #if HAVE(MACHINE_CONTEXT)
     ucontext_t* userContext = static_cast<ucontext_t*>(ucontext);
@@ -163,7 +167,7 @@
     thread->m_platformRegisters = &registersFromUContext(userContext);
 #else
     UNUSED_PARAM(ucontext);
-    PlatformRegisters platformRegisters { getApproximateStackPointer() };
+    PlatformRegisters platformRegisters { approximateStackPointer };
     thread->m_platformRegisters = &platformRegisters;
 #endif
 
@@ -354,10 +358,18 @@
         // But it can be used in a few platforms, like Linux.
         // Instead, we use Thread* stored in a global variable to pass it to the signal handler.
         targetThread.store(this);
-        int result = pthread_kill(m_handle, SigThreadSuspendResume);
-        if (result)
-            return makeUnexpected(result);
-        globalSemaphoreForSuspendResume->wait();
+
+        while (true) {
+            int result = pthread_kill(m_handle, SigThreadSuspendResume);
+            if (result)
+                return makeUnexpected(result);
+            globalSemaphoreForSuspendResume->wait();
+            if (m_platformRegisters)
+                break;
+            // Because of an alternative signal stack, we failed to suspend this thread.
+            // Retry suspension again after yielding.
+            Thread::yield();
+        }
     }
     ++m_suspendCount;
     return { };

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to