[linux] ASSERT: Using an alternative signal stack is not supported. Consider disablin...
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Jul 2018 07:10:20 +0000 (07:10 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Jul 2018 07:10:20 +0000 (07:10 +0000)
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.

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

Source/WTF/ChangeLog
Source/WTF/wtf/ThreadingPthreads.cpp

index 09beca7..b75ebc0 100644 (file)
@@ -1,3 +1,25 @@
+2018-07-07  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [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  <fwang@igalia.com>
 
         WTF's internal std::optional implementation should abort() on bad optional access
index 6aa83ff..dd01c6c 100644 (file)
@@ -119,10 +119,11 @@ static std::atomic<Thread*> targetThread { nullptr };
 #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 @@ static UNUSED_FUNCTION NEVER_INLINE void* getApproximateStackPointer()
 #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,14 +149,25 @@ void Thread::signalHandlerSuspendResume(int, siginfo_t*, void* ucontext)
         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);
     thread->m_platformRegisters = &registersFromUContext(userContext);
 #else
     UNUSED_PARAM(ucontext);
-    PlatformRegisters platformRegisters { getApproximateStackPointer() };
+    PlatformRegisters platformRegisters { approximateStackPointer };
     thread->m_platformRegisters = &platformRegisters;
 #endif
 
@@ -354,10 +358,18 @@ auto Thread::suspend() -> Expected<void, PlatformSuspendError>
         // 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 { };