From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26698 invoked by alias); 20 Mar 2014 13:49:13 -0000 Mailing-List: contact gdb-prs-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-prs-owner@sourceware.org Received: (qmail 26668 invoked by uid 55); 20 Mar 2014 13:49:13 -0000 From: "cvs-commit at gcc dot gnu.org" To: gdb-prs@sourceware.org Subject: [Bug breakpoints/7143] Watchpoint does not trigger when first set Date: Thu, 20 Mar 2014 13:49:00 -0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gdb X-Bugzilla-Component: breakpoints X-Bugzilla-Version: unknown X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: cvs-commit at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: palves at redhat dot com X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Bugzilla-URL: http://sourceware.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-SW-Source: 2014-q1/txt/msg00422.txt.bz2 https://sourceware.org/bugzilla/show_bug.cgi?id=7143 --- Comment #9 from cvs-commit at gcc dot gnu.org --- This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "gdb and binutils". The branch, master has been updated via 9f5e1e021a843e573b72ee448397a4db139adf2e (commit) via 99619beac6252113fed212fdb9e1ab97bface423 (commit) via 2adfaa28b5ba2fb78ba5113977082c4d04752bd6 (commit) via 31e77af205cf6564c2bf4c18400b4ca16bdf92cd (commit) via b9f437de50bcca478359c4c2ec0da50c29ddc512 (commit) from bcf83b2a66f0d968b51af8357f1543523ef83470 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9f5e1e021a843e573b72ee448397a4db139adf2e commit 9f5e1e021a843e573b72ee448397a4db139adf2e Author: Pedro Alves Date: Thu Mar 20 13:26:33 2014 +0000 Make signal-while-stepping-over-bp-other-thread.exp run against remote targets too. Use pthread_kill instead of the host's "kill". The reason the test wasn't written that way to begin with, is that done this way, before the previous fixes to make GDB step-over all other threads before the stepping thread, the test would fail... Tested on x86_64 Fedora 17, native and gdbserver. gdb/testsuite/ 2014-03-20 Pedro Alves * gdb.threads/signal-while-stepping-over-bp-other-thread.c (main): Use pthread_kill to signal thread 2. * gdb.threads/signal-while-stepping-over-bp-other-thread.exp: Adjust to make the test send itself a signal rather than using the host's "kill" command. https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=99619beac6252113fed212fdb9e1ab97bface423 commit 99619beac6252113fed212fdb9e1ab97bface423 Author: Pedro Alves Date: Thu Mar 20 13:26:32 2014 +0000 Handle multiple step-overs. This test fails with current mainline. If the program stopped for a breakpoint in thread 1, and then the user switches to thread 2, and resumes the program, GDB first switches back to thread 1 to step it over the breakpoint, in order to make progress. However, that logic only considers the last reported event, assuming only one thread needs that stepping over dance. That's actually not true when we play with scheduler-locking. The patch adds an example to the testsuite of multiple threads needing a step-over before the stepping thread can be resumed. With current mainline, the program re-traps the same breakpoint it had already trapped before. E.g.: Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99 99 wait_threads (); /* set wait-threads breakpoint here */ (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint info threads Id Target Id Frame 3 Thread 0x7ffff77c9700 (LWP 4310) "multiple-step-o" 0x00000000004007ca in child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:43 2 Thread 0x7ffff7fca700 (LWP 4309) "multiple-step-o" 0x0000000000400827 in child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:60 * 1 Thread 0x7ffff7fcb740 (LWP 4305) "multiple-step-o" main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99 (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: info threads shows all threads set scheduler-locking on (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: set scheduler-locking on break 44 Breakpoint 3 at 0x4007d3: file ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c, line 44. (gdb) break 61 Breakpoint 4 at 0x40082d: file ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c, line 61. (gdb) thread 3 [Switching to thread 3 (Thread 0x7ffff77c9700 (LWP 4310))] #0 0x00000000004007ca in child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:43 43 (*myp) ++; (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 3 continue Continuing. Breakpoint 3, child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:44 44 callme (); /* set breakpoint thread 3 here */ (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint in thread 3 p *myp = 0 $1 = 0 (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: unbreak loop in thread 3 thread 2 [Switching to thread 2 (Thread 0x7ffff7fca700 (LWP 4309))] #0 0x0000000000400827 in child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:60 60 (*myp) ++; (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 2 continue Continuing. Breakpoint 4, child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:61 61 callme (); /* set breakpoint thread 2 here */ (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint in thread 2 p *myp = 0 $2 = 0 (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: unbreak loop in thread 2 thread 1 [Switching to thread 1 (Thread 0x7ffff7fcb740 (LWP 4305))] #0 main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99 99 wait_threads (); /* set wait-threads breakpoint here */ (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 1 set scheduler-locking off (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: set scheduler-locking off At this point all thread are stopped for a breakpoint that needs stepping over. (gdb) step Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99 99 wait_threads (); /* set wait-threads breakpoint here */ (gdb) FAIL: gdb.threads/multiple-step-overs.exp: step But that "step" retriggers the same breakpoint instead of making progress. The patch teaches GDB to step over all breakpoints of all threads before resuming the stepping thread. Tested on x86_64 Fedora 17, against pristine mainline, and also my branch that implements software single-stepping on x86. gdb/ 2014-03-20 Pedro Alves * infrun.c (prepare_to_proceed): Delete. (thread_still_needs_step_over): New function. (find_thread_needs_step_over): New function. (proceed): If the current thread needs a step-over, set its steping_over_breakpoint flag. Adjust to use find_thread_needs_step_over instead of prepare_to_proceed. (process_event_stop_test): For BPSTAT_WHAT_STOP_NOISY and BPSTAT_WHAT_STOP_SILENT, assume the thread stopped for a breakpoint. (switch_back_to_stepped_thread): Step over breakpoints of all threads not the stepping thread, before switching back to the stepping thread. gdb/testsuite/ 2014-03-20 Pedro Alves * gdb.threads/multiple-step-overs.c: New file. * gdb.threads/multiple-step-overs.exp: New file. * gdb.threads/signal-while-stepping-over-bp-other-thread.exp: Adjust expected infrun debug output. https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2adfaa28b5ba2fb78ba5113977082c4d04752bd6 commit 2adfaa28b5ba2fb78ba5113977082c4d04752bd6 Author: Pedro Alves Date: Thu Mar 20 13:26:32 2014 +0000 Fix for even more missed events; eliminate thread-hop code. Even with deferred_step_ptid out of the way, GDB can still lose watchpoints. If a watchpoint triggers and the PC points to an address where a thread-specific breakpoint for another thread is set, the thread-hop code triggers, and we lose the watchpoint: if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP) { int thread_hop_needed = 0; struct address_space *aspace = get_regcache_aspace (get_thread_regcache (ecs->ptid)); /* Check if a regular breakpoint has been hit before checking for a potential single step breakpoint. Otherwise, GDB will not see this breakpoint hit when stepping onto breakpoints. */ if (regular_breakpoint_inserted_here_p (aspace, stop_pc)) { if (!breakpoint_thread_match (aspace, stop_pc, ecs->ptid)) thread_hop_needed = 1; ^^^^^^^^^^^^^^^^^^^^^ } And on software single-step targets, even without a thread-specific breakpoint in the way, here in the thread-hop code: else if (singlestep_breakpoints_inserted_p) { ... if (!ptid_equal (singlestep_ptid, ecs->ptid) && in_thread_list (singlestep_ptid)) { /* If the PC of the thread we were trying to single-step has changed, discard this event (which we were going to ignore anyway), and pretend we saw that thread trap. This prevents us continuously moving the single-step breakpoint forward, one instruction at a time. If the PC has changed, then the thread we were trying to single-step has trapped or been signalled, but the event has not been reported to GDB yet. There might be some cases where this loses signal information, if a signal has arrived at exactly the same time that the PC changed, but this is the best we can do with the information available. Perhaps we should arrange to report all events for all threads when they stop, or to re-poll the remote looking for this particular thread (i.e. temporarily enable schedlock). */ CORE_ADDR new_singlestep_pc = regcache_read_pc (get_thread_regcache (singlestep_ptid)); if (new_singlestep_pc != singlestep_pc) { enum gdb_signal stop_signal; if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: unexpected thread," " but expected thread advanced also\n"); /* The current context still belongs to singlestep_ptid. Don't swap here, since that's the context we want to use. Just fudge our state and continue. */ stop_signal = ecs->event_thread->suspend.stop_signal; ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; ecs->ptid = singlestep_ptid; ecs->event_thread = find_thread_ptid (ecs->ptid); ecs->event_thread->suspend.stop_signal = stop_signal; stop_pc = new_singlestep_pc; } else { if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: unexpected thread\n"); thread_hop_needed = 1; stepping_past_singlestep_breakpoint = 1; saved_singlestep_ptid = singlestep_ptid; } } } we either end up with thread_hop_needed, ignoring the watchpoint SIGTRAP, or switch to the stepping thread, again ignoring that the SIGTRAP could be for some other event. The new test added by this patch exercises both paths. So the fix is similar to the deferred_step_ptid fix -- defer the thread hop to _after_ the SIGTRAP had a change of passing through the regular bpstat handling. If the wrong thread hits a breakpoint, we'll just end up with BPSTAT_WHAT_SINGLE, and if nothing causes a stop, keep_going starts a step-over. Most of the stepping_past_singlestep_breakpoint mechanism is really not necessary -- setting the thread to step over a breakpoint with thread->trap_expected is sufficient to keep all other threads locked. It's best to still keep the flag in some form though, because when we get to keep_going, the software single-step breakpoint we need to step over is already gone -- an optimization done by a follow up patch will check whether a step-over is still be necessary by looking to see whether the breakpoint is still there, and would find the thread no longer needs a step-over, while we still want it. Special care is still needed to handle the case of PC of the thread we were trying to single-step having changed, like in the old code. We can't just keep_going and re-step it, as in that case we can over-step the thread (if it was already done with the step, but hasn't reported it yet, we'd ask it to step even further). That's now handled in switch_back_to_stepped_thread. As bonus, we're now using a technique that doesn't lose signals, unlike the old code -- we now insert a breakpoint at PC, and resume, which either reports the breakpoint immediately, or any pending signal. Tested on x86_64 Fedora 17, against pristine mainline, and against a branch that implements software single-step on x86. gdb/ 2014-03-20 Pedro Alves * breakpoint.c (single_step_breakpoint_inserted_here_p): Make extern. * breakpoint.h (single_step_breakpoint_inserted_here_p): Declare. * infrun.c (saved_singlestep_ptid) (stepping_past_singlestep_breakpoint): Delete. (resume): Remove stepping_past_singlestep_breakpoint handling. (proceed): Store the prev_pc of the stepping thread too. (init_wait_for_inferior): Adjust. Clear singlestep_ptid and singlestep_pc. (enum infwait_states): Delete infwait_thread_hop_state. (struct execution_control_state) : New field. (handle_inferior_event): Adjust. (handle_signal_stop): Delete stepping_past_singlestep_breakpoint handling and the thread-hop code. Before removing single-step breakpoints, check whether the thread hit a single-step breakpoint of another thread. If it did, the trap is not a random signal. (switch_back_to_stepped_thread): If the event thread hit a single-step breakpoint, unblock it before switching to the stepping thread. Handle the case of the stepped thread having advanced already. (keep_going): Handle the case of the current thread moving past a single-step breakpoint. gdb/testsuite/ 2014-03-20 Pedro Alves * gdb.threads/step-over-trips-on-watchpoint.c: New file. * gdb.threads/step-over-trips-on-watchpoint.exp: New file. https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=31e77af205cf6564c2bf4c18400b4ca16bdf92cd commit 31e77af205cf6564c2bf4c18400b4ca16bdf92cd Author: Pedro Alves Date: Thu Mar 20 13:26:32 2014 +0000 PR breakpoints/7143 - Watchpoint does not trigger when first set Say the program is stopped at a breakpoint, and the user sets a watchpoint. When the program is next resumed, GDB will first step over the breakpoint, as explained in the manual: @value {GDBN} normally ignores breakpoints when it resumes execution, until at least one instruction has been executed. If it it did not do this, you would be unable to proceed past a breakpoint without first disabling the breakpoint. This rule applies whether or not the breakpoint already existed when your program stopped. However, GDB currently also removes watchpoints, catchpoints, etc., and that means that the first instruction off the breakpoint does not trigger the watchpoint, catchpoint, etc. testsuite/gdb.base/watchpoint.exp has a kfail for this. The PR proposes installing watchpoints only when stepping over a breakpoint, but that misses catchpoints, etc. A better fix would instead work from the opposite direction -- remove only real breakpoints, leaving all other kinds of breakpoints inserted. But, going further, it's really a waste to constantly remove/insert all breakpoints when stepping over a single breakpoint (generating a pair of RSP z/Z packets for each breakpoint), so the fix goes a step further and makes GDB remove _only_ the breakpoint being stepped over, leaving all others installed. This then has the added benefit of reducing breakpoint-related RSP traffic substancialy when there are many breakpoints set. gdb/ 2014-03-20 Pedro Alves PR breakpoints/7143 * breakpoint.c (should_be_inserted): Don't insert breakpoints that are being stepped over. (breakpoint_address_match): Make extern. * breakpoint.h (breakpoint_address_match): New declaration. * inferior.h (stepping_past_instruction_at): New declaration. * infrun.c (struct step_over_info): New type. (step_over_info): New global. (set_step_over_info, clear_step_over_info) (stepping_past_instruction_at): New functions. (handle_inferior_event): Clear the step-over info when trap_expected is cleared. (resume): Remove now stale comment. (clear_proceed_status): Clear step-over info. (proceed): Adjust step-over handling to set or clear the step-over info instead of removing all breakpoints. (handle_signal_stop): When setting up a thread-hop, don't remove breakpoints here. (stop_stepping): Clear step-over info. (keep_going): Adjust step-over handling to set or clear step-over info and then always inserting breakpoints, instead of removing all breakpoints when stepping over one. gdb/testsuite/ 2014-03-20 Pedro Alves PR breakpoints/7143 * gdb.base/watchpoint.exp: Mention bugzilla bug number instead of old gnats gdb/38. Remove kfail. Adjust to use gdb_test instead of gdb_test_multiple. * gdb.cp/annota2.exp: Remove kfail for gdb/38. * gdb.cp/annota3.exp: Remove kfail for gdb/38. https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b9f437de50bcca478359c4c2ec0da50c29ddc512 commit b9f437de50bcca478359c4c2ec0da50c29ddc512 Author: Pedro Alves Date: Thu Mar 20 13:26:31 2014 +0000 Fix missing breakpoint/watchpoint hits, eliminate deferred_step_ptid. Consider the case of the user doing "step" in thread 2, while thread 1 had previously stopped for a breakpoint. In order to make progress, GDB makes thread 1 step over its breakpoint first (with all other threads stopped), and once that is over, thread 2 then starts stepping (with thread 1 and all others running free, by default). If GDB didn't do that, thread 1 would just trip on the same breakpoint immediately again. This is what the prepare_to_proceed / deferred_step_ptid code is all about. However, deferred_step_ptid code resumes the target with: resume (1, GDB_SIGNAL_0); prepare_to_wait (ecs); return; Recall we were just stepping over a breakpoint when we get here. That means that _nothing_ had installed breakpoints yet! If there's another breakpoint just after the breakpoint that was just stepped, we'll miss it. The fix for that would be to use keep_going instead. However, there are more problems. What if the instruction that was just single-stepped triggers a watchpoint? Currently, GDB just happily resumes the thread, losing that too... Missed watchpoints will need yet further fixes, but we should keep those in mind. So the fix must be to let the trap fall through the regular bpstat handling, and only if no breakpoint, watchpoint, etc. claims the trap, shall we switch back to the stepped thread. Now, nowadays, we have code at the tail end of trap handling that does exactly that -- switch back to the stepped thread (switch_back_to_the_stepped_thread). So the deferred_step_ptid code is just standing in the way, and can simply be eliminated, fixing bugs in the process. Sweet. The comment about spurious "Switching to ..." made me pause, but is actually stale nowadays. That isn't needed anymore. previous_inferior_ptid used to be re-set at each (internal) event, but now it's only touched in proceed and normal stop. The two tests added by this patch fail without the fix. Tested on x86_64 Fedora 17 (also against my software single-stepping on x86 branch). gdb/ 2014-03-20 Pedro Alves * infrun.c (previous_inferior_ptid): Adjust comment. (deferred_step_ptid): Delete. (infrun_thread_ptid_changed, prepare_to_proceed) (init_wait_for_inferior): Adjust. (handle_signal_stop): Delete deferred_step_ptid handling. gdb/testsuite/ 2014-03-20 Pedro Alves * gdb.threads/step-over-lands-on-breakpoint.c: New file. * gdb.threads/step-over-lands-on-breakpoint.exp: New file. ----------------------------------------------------------------------- Summary of changes: gdb/ChangeLog | 74 ++ gdb/breakpoint.c | 25 +- gdb/breakpoint.h | 13 + gdb/inferior.h | 6 + gdb/infrun.c | 767 ++++++++++---------- gdb/testsuite/ChangeLog | 34 + gdb/testsuite/gdb.base/watchpoint.exp | 13 +- gdb/testsuite/gdb.cp/annota2.exp | 3 - gdb/testsuite/gdb.cp/annota3.exp | 3 - gdb/testsuite/gdb.threads/multiple-step-overs.c | 105 +++ gdb/testsuite/gdb.threads/multiple-step-overs.exp | 80 ++ .../signal-while-stepping-over-bp-other-thread.c | 2 + .../signal-while-stepping-over-bp-other-thread.exp | 16 +- .../gdb.threads/step-over-lands-on-breakpoint.c | 65 ++ .../gdb.threads/step-over-lands-on-breakpoint.exp | 62 ++ .../gdb.threads/step-over-trips-on-watchpoint.c | 67 ++ .../gdb.threads/step-over-trips-on-watchpoint.exp | 90 +++ 17 files changed, 986 insertions(+), 439 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/multiple-step-overs.c create mode 100644 gdb/testsuite/gdb.threads/multiple-step-overs.exp create mode 100644 gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c create mode 100644 gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp create mode 100644 gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c create mode 100644 gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp -- You are receiving this mail because: You are on the CC list for the bug.