Message ID | 20181019174958.26616-1-peter.maydell@linaro.org |
---|---|
Headers | show |
Series | linux-user: Don't call gdb_handlesig unnecessarily | expand |
On 10/19/18 10:49 AM, Peter Maydell wrote: > This patchset fixes a minor bug in our handling of SIGTRAP > in linux-user. > > The CPU main-loop routines for linux-user generally call > gdb_handlesig() when they're about to queue a SIGTRAP signal. This > is wrong, because queue_signal() will cause us to pend a signal, and > process_pending_signals() will then call gdb_handlesig() itself. So > the effect is that we notify gdb of the SIGTRAP, and then if gdb says > "OK, continue with signal X" we will incorrectly notify gdb of the > signal X as well. We don't do this double-notify for anything else, > only SIGTRAP. > > This bug only manifests if the user responds to the reported SIGTRAP > using "signal SIGFOO" rather than "continue"; since the latter is the > overwhelmingly common thing to do after a breakpoint most people > won't have hit this. > > Patch 1 fixes this bug for every target except nios2, by > deleting the incorrect code. > > Patch 2 fixes nios2 separately, because it was doing some odd > things with gdb_handlesig(). This also fixes in passing a Coverity > issue. > > Tested with "make check-tcg", and with some by-hand stepping > around with an attached gdb. NB that the nios2 patch is only > compile tested as I don't have a nios2 linux-user environment > and check-tcg doesn't cover it. > > thanks > -- PMM > > Peter Maydell (2): > linux-user: Don't call gdb_handlesig() before queue_signal() > linux-user: Clean up nios2 main loop signal handling Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Ping? This got reviewed but hasn't made it to master... thanks -- PMM On 19 October 2018 at 18:49, Peter Maydell <peter.maydell@linaro.org> wrote: > This patchset fixes a minor bug in our handling of SIGTRAP > in linux-user. > > The CPU main-loop routines for linux-user generally call > gdb_handlesig() when they're about to queue a SIGTRAP signal. This > is wrong, because queue_signal() will cause us to pend a signal, and > process_pending_signals() will then call gdb_handlesig() itself. So > the effect is that we notify gdb of the SIGTRAP, and then if gdb says > "OK, continue with signal X" we will incorrectly notify gdb of the > signal X as well. We don't do this double-notify for anything else, > only SIGTRAP. > > This bug only manifests if the user responds to the reported SIGTRAP > using "signal SIGFOO" rather than "continue"; since the latter is the > overwhelmingly common thing to do after a breakpoint most people > won't have hit this. > > Patch 1 fixes this bug for every target except nios2, by > deleting the incorrect code. > > Patch 2 fixes nios2 separately, because it was doing some odd > things with gdb_handlesig(). This also fixes in passing a Coverity > issue. > > Tested with "make check-tcg", and with some by-hand stepping > around with an attached gdb. NB that the nios2 patch is only > compile tested as I don't have a nios2 linux-user environment > and check-tcg doesn't cover it. > > thanks > -- PMM > > Peter Maydell (2): > linux-user: Don't call gdb_handlesig() before queue_signal() > linux-user: Clean up nios2 main loop signal handling > > linux-user/aarch64/cpu_loop.c | 13 +++++-------- > linux-user/alpha/cpu_loop.c | 12 ++++-------- > linux-user/arm/cpu_loop.c | 16 ++++------------ > linux-user/cris/cpu_loop.c | 16 ++++------------ > linux-user/hppa/cpu_loop.c | 11 ++++------- > linux-user/i386/cpu_loop.c | 16 ++++------------ > linux-user/m68k/cpu_loop.c | 16 ++++------------ > linux-user/microblaze/cpu_loop.c | 16 ++++------------ > linux-user/mips/cpu_loop.c | 16 ++++------------ > linux-user/nios2/cpu_loop.c | 14 +++++--------- > linux-user/openrisc/cpu_loop.c | 11 ++++------- > linux-user/ppc/cpu_loop.c | 15 +++++---------- > linux-user/riscv/cpu_loop.c | 2 +- > linux-user/s390x/cpu_loop.c | 9 +++------ > linux-user/sh4/cpu_loop.c | 17 ++++------------- > linux-user/sparc/cpu_loop.c | 16 ++++------------ > linux-user/xtensa/cpu_loop.c | 11 ++++------- > 17 files changed, 67 insertions(+), 160 deletions(-)
On 12/11/2018 15:39, Peter Maydell wrote:
> Ping? This got reviewed but hasn't made it to master...
Do you want it in 3.1 too?
Thanks,
Laurent
On 12 November 2018 at 14:46, Laurent Vivier <laurent@vivier.eu> wrote: > On 12/11/2018 15:39, Peter Maydell wrote: >> Ping? This got reviewed but hasn't made it to master... > > Do you want it in 3.1 too? Well, it is a bugfix, and we haven't rolled rc1 yet, so I would default to yes, unless you'd rather defer it to 4.0. thanks -- PMM