Attachment 270116 Details for Bug 153584 – Patch

[patch] Patch

bug-153584-20160129012927.patch (text/plain), 18.34 KB, created by Yusuke Suzuki on 2016-01-28 08:29:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2016-01-28 08:29:53 PST

Size: 18.34 KB

patch

obsolete

>Subversion Revision: 195725
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 9acd84502cf52fefa2c1e4f45f4db7635411405f..0bfa10ea146ed67caa683a4ca496d4640683ecc7 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,43 @@
>+2016-01-28  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Enable SamplingProfiler on POSIX environment
>+        https://bugs.webkit.org/show_bug.cgi?id=153584
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        In this patch, we implement suspend and resume mechanizm for POSIX threads.
>+        And with GLIBC, we can retrieve registers from it.
>+
>+        We take the following strategy.
>+
>+        Suspend side.
>+        1. install sigaction to the threads.
>+        2. in the profiler (suspend / resume callers), emit signal with pthread_kill and wait with POSIX semaphore.
>+        3. in the signal handler, up the POSIX semaphore. Use sem_post because it is the async-signal-safe function in POSIX.
>+        4. in the signal handler, perform sigsuspend to stop the thread until being resumed.
>+        5. in the profiler, we can be waken up from the semaphore because (3) ups.
>+
>+        Resume side.
>+        1. in the profiler, emit signal and wait on the semaphore.
>+        2. in the signal handler, it is waken up from the sigsuspend.
>+        3. in the signal handler, up the semaphore.
>+        4. in the profiler, the profiler is waken up from the semaphore. It is ensured that the given thread is resumed by the signal.
>+
>+        * heap/MachineStackMarker.cpp:
>+        (pthreadSignalHandlerSuspendResume):
>+        (JSC::MachineThreads::Thread::Thread):
>+        (JSC::MachineThreads::Thread::~Thread):
>+        (JSC::MachineThreads::Thread::suspend):
>+        (JSC::MachineThreads::Thread::resume):
>+        (JSC::MachineThreads::Thread::getRegisters):
>+        (JSC::MachineThreads::Thread::Registers::stackPointer):
>+        (JSC::MachineThreads::Thread::Registers::framePointer):
>+        (JSC::MachineThreads::Thread::Registers::instructionPointer):
>+        (JSC::MachineThreads::Thread::freeRegisters):
>+        * heap/MachineStackMarker.h:
>+        * runtime/SamplingProfiler.cpp:
>+        (JSC::reportStats):
>+
> 2016-01-27  Benjamin Poulain  <bpoulain@apple.com>
> 
>         [JSC] adjustFrameAndStackInOSRExitCompilerThunk() can trash values in FTL
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 9b9c8c20574b355f38a2850a8382bf31b942ec73..6bfe4cbe8aeb6626dc3ee84baa5c053d1334612f 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,14 @@
>+2016-01-28  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Enable SamplingProfiler on POSIX environment
>+        https://bugs.webkit.org/show_bug.cgi?id=153584
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Use __GLIBC__ since mcontext_t layout depends on it.
>+
>+        * wtf/Platform.h:
>+
> 2016-01-27  Chris Dumez  <cdumez@apple.com>
> 
>         window.atob() should ignore spaces in input
>diff --git a/Source/JavaScriptCore/heap/MachineStackMarker.cpp b/Source/JavaScriptCore/heap/MachineStackMarker.cpp
>index f8ad5541e318aea83d7c52883a0b952efbfca042..77376b7de50045f20177228f22bc726602c9a6e0 100644
>--- a/Source/JavaScriptCore/heap/MachineStackMarker.cpp
>+++ b/Source/JavaScriptCore/heap/MachineStackMarker.cpp
>@@ -62,17 +62,42 @@
> #if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN)
> #include <signal.h>
> 
>+// We use SIGUSR2 to suspend and resume machine threads in JavaScriptCore.
> static const int SigThreadSuspendResume = SIGUSR2;
>+thread_local static std::atomic<JSC::MachineThreads::Thread*> threadLocalCurrentThread;
> 
>-#if defined(SA_RESTART)
>-static void pthreadSignalHandlerSuspendResume(int)
>+static void pthreadSignalHandlerSuspendResume(int, siginfo_t*, void* ucontext)
> {
>-    sigset_t signalSet;
>-    sigemptyset(&signalSet);
>-    sigaddset(&signalSet, SigThreadSuspendResume);
>-    sigsuspend(&signalSet);
>+    // Touching thread local atomic types from signal handlers is allowed.
>+    JSC::MachineThreads::Thread* thread = threadLocalCurrentThread.load();
>+
>+    if (thread->suspended.load(std::memory_order_acquire)) {
>+        // This is signal handler invocation that is intended to be used to resume sigsuspend.
>+        // So this handler invocation itself should not process.
>+        return;
>+    }
>+
>+    struct ucontext* userContext = static_cast<struct ucontext*>(ucontext);
>+    thread->suspendedMachineContext = userContext->uc_mcontext;
>+
>+    // Allow suspend caller to see that this thread is suspended.
>+    // sem_post is async-signal-safe function. It means that we can call this from a signal handler.
>+    // http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html#tag_02_04_03
>+    //
>+    // And sem_post emits memory barrier that ensures that suspendedMachineContext is correctly saved.
>+    // http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_11
>+    sem_post(&thread->semaphoreForSuspendResume);
>+
>+    // Reaching here, SigThreadSuspendResume is blocked in this handler (this is configured by sigaction's sa_mask).
>+    // So before calling sigsuspend, SigThreadSuspendResume to this thread is deferred. This ensures that the handler is not executed recursively.
>+    sigset_t blockedSignalSet;
>+    sigfillset(&blockedSignalSet);
>+    sigdelset(&blockedSignalSet, SigThreadSuspendResume);
>+    sigsuspend(&blockedSignalSet);
>+
>+    // Allow resume caller to see that this thread is resumed.
>+    sem_post(&thread->semaphoreForSuspendResume);
> }
>-#endif // defined(SA_RESTART)
> #endif // USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN)
> 
> #endif
>@@ -296,24 +321,35 @@ void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoot
>     , stackBase(base)
>     , stackEnd(end)
> {
>-#if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN) && defined(SA_RESTART)
>-    // if we have SA_RESTART, enable SIGUSR2 debugging mechanism
>-    struct sigaction action;
>-    action.sa_handler = pthreadSignalHandlerSuspendResume;
>-    sigemptyset(&action.sa_mask);
>-    action.sa_flags = SA_RESTART;
>-    sigaction(SigThreadSuspendResume, &action, 0);
>-
>-    sigset_t mask;
>-    sigemptyset(&mask);
>-    sigaddset(&mask, SigThreadSuspendResume);
>-    pthread_sigmask(SIG_UNBLOCK, &mask, 0);
>-#elif OS(WINDOWS)
>+#if OS(WINDOWS)
>     ASSERT(platformThread == GetCurrentThreadId());
>     bool isSuccessful =
>         DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(),
>             &platformThreadHandle, 0, FALSE, DUPLICATE_SAME_ACCESS);
>     RELEASE_ASSERT(isSuccessful);
>+#elif USE(PTHREADS) && !OS(DARWIN)
>+    threadLocalCurrentThread.store(this);
>+
>+    // Signal handlers are process global configuration.
>+    static std::once_flag initializeSignalHandler;
>+    std::call_once(initializeSignalHandler, [] {
>+        // Intentionally block SigThreadSuspendResume in the handler.
>+        // SigThreadSuspendResume will be allowed in the handler by sigsuspend.
>+        struct sigaction action;
>+        sigemptyset(&action.sa_mask);
>+        sigaddset(&action.sa_mask, SigThreadSuspendResume);
>+
>+        action.sa_sigaction = pthreadSignalHandlerSuspendResume;
>+        action.sa_flags = SA_RESTART | SA_SIGINFO;
>+        sigaction(SigThreadSuspendResume, &action, 0);
>+    });
>+
>+    sigset_t mask;
>+    sigemptyset(&mask);
>+    sigaddset(&mask, SigThreadSuspendResume);
>+    pthread_sigmask(SIG_UNBLOCK, &mask, 0);
>+
>+    sem_init(&semaphoreForSuspendResume, /* Only available in this process. */ 0, /* Initial value for the semaphore. */ 0);
> #endif
> }
> 
>@@ -321,6 +357,8 @@ void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoot
> {
> #if OS(WINDOWS)
>     CloseHandle(platformThreadHandle);
>+#elif USE(PTHREADS) && !OS(DARWIN)
>+    sem_destroy(&semaphoreForSuspendResume);
> #endif
> }
> 
>@@ -334,7 +372,21 @@ bool MachineThreads::Thread::suspend()
>     ASSERT(threadIsSuspended);
>     return threadIsSuspended;
> #elif USE(PTHREADS)
>-    pthread_kill(platformThread, SigThreadSuspendResume);
>+    {
>+        // During suspend, suspend or resume should not be executed from the other threads.
>+        LockHolder lock(suspendLock);
>+        if (!suspendCount) {
>+            // Ideally, we would like to use pthread_sigqueue. It allows us to pass the argument to the signal handler.
>+            // But it can be used in a few platforms, like Linux.
>+            // Instead, we use Thread* stored in the thread local storage to pass it to the signal handler.
>+            if (pthread_kill(platformThread, SigThreadSuspendResume) == ESRCH)
>+                return false;
>+            sem_wait(&semaphoreForSuspendResume);
>+            // Release barrier ensures that this operation is always executed after all the above processing is done.
>+            suspended.store(true, std::memory_order_release);
>+        }
>+        ++suspendCount;
>+    }
>     return true;
> #else
> #error Need a way to suspend threads on this platform
>@@ -348,7 +400,25 @@ void MachineThreads::Thread::resume()
> #elif OS(WINDOWS)
>     ResumeThread(platformThreadHandle);
> #elif USE(PTHREADS)
>-    pthread_kill(platformThread, SigThreadSuspendResume);
>+    {
>+        // During resume, suspend or resume should not be executed from the other threads.
>+        LockHolder lock(suspendLock);
>+        if (suspendCount == 1) {
>+            // When allowing SigThreadSuspendResume interrupt in the signal handler by sigsuspend and SigThreadSuspendResume is actually issued,
>+            // the signal handler itself will be called once again.
>+            // There are several ways to distinguish the handler invocation for suspend and resume.
>+            // 1. Use different signal numbers. And check the signal number in the handler.
>+            // 2. Use some arguments to distinguish suspend and resume in the handler. If pthread_sigqueue can be used, we can take this.
>+            // 3. Use thread local storage with atomic variables in the signal handler.
>+            // In this implementaiton, we take (3). suspended flag is used to distinguish it.
>+            if (pthread_kill(platformThread, SigThreadSuspendResume) == ESRCH)
>+                return;
>+            sem_wait(&semaphoreForSuspendResume);
>+            // Release barrier ensures that this operation is always executed after all the above processing is done.
>+            suspended.store(false, std::memory_order_release);
>+        }
>+        --suspendCount;
>+    }
> #else
> #error Need a way to resume threads on this platform
> #endif
>@@ -394,16 +464,17 @@ size_t MachineThreads::Thread::getRegisters(Thread::Registers& registers)
>     GetThreadContext(platformThreadHandle, &regs);
>     return sizeof(CONTEXT);
> #elif USE(PTHREADS)
>-    pthread_attr_init(&regs);
>+    pthread_attr_init(&regs.attribute);
> #if HAVE(PTHREAD_NP_H) || OS(NETBSD)
> #if !OS(OPENBSD)
>     // e.g. on FreeBSD 5.4, neundorf@kde.org
>-    pthread_attr_get_np(platformThread, &regs);
>+    pthread_attr_get_np(platformThread, &regs.attribute);
> #endif
> #else
>     // FIXME: this function is non-portable; other POSIX systems may have different np alternatives
>-    pthread_getattr_np(platformThread, &regs);
>+    pthread_getattr_np(platformThread, &regs.attribute);
> #endif
>+    regs.machineContext = suspendedMachineContext;
>     return 0;
> #else
> #error Need a way to get thread registers on this platform
>@@ -468,7 +539,7 @@ void* MachineThreads::Thread::Registers::stackPointer() const
>     stackBase = (void*)((size_t) ss.ss_sp - ss.ss_size);
>     stackSize = ss.ss_size;
> #else
>-    int rc = pthread_attr_getstack(&regs, &stackBase, &stackSize);
>+    int rc = pthread_attr_getstack(&regs.attribute, &stackBase, &stackSize);
> #endif
>     (void)rc; // FIXME: Deal with error code somehow? Seems fatal.
>     ASSERT(stackBase);
>@@ -524,6 +595,23 @@ void* MachineThreads::Thread::Registers::framePointer() const
> #error Unknown Architecture
> #endif
> 
>+#elif defined(__GLIBC__)
>+
>+// The following sequence depends on glibc's sys/ucontext.h.
>+#if CPU(X86)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.gregs[REG_EBP]);
>+#elif CPU(X86_64)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.gregs[REG_RBP]);
>+#elif CPU(ARM)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.arm_fp);
>+#elif CPU(ARM64)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.regs[29]);
>+#elif CPU(MIPS)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.gregs[30]);
>+#else
>+#error Unknown Architecture
>+#endif
>+
> #else
> #error Need a way to get the frame pointer for another thread on this platform
> #endif
>@@ -573,6 +661,23 @@ void* MachineThreads::Thread::Registers::instructionPointer() const
> #error Unknown Architecture
> #endif
> 
>+#elif defined(__GLIBC__)
>+
>+// The following sequence depends on glibc's sys/ucontext.h.
>+#if CPU(X86)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.gregs[REG_EIP]);
>+#elif CPU(X86_64)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.gregs[REG_RIP]);
>+#elif CPU(ARM)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.arm_pc);
>+#elif CPU(ARM64)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.pc);
>+#elif CPU(MIPS)
>+    return reinterpret_cast<void*>((uintptr_t) regs.machineContext.pc);
>+#else
>+#error Unknown Architecture
>+#endif
>+
> #else
> #error Need a way to get the instruction pointer for another thread on this platform
> #endif
>@@ -583,7 +688,7 @@ void MachineThreads::Thread::freeRegisters(Thread::Registers& registers)
> {
>     Thread::Registers::PlatformRegisters& regs = registers.regs;
> #if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN)
>-    pthread_attr_destroy(&regs);
>+    pthread_attr_destroy(&regs.attribute);
> #else
>     UNUSED_PARAM(regs);
> #endif
>diff --git a/Source/JavaScriptCore/heap/MachineStackMarker.h b/Source/JavaScriptCore/heap/MachineStackMarker.h
>index 4b6a6eefe40ee686120b313f7df3098af9d106a3..1dde6ace7f9afb8b8a0488c1e32b3e020183e527 100644
>--- a/Source/JavaScriptCore/heap/MachineStackMarker.h
>+++ b/Source/JavaScriptCore/heap/MachineStackMarker.h
>@@ -31,6 +31,11 @@
> #include <mach/thread_act.h>
> #endif
> 
>+#if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN)
>+#include <semaphore.h>
>+#include <signal.h>
>+#endif
>+
> #if OS(DARWIN)
> typedef mach_port_t PlatformThread;
> #elif OS(WINDOWS)
>@@ -94,7 +99,10 @@ class MachineThreads {
> #elif OS(WINDOWS)
>             typedef CONTEXT PlatformRegisters;
> #elif USE(PTHREADS)
>-            typedef pthread_attr_t PlatformRegisters;
>+            struct PlatformRegisters {
>+                pthread_attr_t attribute;
>+                mcontext_t machineContext;
>+            };
> #else
> #error Need a thread register struct for this platform
> #endif
>@@ -117,6 +125,12 @@ class MachineThreads {
>         void* stackEnd;
> #if OS(WINDOWS)
>         HANDLE platformThreadHandle;
>+#elif USE(PTHREADS) && !OS(DARWIN)
>+        sem_t semaphoreForSuspendResume;
>+        mcontext_t suspendedMachineContext;
>+        Lock suspendLock;
>+        int suspendCount;
>+        std::atomic<bool> suspended { false };
> #endif
>     };
> 
>diff --git a/Source/JavaScriptCore/runtime/SamplingProfiler.cpp b/Source/JavaScriptCore/runtime/SamplingProfiler.cpp
>index 33b3dfbd039baafc69f2d686306f79f37cc9016e..fc8be20eb4950da8da6142817bf6e8284b38125a 100644
>--- a/Source/JavaScriptCore/runtime/SamplingProfiler.cpp
>+++ b/Source/JavaScriptCore/runtime/SamplingProfiler.cpp
>@@ -64,9 +64,9 @@ ALWAYS_INLINE static void reportStats()
>     if (sReportStats && sNumTotalWalks && static_cast<uint64_t>(sNumTotalWalks) % sNumWalkReportingFrequency == 0) {
>         if (!sReportStatsOnlyWhenTheyreAboveThreshold || (sNumFailedWalks / sNumTotalWalks > sWalkErrorPercentage)) {
>             dataLogF("Num total walks: %llu. Failed walks percent: %lf\n",
>-                static_cast<uint64_t>(sNumTotalWalks), sNumFailedWalks / sNumTotalWalks);
>+                static_cast<unsigned long long>(sNumTotalWalks), sNumFailedWalks / sNumTotalWalks);
>             dataLogF("Total stack traces: %llu. Needs verification percent: %lf\n",
>-                static_cast<uint64_t>(sNumTotalStackTraces), sNumUnverifiedStackTraces / sNumTotalStackTraces);
>+                static_cast<unsigned long long>(sNumTotalStackTraces), sNumUnverifiedStackTraces / sNumTotalStackTraces);
>         }
>     }
> }
>diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
>index eb96fd621d3e3142620131c58db2081373bc2237..121efa5124c3193c7373a615dc65c9f32bdad683 100644
>--- a/Source/WTF/wtf/Platform.h
>+++ b/Source/WTF/wtf/Platform.h
>@@ -426,6 +426,12 @@
> 
> /* Operating environments */
> 
>+/* Standard libraries */
>+#if defined(HAVE_FEATURES_H)
>+/* If the included features.h is glibc's one, __GLIBC__ is defined. */
>+#include <features.h>
>+#endif
>+
> /* FIXME: these are all mixes of OS, operating environment and policy choices. */
> /* PLATFORM(EFL) */
> /* PLATFORM(GTK) */
>@@ -798,8 +804,10 @@
> #endif
> 
> /* The SamplingProfiler is the probabilistic and low-overhead profiler used by
>- * JSC to measure where time is spent inside a JavaScript program. */
>-#if (OS(DARWIN) || OS(WINDOWS)) && ENABLE(JIT)
>+ * JSC to measure where time is spent inside a JavaScript program.
>+ * In configurations other than Windows and Darwin, because layout of mcontext_t depends on standard libraries (like glibc),
>+ * sampling profiler is enabled if WebKit uses pthreads and glibc. */
>+#if (OS(DARWIN) || OS(WINDOWS) || (USE(PTHREADS) && defined(__GLIBC__))) && ENABLE(JIT)
> #define ENABLE_SAMPLING_PROFILER 1
> #else
> #define ENABLE_SAMPLING_PROFILER 0
>diff --git a/Source/cmake/OptionsCommon.cmake b/Source/cmake/OptionsCommon.cmake
>index 339c57bb80245d137f1c688f24f0fd10dd1583dd..46d695e9291e8e7db2d066091360983054188f06 100644
>--- a/Source/cmake/OptionsCommon.cmake
>+++ b/Source/cmake/OptionsCommon.cmake
>@@ -147,3 +147,9 @@ endif ()
> if (NOT APPLE)
>     set(CMAKE_NINJA_FORCE_RESPONSE_FILE 1)
> endif ()
>+
>+# Check whether features.h header exists.
>+# Including glibc's one defines __GLIBC__, that is used in Platform.h
>+include(CheckIncludeFiles)
>+check_include_files(features.h HAVE_FEATURES_H)
>+SET_AND_EXPOSE_TO_BUILD(HAVE_FEATURES_H ${HAVE_FEATURES_H})
>diff --git a/ChangeLog b/ChangeLog
>index d2edc6299ce0ff3c48f76121d507118169f9ff22..91f4dcea0fefe5ff31125ee5430e466a546aca9c 100644
>--- a/ChangeLog
>+++ b/ChangeLog
>@@ -1,3 +1,14 @@
>+2016-01-28  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Enable SamplingProfiler on POSIX environment
>+        https://bugs.webkit.org/show_bug.cgi?id=153584
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add features.h header check. It will define __GLIBC__.
>+
>+        * Source/cmake/OptionsCommon.cmake:
>+
> 2016-01-27  Alexey Proskuryakov  <ap@apple.com>
> 
>         Remove ENABLE_CURRENTSRC

Actions: View | Formatted Diff | Diff

Attachments on bug 153584: 270116 | 270125 | 270304