From 290e2b55c47c59adb43c1b0cd9234aabb01968c3 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Thu, 10 Sep 2020 17:01:43 -0700 Subject: [PATCH 1/2] Fixes guest default signal handling and pending signals glibc raise() actually disables all signals, raises the signal that the user wants, then reenables the previous signal mask. On the reenabling of the previous signal mask, that is when the pending signal fires. This needs to be accurately handled, so check for pending signals getting unmasked in sigprocmask This also then needs to be further fixed to handle default signal handling, where if a signal was sent that is default ignored (NOT set to ignore!) then it would just exit the application before. Now something likg SIGCHLD with the default handler (aka ignore) won't kill the process --- .../Source/Interface/Core/SignalDelegator.cpp | 60 ++++++++++++++++++- .../Source/Interface/Core/SignalDelegator.h | 9 +++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/External/FEXCore/Source/Interface/Core/SignalDelegator.cpp b/External/FEXCore/Source/Interface/Core/SignalDelegator.cpp index f4fddbb7c..e32c33c79 100644 --- a/External/FEXCore/Source/Interface/Core/SignalDelegator.cpp +++ b/External/FEXCore/Source/Interface/Core/SignalDelegator.cpp @@ -7,6 +7,10 @@ #include +#include +#include +#include + namespace FEXCore { constexpr static uint32_t SS_AUTODISARM = (1U << 31); constexpr static uint32_t X86_MINSIGSTKSZ = 0x2000U; @@ -116,9 +120,25 @@ namespace FEXCore { // We have an emulation thread pointer, we can now modify its state if (Handler.GuestAction.sigaction_handler.handler == SIG_DFL) { - // XXX: Maybe this should actually go down guest handler state? - signal(Signal, SIG_DFL); - return; + if (Handler.DefaultBehaviour == DEFAULT_TERM) { + if (Thread->State.ThreadManager.clear_child_tid) { + std::atomic *Addr = reinterpret_cast*>(Thread->State.ThreadManager.clear_child_tid); + Addr->store(0); + syscall(SYS_futex, + Thread->State.ThreadManager.clear_child_tid, + FUTEX_WAKE, + ~0ULL, + 0, + 0, + 0); + } + + Thread->StatusCode = -Signal; + + // Doesn't return + Thread->CTX->StopThread(Thread); + std::unexpected(); + } } else if (Handler.GuestAction.sigaction_handler.handler == SIG_IGN) { return; @@ -238,6 +258,29 @@ namespace FEXCore { // "Userspace" SIGRTMIN starts at 34 because of this HostHandlers[__SIGRTMIN].Installed = true; HostHandlers[__SIGRTMIN+1].Installed = true; + + // Most signals default to termination + // These ones are slightly different + const std::vector> SignalDefaultBehaviours = { + {SIGQUIT, DEFAULT_COREDUMP}, + {SIGILL, DEFAULT_COREDUMP}, + {SIGTRAP, DEFAULT_COREDUMP}, + {SIGABRT, DEFAULT_COREDUMP}, + {SIGBUS, DEFAULT_COREDUMP}, + {SIGFPE, DEFAULT_COREDUMP}, + {SIGSEGV, DEFAULT_COREDUMP}, + {SIGCHLD, DEFAULT_IGNORE}, + {SIGCONT, DEFAULT_IGNORE}, + {SIGURG, DEFAULT_IGNORE}, + {SIGXCPU, DEFAULT_COREDUMP}, + {SIGXFSZ, DEFAULT_COREDUMP}, + {SIGSYS, DEFAULT_COREDUMP}, + {SIGWINCH, DEFAULT_IGNORE}, + }; + + for (auto Behaviour : SignalDefaultBehaviours) { + HostHandlers[Behaviour.first].DefaultBehaviour = Behaviour.second; + } } void SignalDelegator::RegisterTLSState(FEXCore::Core::InternalThreadState *Thread) { @@ -438,6 +481,17 @@ namespace FEXCore { } } + // Do we have any pending signals that became unmasked? + uint64_t PendingSignals = ~ThreadData.GuestProcMask & ThreadData.PendingSignals; + if (PendingSignals != 0) { + for (int i = 0; i < 64; ++i) { + if (PendingSignals & (1ULL << i)) { + tgkill(ThreadData.Thread->State.ThreadManager.PID, ThreadData.Thread->State.ThreadManager.TID, i + 1); + PendingSignals &= ~(1ULL << i); + } + } + } + return 0; } diff --git a/External/FEXCore/Source/Interface/Core/SignalDelegator.h b/External/FEXCore/Source/Interface/Core/SignalDelegator.h index 9b1de9293..bb9202747 100644 --- a/External/FEXCore/Source/Interface/Core/SignalDelegator.h +++ b/External/FEXCore/Source/Interface/Core/SignalDelegator.h @@ -97,6 +97,14 @@ namespace Core { constexpr static size_t SIGNAL_FOR_PAUSE {63}; private: + enum DefaultBehaviour { + DEFAULT_TERM, + // Core dump based signals are supposed to have a coredump appear + // For FEX's behaviour we don't really care right now + DEFAULT_COREDUMP = DEFAULT_TERM, + DEFAULT_IGNORE, + }; + struct SignalHandler { std::atomic Installed{}; struct sigaction HostAction{}; @@ -104,6 +112,7 @@ namespace Core { HostSignalDelegatorFunction Handler{}; HostSignalDelegatorFunctionForGuest GuestHandler{}; GuestSigAction GuestAction{}; + DefaultBehaviour DefaultBehaviour {DEFAULT_TERM}; }; SignalHandler HostHandlers[MAX_SIGNALS + 1]{}; From 4f593f8e88c158317e008fcb7f9e21ece3bd7753 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Thu, 10 Sep 2020 18:12:45 -0700 Subject: [PATCH 2/2] Updates POSIX unit tests results These have changed behaviour due to the new changes --- unittests/POSIX/Disabled_Tests | 5 +++++ unittests/POSIX/Known_Failures | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/unittests/POSIX/Disabled_Tests b/unittests/POSIX/Disabled_Tests index 0695668a1..785af3fb0 100644 --- a/unittests/POSIX/Disabled_Tests +++ b/unittests/POSIX/Disabled_Tests @@ -1,5 +1,8 @@ # These tests are inconsistent conformance-interfaces-mmap-12-1.test +conformance-interfaces-mmap-6-1.test +conformance-interfaces-mmap-6-2.test +conformance-interfaces-mmap-6-3.test # These tests take too long to run, and might timeout conformance-interfaces-clock-1-1.test @@ -545,3 +548,5 @@ conformance-interfaces-mmap-11-2.test conformance-interfaces-munmap-1-1.test conformance-interfaces-munmap-1-2.test +conformance-interfaces-killpg-1-2.test # uses rt_sigsuspend +conformance-interfaces-kill-1-2.test # uses rt_sigtimedwait diff --git a/unittests/POSIX/Known_Failures b/unittests/POSIX/Known_Failures index aa9b4f96f..7b43a64a8 100644 --- a/unittests/POSIX/Known_Failures +++ b/unittests/POSIX/Known_Failures @@ -11,7 +11,6 @@ conformance-interfaces-munmap-2-1.test conformance-interfaces-nanosleep-10000-1.test conformance-interfaces-nanosleep-5-1.test conformance-interfaces-nanosleep-6-1.test -conformance-interfaces-raise-1-1.test conformance-interfaces-sigpending-1-2.test conformance-interfaces-sigpending-1-3.test conformance-interfaces-sigprocmask-6-1.test @@ -46,8 +45,6 @@ conformance-interfaces-sigwaitinfo-8-1.test conformance-interfaces-sigwaitinfo-9-1.test conformance-interfaces-clock_nanosleep-10-1.test # uses sigabort conformance-interfaces-clock_nanosleep-9-1.test # uses sigabort -conformance-interfaces-kill-1-2.test # uses rt_sigtimedwait -conformance-interfaces-killpg-1-2.test # uses getpgid, setpgid conformance-interfaces-nanosleep-5-2.test # uses sigabort conformance-interfaces-nanosleep-7-1.test # uses sigabort conformance-interfaces-nanosleep-7-2.test # uses sigabort