| Message ID | 20200201122746.1478003-1-laurent@vivier.eu |
|---|---|
| Headers | show |
| Series | linux-user: fix use of SIGRTMIN | expand |
FWIW, this removes the need for the target-specific code for Hexagon in signal.c. Thanks, Taylor PS Stay tuned for a Hexagon target patch series once this is merged. > -----Original Message----- > From: Laurent Vivier <laurent@vivier.eu> > Sent: Saturday, February 1, 2020 6:28 AM > To: qemu-devel@nongnu.org > Cc: Josh Kunz <jkz@google.com>; milos.stojanovic@rt-rk.com; Matus Kysel > <mkysel@tachyum.com>; Aleksandar Markovic <aleksandar.markovic@rt- > rk.com>; Marlies Ruck <marlies.ruck@gmail.com>; Laurent Vivier > <laurent@vivier.eu>; Peter Maydell <peter.maydell@linaro.org>; Taylor > Simpson <tsimpson@quicinc.com>; Riku Voipio <riku.voipio@iki.fi> > Subject: [PATCH 0/4] linux-user: fix use of SIGRTMIN > > This series fixes the problem of the first real-time signals already in use by > the glibc that are not available for the target glibc. > > Instead of reverting the first and last real-time signals we rely on the value > provided by the glibc (SIGRTMIN) to know the first available signal and we > map all the signals from this value to SIGRTMAX on top of > TARGET_SIGRTMIN. So the consequence is we have less available signals in > the target (generally 2) but all seems fine as at least 30 signals are still > available. > > This has been tested with Go (golang 1.10.1 linux/arm64, bionic) on x86_64 > fedora 31. We can avoid the failure in this case allowing the unsupported > signals when we don't provide the "act" parameters to sigaction, only the > "oldact" one. I have also run the LTP suite with several target and debian > based distros. > > Laurent Vivier (4): > linux-user: add missing TARGET_SIGRTMIN for hppa > linux-user: cleanup signal.c > linux-user: fix TARGET_NSIG and _NSIG uses > linux-user: fix use of SIGRTMIN > > linux-user/hppa/target_signal.h | 1 + > linux-user/signal.c | 110 +++++++++++++++++++++++--------- > linux-user/trace-events | 3 + > 3 files changed, 85 insertions(+), 29 deletions(-) > > -- > 2.24.1 >
On Sat, Feb 1, 2020 at 4:27 AM Laurent Vivier <laurent@vivier.eu> wrote: > This has been tested with Go (golang 1.10.1 linux/arm64, bionic) on x86_64 > fedora 31. We can avoid the failure in this case allowing the unsupported > signals when we don't provide the "act" parameters to sigaction, only the > "oldact" one. I have also run the LTP suite with several target and debian > based distros. This breaks with go1.13+ (I also verified at hash 753d56d364)[1]. I tested using an aarch64 guest on an x86 system, but this should manifest on any architecture when the guest/host have the same number of signals (and glibc reserves some host signals). From the traceback, you can see it dies in `initsig` which is called at startup. Any Go program should fail. Since go does not use a libc, it assumes that all signals from [1.._NSIG) are available[2], and will panic if it cannot do an rt_sigaction for all of them. Go already has some special handling for QEMU where it silently discards failing rt_sigaction calls to signals 32, 33, and 64 [3]. Since this patch reserves an extra signal for __SIGRTMIN+1 as well, it blocks out guest signal 63 and Go fails to initialize. While I personally support this patch series (current handling of guest glibc signals is broken), it *will* break Go binaries. I don't know of a way to avoid this while supporting guest __SIGRTMIN+1, without either doing true signal multiplexing, or patching Go. [1] https://gist.github.com/joshkunz/b6c80724072cc1dce79a6253d40b016f [2] https://github.com/golang/go/blob/67f0f83216930e053441500e2b28c3fa2b667581/src/runtime/signal_unix.go#L123 [3] https://github.com/golang/go/blob/master/src/runtime/os_linux.go#L466-L473 > > Laurent Vivier (4): > linux-user: add missing TARGET_SIGRTMIN for hppa > linux-user: cleanup signal.c > linux-user: fix TARGET_NSIG and _NSIG uses > linux-user: fix use of SIGRTMIN > > linux-user/hppa/target_signal.h | 1 + > linux-user/signal.c | 110 +++++++++++++++++++++++--------- > linux-user/trace-events | 3 + > 3 files changed, 85 insertions(+), 29 deletions(-) > > -- > 2.24.1 >
Le 04/02/2020 à 01:03, Josh Kunz a écrit : > On Sat, Feb 1, 2020 at 4:27 AM Laurent Vivier <laurent@vivier.eu> wrote: >> This has been tested with Go (golang 1.10.1 linux/arm64, bionic) on x86_64 >> fedora 31. We can avoid the failure in this case allowing the unsupported >> signals when we don't provide the "act" parameters to sigaction, only the >> "oldact" one. I have also run the LTP suite with several target and debian >> based distros. > > This breaks with go1.13+ (I also verified at hash 753d56d364)[1]. I > tested using an aarch64 guest on an x86 system, but this should > manifest on any architecture when the guest/host have the same number > of signals (and glibc reserves some host signals). From the traceback, > you can see it dies in `initsig` which is called at startup. Any Go > program should fail. I reproduced the problem with aarch64/eoan, go1.12.10. Thank you. > Since go does not use a libc, it assumes that all signals from > [1.._NSIG) are available[2], and will panic if it cannot do an > rt_sigaction for all of them. Go already has some special handling for > QEMU where it silently discards failing rt_sigaction calls to signals > 32, 33, and 64 [3]. Since this patch reserves an extra signal for > __SIGRTMIN+1 as well, it blocks out guest signal 63 and Go fails to > initialize. We should add signal 63 here, but it's becoming not very clean. > While I personally support this patch series (current handling of > guest glibc signals is broken), it *will* break Go binaries. I don't > know of a way to avoid this while supporting guest __SIGRTMIN+1, > without either doing true signal multiplexing, or patching Go. I think we could also simply ignore the error. As returning an error is generally an abort condition even if the signal is never used, and it's what we would do in go to avoid the problem. We will have problem later with programs that really want to use the signal but I don't think we can do better (do we know programs using 31 RT signals? or starting by the end of the list?). something like: diff --git a/linux-user/signal.c b/linux-user/signal.c index c4abacde49a0..169a84afe90b 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -866,8 +866,8 @@ int do_sigaction(int sig, const struct target_sigaction *act, trace_signal_do_sigaction_host(host_sig, TARGET_NSIG); if (host_sig > SIGRTMAX) { /* we don't have enough host signals to map all target signals */ - qemu_log_mask(LOG_UNIMP, "Unsupported target signal #%d\n", sig); - return -TARGET_EINVAL; + qemu_log_mask(LOG_UNIMP, "Unsupported target signal #%d, ignored", sig); + return 0; } Thanks, Laurent
On Tue, Feb 4, 2020 at 3:55 AM Laurent Vivier <laurent@vivier.eu> wrote: > We should add signal 63 here, but it's becoming not very clean. https://golang.org/issue/33746 has some discussion of the issue. The Go maintainers wanted to clean this up rather than just adding 63. The patch is on ice right now because it was unclear if QEMU was going to add a breaking patch. Now that QEMU has this behavior, I think they would be willing to accept it, or something similar. > I think we could also simply ignore the error. As returning an error is > generally an abort condition even if the signal is never used, and it's > what we would do in go to avoid the problem. We will have problem later > with programs that really want to use the signal but I don't think we > can do better (do we know programs using 31 RT signals? or starting by > the end of the list?). My general sense is that RT signals are very rarely used, and the only uses I can find are based off SIGRTMIN. This sounds reasonable to me. Josh