From 427cd150eed8c0dd4f0d0a1105448b4ebe36da6d Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Tue, 18 Jun 2013 19:57:49 +0000 Subject: [PATCH] Fix PR cli/15603 This fixes PR cli/15603. The bug here is that when a software watchpoint is being used, gdb will stop responding to C-c. This is a regression caused by the "catch signal" patch. The problem is that software watchpoints always end up on the bpstat list. However, this makes bpstat_explains_signal return BPSTAT_SIGNAL_HIDE, causing infrun to think that the signal is not a "random signal". The fix is to change bpstat_explains_signal to handle this better. I chose to do it in a "clean API" way, by passing the signal value to bpstat_explains_signal and then adding an explains_signal method for watchpoints, which handles the specifics. Built and regtested on x86-64 Fedora 18. New test case included. * break-catch-sig.c (signal_catchpoint_explains_signal): Add 'sig' argument. * breakpoint.c (bpstat_explains_signal): Add 'sig' argument. Special case signals other than GDB_SIGNAL_TRAP. (explains_signal_watchpoint): New function. (base_breakpoint_explains_signal): Add 'sig' argument. (initialize_breakpoint_ops): Set 'explains_signal' method for watchpoints. * breakpoint.h (struct breakpoint_ops) : Add signal argument. (bpstat_explains_signal): Likewise. * infrun.c (handle_syscall_event, handle_inferior_event): Update. * gdb.base/random-signal.c: New file. * gdb.base/random-signal.exp: New file. --- gdb/ChangeLog | 15 ++++++++++++ gdb/break-catch-sig.c | 2 +- gdb/breakpoint.c | 35 ++++++++++++++++++++++---- gdb/breakpoint.h | 6 +++-- gdb/infrun.c | 18 +++++++++----- gdb/testsuite/ChangeLog | 5 ++++ gdb/testsuite/gdb.base/random-signal.c | 29 ++++++++++++++++++++++ gdb/testsuite/gdb.base/random-signal.exp | 42 ++++++++++++++++++++++++++++++++ 8 files changed, 138 insertions(+), 14 deletions(-) create mode 100644 gdb/testsuite/gdb.base/random-signal.c create mode 100644 gdb/testsuite/gdb.base/random-signal.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0b5323c..d658bfb 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,20 @@ 2013-06-18 Tom Tromey + * break-catch-sig.c (signal_catchpoint_explains_signal): Add 'sig' + argument. + * breakpoint.c (bpstat_explains_signal): Add 'sig' argument. + Special case signals other than GDB_SIGNAL_TRAP. + (explains_signal_watchpoint): New function. + (base_breakpoint_explains_signal): Add 'sig' argument. + (initialize_breakpoint_ops): Set 'explains_signal' method for + watchpoints. + * breakpoint.h (struct breakpoint_ops) : Add + signal argument. + (bpstat_explains_signal): Likewise. + * infrun.c (handle_syscall_event, handle_inferior_event): Update. + +2013-06-18 Tom Tromey + * python/py-inferior.c (gdbpy_selected_inferior): Don't incref. 2013-06-18 Tom Tromey diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c index c162cc2..02d8b4a 100644 --- a/gdb/break-catch-sig.c +++ b/gdb/break-catch-sig.c @@ -351,7 +351,7 @@ signal_catchpoint_print_recreate (struct breakpoint *b, struct ui_file *fp) catchpoints. */ static enum bpstat_signal_value -signal_catchpoint_explains_signal (struct breakpoint *b) +signal_catchpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig) { return BPSTAT_SIGNAL_PASS; } diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index d4ccc81..f4933fc 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -4153,7 +4153,7 @@ bpstat_find_breakpoint (bpstat bsp, struct breakpoint *breakpoint) /* See breakpoint.h. */ enum bpstat_signal_value -bpstat_explains_signal (bpstat bsp) +bpstat_explains_signal (bpstat bsp, enum gdb_signal sig) { enum bpstat_signal_value result = BPSTAT_SIGNAL_NO; @@ -4161,10 +4161,20 @@ bpstat_explains_signal (bpstat bsp) { /* Ensure that, if we ever entered this loop, then we at least return BPSTAT_SIGNAL_HIDE. */ - enum bpstat_signal_value newval = BPSTAT_SIGNAL_HIDE; + enum bpstat_signal_value newval; - if (bsp->breakpoint_at != NULL) - newval = bsp->breakpoint_at->ops->explains_signal (bsp->breakpoint_at); + if (bsp->breakpoint_at == NULL) + { + /* A moribund location can never explain a signal other than + GDB_SIGNAL_TRAP. */ + if (sig == GDB_SIGNAL_TRAP) + newval = BPSTAT_SIGNAL_HIDE; + else + newval = BPSTAT_SIGNAL_NO; + } + else + newval = bsp->breakpoint_at->ops->explains_signal (bsp->breakpoint_at, + sig); if (newval > result) result = newval; @@ -10630,6 +10640,20 @@ print_recreate_watchpoint (struct breakpoint *b, struct ui_file *fp) print_recreate_thread (b, fp); } +/* Implement the "explains_signal" breakpoint_ops method for + watchpoints. */ + +static enum bpstat_signal_value +explains_signal_watchpoint (struct breakpoint *b, enum gdb_signal sig) +{ + /* A software watchpoint cannot cause a signal other than + GDB_SIGNAL_TRAP. */ + if (b->type == bp_watchpoint && sig != GDB_SIGNAL_TRAP) + return BPSTAT_SIGNAL_NO; + + return BPSTAT_SIGNAL_HIDE; +} + /* The breakpoint_ops structure to be used in hardware watchpoints. */ static struct breakpoint_ops watchpoint_breakpoint_ops; @@ -12733,7 +12757,7 @@ base_breakpoint_decode_linespec (struct breakpoint *b, char **s, /* The default 'explains_signal' method. */ static enum bpstat_signal_value -base_breakpoint_explains_signal (struct breakpoint *b) +base_breakpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig) { return BPSTAT_SIGNAL_HIDE; } @@ -15743,6 +15767,7 @@ initialize_breakpoint_ops (void) ops->print_it = print_it_watchpoint; ops->print_mention = print_mention_watchpoint; ops->print_recreate = print_recreate_watchpoint; + ops->explains_signal = explains_signal_watchpoint; /* Masked watchpoints. */ ops = &masked_watchpoint_breakpoint_ops; diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 94210f4..43ab906 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -613,7 +613,8 @@ struct breakpoint_ops should still be delivered to the inferior. This is used to make 'catch signal' interact properly with 'handle'; see bpstat_explains_signal. */ - enum bpstat_signal_value (*explains_signal) (struct breakpoint *); + enum bpstat_signal_value (*explains_signal) (struct breakpoint *, + enum gdb_signal); }; /* Helper for breakpoint_ops->print_recreate implementations. Prints @@ -1009,7 +1010,8 @@ bpstat bpstat_find_breakpoint (bpstat, struct breakpoint *); /* Nonzero if a signal that we got in wait() was due to circumstances explained by the bpstat; and the signal should therefore not be delivered. */ -extern enum bpstat_signal_value bpstat_explains_signal (bpstat); +extern enum bpstat_signal_value bpstat_explains_signal (bpstat, + enum gdb_signal); /* Nonzero is this bpstat causes a stop. */ extern int bpstat_causes_stop (bpstat); diff --git a/gdb/infrun.c b/gdb/infrun.c index 51a032b..720c4ed 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3124,7 +3124,8 @@ handle_syscall_event (struct execution_control_state *ecs) = bpstat_stop_status (get_regcache_aspace (regcache), stop_pc, ecs->ptid, &ecs->ws); - sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat); + sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat, + GDB_SIGNAL_TRAP); ecs->random_signal = sval == BPSTAT_SIGNAL_NO; if (!ecs->random_signal) @@ -3374,7 +3375,8 @@ handle_inferior_event (struct execution_control_state *ecs) stop_pc, ecs->ptid, &ecs->ws); sval - = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat); + = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat, + GDB_SIGNAL_TRAP); ecs->random_signal = sval == BPSTAT_SIGNAL_NO; if (!ecs->random_signal) @@ -3673,7 +3675,8 @@ handle_inferior_event (struct execution_control_state *ecs) = bpstat_stop_status (get_regcache_aspace (get_current_regcache ()), stop_pc, ecs->ptid, &ecs->ws); ecs->random_signal - = (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat) + = (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat, + GDB_SIGNAL_TRAP) == BPSTAT_SIGNAL_NO); /* Note that this may be referenced from inside @@ -4248,7 +4251,8 @@ handle_inferior_event (struct execution_control_state *ecs) if (debug_infrun && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP - && (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat) + && (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat, + GDB_SIGNAL_TRAP) == BPSTAT_SIGNAL_NO) && stopped_by_watchpoint) fprintf_unfiltered (gdb_stdlog, @@ -4277,7 +4281,8 @@ handle_inferior_event (struct execution_control_state *ecs) if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP) ecs->random_signal - = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat) + = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat, + GDB_SIGNAL_TRAP) != BPSTAT_SIGNAL_NO) || stopped_by_watchpoint || ecs->event_thread->control.trap_expected @@ -4288,7 +4293,8 @@ handle_inferior_event (struct execution_control_state *ecs) { enum bpstat_signal_value sval; - sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat); + sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat, + ecs->event_thread->suspend.stop_signal); ecs->random_signal = (sval == BPSTAT_SIGNAL_NO); if (sval == BPSTAT_SIGNAL_HIDE) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 395b86b..2228dc5 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2013-06-18 Tom Tromey + + * gdb.base/random-signal.c: New file. + * gdb.base/random-signal.exp: New file. + 2013-06-18 Will Newton * gdb.base/skip.c: Use comma to evaluate results of foo() diff --git a/gdb/testsuite/gdb.base/random-signal.c b/gdb/testsuite/gdb.base/random-signal.c new file mode 100644 index 0000000..3d23bf7 --- /dev/null +++ b/gdb/testsuite/gdb.base/random-signal.c @@ -0,0 +1,29 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +int v; + +int main() +{ + /* Don't let the test case run forever. */ + alarm (60); + + for (;;) + ; +} diff --git a/gdb/testsuite/gdb.base/random-signal.exp b/gdb/testsuite/gdb.base/random-signal.exp new file mode 100644 index 0000000..bd23513 --- /dev/null +++ b/gdb/testsuite/gdb.base/random-signal.exp @@ -0,0 +1,42 @@ +# Copyright 2013 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +if [target_info exists gdb,nosignals] { + verbose "Skipping catch-signal.exp because of nosignals." + continue +} + +standard_testfile + +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} { + return -1 +} + +if {![runto_main]} { + return -1 +} + +gdb_test_no_output "set can-use-hw-watchpoints 0" +gdb_test "watch v" "Watchpoint .*" +gdb_test_multiple "continue" "continue" { + -re "Continuing" { + pass "continue" + } +} + +# For this to work we must be sure to consume the "Continuing." +# message first, or GDB's signal handler may not be in place. +after 500 {send_gdb "\003"} +gdb_test "" "Program received signal SIGINT.*" "stop with control-c" -- 2.7.4