Bug 7143

From: carlton@sources.redhat.com
To: gdb-gnats@sourceware.org
Cc:  
Subject: breakpoints/38
Date: 17 Mar 2003 19:51:59 -0000

 CVSROOT:	/cvs/src
 Module name:	src
 Changes by:	carlton@sourceware.org	2003-03-17 19:51:59
 
 Modified files:
 	gdb/testsuite  : ChangeLog 
 	gdb/testsuite/gdb.base: watchpoint.c watchpoint.exp 
 
 Log message:
 	2003-03-17  David Carlton  <carlton@math.stanford.edu>
 	
 	* gdb.base/watchpoint.exp (test_watchpoint_and_breakpoint): New
 	test, for PR breakpoints/38.
 	Call test_watchpoint_and_breakpoint.
 	* gdb.base/watchpoint.c (func3): New function.
 	(main): Call func3.
 
 Patches:
 http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/testsuite/ChangeLog.diff?cvsroot=src&r1=1.504&r2=1.505
 http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/watchpoint.c.diff?cvsroot=src&r1=1.1.1.2&r2=1.2
 http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/watchpoint.exp.diff?cvsroot=src&r1=1.7&r2=1.8

Well formed problem. Relevant snippets from this test case : 

int i = 0;

int main ()
{
  i = 1;
  i = 2;
  i = 3;
  return 0;
}

(gdb) break main
Breakpoint 1 at 0x80483c3: file x2.c, line 5.
(gdb) run
Starting program: /gehman/home/chastain/tmp/x2 

Breakpoint 1, main () at x2.c:5
5	  i = 1;
(gdb) watch i
Hardware watchpoint 2: i
(gdb) next
6	  i = 2;
(gdb) next
Hardware watchpoint 2: i

Old value = 0
New value = 2
main () at x2.c:7
7	  i = 3;

Change from 0->1 is not 'watched'. 

Breakpoint will be hit , before executing the instruction. Whereas
Watchpoint will be hit , after  executing the instruction. 

In the first 'next' after 'watch' , breakpoints are not enabled , as current pc
is still pointing to breakpoint 1 ( 'main' ). 

infrun.c : 

    791   if (oneproc)
    792     /* We will get a trace trap after one instruction.
    793        Continue it automatically and insert breakpoints then.  */
    794     stepping_over_breakpoint = 1;
    795   else
    796     insert_breakpoints ();

Here the watchpoints can be enabled even for single stepping. Following can be
done : 

1. in breakpoint.c, rename insert_watchpoints to a local function (static
function ) and add a boolean parameter , say watchpoint only. 
   -> And skip non-watchpoint bpts if watchpoint_only=TRUE.
2. Create insert_watchpoints , which calls insert_watchpoints_local with FALSE. 
3. Introduce a new function insert_watchpoints , which calls
insert_watchpoints_local with TRUE. 
4. In infrun.c:proceed , if ( single stepping ) call insert_watchpoints.

With a modified gdb : 

Breakpoint 1, main () at x2.c:5
5         i = 1;
(top-gdb) watch i
Hardware watchpoint 2: i
(top-gdb) next
During symbol reading, incomplete CFI data; unspecified registers (e.g., rax) at
0x4004ac.
Hardware watchpoint 2: i

Old value = 0
New value = 1
main () at x2.c:6
6         i = 2;
(top-gdb) next
Hardware watchpoint 2: i

Old value = 1
New value = 2
main () at x2.c:7
7         i = 3;
(top-gdb) next
Hardware watchpoint 2: i

Old value = 2
New value = 3
main () at x2.c:8


-------------------------------------------------------------------------------
Following are the code changes : ( apologize , I am still a newbie to the gdb
dev processes . Will try improve the same :-) ).

breakpoint.h : 

@@ -717,6 +717,8 @@ extern void set_breakpoint (char *addres
 
 extern void insert_breakpoints (void);
 
+extern void insert_watchpoints (void);
+
 extern int remove_breakpoints (void);
 
breakpoint.c : 

@@ -210,6 +210,8 @@ unlink_locations_from_global_list (struc
 static int
 is_hardware_watchpoint (struct breakpoint *bpt);
 
+static void insert_breakpoints_local (int watchpoint_only);
+
 /* Prototypes for exported functions. */
 
 /* If FALSE, gdb will not use hardware support for watchpoints, even
@@ -1220,6 +1222,18 @@ Note: automatically using hardware break
 void
 insert_breakpoints (void)
 {
+  insert_breakpoints_local(FALSE); 
+}
+
+void
+insert_watchpoints (void)
+{
+  insert_breakpoints_local(TRUE); 
+}
+
+static void
+insert_breakpoints_local (int watchpoint_only)
+{
   struct breakpoint *bpt;
   struct bp_location *b, *temp;
   int error = 0;
@@ -1244,6 +1258,9 @@ insert_breakpoints (void)
       if (!breakpoint_enabled (b->owner))
        continue;
 
+      if (watchpoint_only && !is_hardware_watchpoint(b->owner))
+        continue;
+
       /* There is no point inserting thread-specific breakpoints if the
         thread no longer exists.  */
       if (b->owner->thread != -1
@@ -1707,6 +1724,14 @@ breakpoint_here_p (CORE_ADDR pc)
 
   ALL_BP_LOCATIONS (bpt)
     {
+      struct breakpoint *b=bpt->owner;
+      
+      if (bpt->loc_type == bp_loc_hardware_breakpoint && 
+          (b->type == bp_hardware_watchpoint || 
+          b->type == bp_read_watchpoint     ||
+          b->type == bp_access_watchpoint))
+       continue;
+
       if (bpt->loc_type != bp_loc_software_breakpoint
          && bpt->loc_type != bp_loc_hardware_breakpoint)
        continue;

infrun.c : 

@@ -789,9 +789,13 @@ proceed (CORE_ADDR addr, enum target_sig
     oneproc = 1;
 
   if (oneproc)
+  {
     /* We will get a trace trap after one instruction.
-       Continue it automatically and insert breakpoints then.  */
+       Continue it automatically and insert breakpoints then. But
+       watchpoints are different as they needs to be enabled  now. */
     stepping_over_breakpoint = 1;
+    insert_watchpoints (); 
+  }
   else
     insert_breakpoints ();

watchpoint.exp passes entirely for me on x86-64 Fedora 15.
So, I think this was fixed at some point.

*** Bug 7186 has been marked as a duplicate of this bug. ***

I don't think it has been fixed.

I think watchpoints.exp doesn't show the kfail because there's a 

  send_gdb "set can-use-hw-watchpoints 0\n"

and nothing re-enables hw watchpoints, before the relevant test, and
so the test is always run with software watchpoints, which don't have this problem.

*** Bug 13929 has been marked as a duplicate of this bug. ***

For the record, the test was changed to have the kfail visible again (at the end of 2012/01).

Stumbled on this while working on a related series.  I'll fix it.

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 <palves@redhat.com>
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  <palves@redhat.com>
    
    	* 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 <palves@redhat.com>
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  <palves@redhat.com>
    
    	* 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  <palves@redhat.com>
    
    	* 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 <palves@redhat.com>
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  <palves@redhat.com>
    
    	* 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) <hit_singlestep_breakpoint>: 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  <palves@redhat.com>
    
    	* 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 <palves@redhat.com>
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  <palves@redhat.com>
    
    	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  <palves@redhat.com>
    
    	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 <palves@redhat.com>
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  <palves@redhat.com>
    
    	* 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  <palves@redhat.com>
    
    	* 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

Fixed.

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  802e8e6d8465a0d05803a987ba1bb3237fb2fb70 (commit)
       via  932539e3ab93db990ef61c80bbf78a4f2fdcc60b (commit)
       via  4ff0d3d82d37a57507c147886ce9f9bfb7495b99 (commit)
      from  786dc51990a8d593a8d727735910ccf97752d928 (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=802e8e6d8465a0d05803a987ba1bb3237fb2fb70

commit 802e8e6d8465a0d05803a987ba1bb3237fb2fb70
Author: Pedro Alves <palves@redhat.com>
Date:   Tue May 20 18:24:28 2014 +0100

    [GDBserver] Make Zx/zx packet handling idempotent.
    
    This patch fixes hardware breakpoint regressions exposed by my fix for
    "PR breakpoints/7143 - Watchpoint does not trigger when first set", at
    https://sourceware.org/ml/gdb-patches/2014-03/msg00167.html
    
    The testsuite caught them on Linux/x86_64, at least.  gdb.sum:
    
    gdb.sum:
    
     FAIL: gdb.base/hbreak2.exp: next over recursive call
     FAIL: gdb.base/hbreak2.exp: backtrace from factorial(5.1)
     FAIL: gdb.base/hbreak2.exp: continue until exit at recursive next test
    
    gdb.log:
    
     (gdb) next
    
     Program received signal SIGTRAP, Trace/breakpoint trap.
     factorial (value=4) at ../../../src/gdb/testsuite/gdb.base/break.c:113
     113       if (value > 1) {  /* set breakpoint 7 here */
     (gdb) FAIL: gdb.base/hbreak2.exp: next over recursive call
    
    Actually, that patch just exposed a latent issue to "breakpoints
    always-inserted off" mode, not really caused it.  After that patch,
    GDB no longer removes breakpoints at each internal event, thus making
    some scenarios behave like breakpoint always-inserted on.  The bug is
    easy to trigger with always-inserted on.
    
    The issue is that since the target-side breakpoint conditions support,
    if the stub/server supports evaluating breakpoint conditions on the
    target side, then GDB is sending duplicate Zx packets to the target
    without removing them before, and GDBserver is not really expecting
    that for Z packets other than Z0/z0.  E.g., with "set breakpoint
    always-inserted on" and "set debug remote 1":
    
     (gdb) b main
     Sending packet: $m410943,1#ff...Packet received: 48
     Breakpoint 4 at 0x410943: file ../../../src/gdb/gdbserver/server.c, line 3028.
     Sending packet: $Z0,410943,1#48...Packet received: OK
                     ^^^^^^^^^^^^
     (gdb) b main
     Note: breakpoint 4 also set at pc 0x410943.
     Sending packet: $m410943,1#ff...Packet received: 48
     Breakpoint 5 at 0x410943: file ../../../src/gdb/gdbserver/server.c, line 3028.
     Sending packet: $Z0,410943,1#48...Packet received: OK
                     ^^^^^^^^^^^^
     (gdb) b main
     Note: breakpoints 4 and 5 also set at pc 0x410943.
     Sending packet: $m410943,1#ff...Packet received: 48
     Breakpoint 6 at 0x410943: file ../../../src/gdb/gdbserver/server.c, line 3028.
     Sending packet: $Z0,410943,1#48...Packet received: OK
                     ^^^^^^^^^^^^
     (gdb) del
     Delete all breakpoints? (y or n) y
     Sending packet: $Z0,410943,1#48...Packet received: OK
     Sending packet: $Z0,410943,1#48...Packet received: OK
     Sending packet: $z0,410943,1#68...Packet received: OK
    
    And for Z1, similarly:
    
     (gdb) hbreak main
     Sending packet: $m410943,1#ff...Packet received: 48
     Hardware assisted breakpoint 4 at 0x410943: file ../../../src/gdb/gdbserver/server.c, line 3028.
     Sending packet: $Z1,410943,1#49...Packet received: OK
                     ^^^^^^^^^^^^
     Packet Z1 (hardware-breakpoint) is supported
     (gdb) hbreak main
     Note: breakpoint 4 also set at pc 0x410943.
     Sending packet: $m410943,1#ff...Packet received: 48
     Hardware assisted breakpoint 5 at 0x410943: file ../../../src/gdb/gdbserver/server.c, line 3028.
     Sending packet: $Z1,410943,1#49...Packet received: OK
                     ^^^^^^^^^^^^
     (gdb) hbreak main
     Note: breakpoints 4 and 5 also set at pc 0x410943.
     Sending packet: $m410943,1#ff...Packet received: 48
     Hardware assisted breakpoint 6 at 0x410943: file ../../../src/gdb/gdbserver/server.c, line 3028.
     Sending packet: $Z1,410943,1#49...Packet received: OK
                     ^^^^^^^^^^^^
     (gdb) del
     Delete all breakpoints? (y or n) y
     Sending packet: $Z1,410943,1#49...Packet received: OK
                     ^^^^^^^^^^^^
     Sending packet: $Z1,410943,1#49...Packet received: OK
                     ^^^^^^^^^^^^
     Sending packet: $z1,410943,1#69...Packet received: OK
                     ^^^^^^^^^^^^
    
    So GDB sent a bunch of Z1 packets, and then when finally removing the
    breakpoint, only one z1 packet was sent.  On the GDBserver side (with
    monitor set debug-hw-points 1), in the Z1 case, we see:
    
     $ ./gdbserver :9999 ./gdbserver
     Process ./gdbserver created; pid = 8629
     Listening on port 9999
     Remote debugging from host 127.0.0.1
     insert_watchpoint (addr=410943, len=1, type=instruction-execute):
    	 CONTROL (DR7): 00000101          STATUS (DR6): 00000000
    	 DR0: addr=0x410943, ref.count=1  DR1: addr=0x0, ref.count=0
    	 DR2: addr=0x0, ref.count=0  DR3: addr=0x0, ref.count=0
     insert_watchpoint (addr=410943, len=1, type=instruction-execute):
    	 CONTROL (DR7): 00000101          STATUS (DR6): 00000000
    	 DR0: addr=0x410943, ref.count=2  DR1: addr=0x0, ref.count=0
    	 DR2: addr=0x0, ref.count=0  DR3: addr=0x0, ref.count=0
     insert_watchpoint (addr=410943, len=1, type=instruction-execute):
    	 CONTROL (DR7): 00000101          STATUS (DR6): 00000000
    	 DR0: addr=0x410943, ref.count=3  DR1: addr=0x0, ref.count=0
    	 DR2: addr=0x0, ref.count=0  DR3: addr=0x0, ref.count=0
     insert_watchpoint (addr=410943, len=1, type=instruction-execute):
    	 CONTROL (DR7): 00000101          STATUS (DR6): 00000000
    	 DR0: addr=0x410943, ref.count=4  DR1: addr=0x0, ref.count=0
    	 DR2: addr=0x0, ref.count=0  DR3: addr=0x0, ref.count=0
     insert_watchpoint (addr=410943, len=1, type=instruction-execute):
    	 CONTROL (DR7): 00000101          STATUS (DR6): 00000000
    	 DR0: addr=0x410943, ref.count=5  DR1: addr=0x0, ref.count=0
    	 DR2: addr=0x0, ref.count=0  DR3: addr=0x0, ref.count=0
     remove_watchpoint (addr=410943, len=1, type=instruction-execute):
    	 CONTROL (DR7): 00000101          STATUS (DR6): 00000000
    	 DR0: addr=0x410943, ref.count=4  DR1: addr=0x0, ref.count=0
    	 DR2: addr=0x0, ref.count=0  DR3: addr=0x0, ref.count=0
    
    That's one insert_watchpoint call for each Z1 packet, and then one
    remove_watchpoint call for the z1 packet.  Notice how ref.count
    increased for each insert_watchpoint call, and then in the end, after
    GDB told GDBserver to forget about the hardware breakpoint, GDBserver
    ends with the the first debug register still with ref.count=4!  IOW,
    the hardware breakpoint is left armed on the target, while on the GDB
    end it's gone.  If the program happens to execute 0x410943 afterwards,
    then the CPU traps, GDBserver reports the trap to GDB, and GDB not
    having a breakpoint set at that address anymore, reports to the user a
    spurious SIGTRAP.
    
    This is exactly what is happening in the hbreak2.exp test, though in
    that case, it's a shared library event that triggers a
    breakpoint_re_set, when breakpoints are still inserted (because
    nowadays GDB doesn't remove breakpoints while handling internal
    events), and that recreates breakpoint locations, which likewise
    forces breakpoint reinsertion and Zx packet resends...
    
    That is a lot of bogus Zx duplication that should possibly be
    addressed on the GDB side.  GDB resends Zx packets because the way to
    change the target-side condition, is to resend the breakpoint to the
    server with the new condition.  (That's an option in the packet: e.g.,
    "Z1,410943,1;X3,220027" for "hbreak main if 0".  The packets in the
    examples above are shorter because the breakpoints don't have
    conditions attached).  GDB doesn't remove the breakpoint first before
    reinserting it because that'd be bad for non-stop, as it'd open a
    window where the inferior could miss the breakpoint.  The conditions
    actually haven't changed between the resends, but GDB isn't smart
    enough to realize that.
    
    (TBC, if the target doesn't support target-side conditions, then GDB
    doesn't trigger these resends (init_bp_location calls
    mark_breakpoint_location_modified, and that does nothing if condition
    evaluation is on the host side.  The resends are caused by the
    'loc->condition_changed = condition_modified.'  line.)
    
    But, even if GDB was made smarter, GDBserver should really still
    handle the resends anyway.  So target-side conditions also aren't
    really to blame.  The documentation of the Z/z packets says:
    
     "To avoid potential problems with duplicate packets, the operations
     should be implemented in an idempotent way."
    
    As such, we may want to fix GDB, but we should definitely fix
    GDBserver.  The fix is a prerequisite for target-side conditions on
    hardware breakpoints anyway (and while at it, on watchpoints too).
    
    GDBserver indeed already treats duplicate Z0 packets in an idempotent
    way.  mem-break.c has the concept of high-level and low-level
    breakpoints, somewhat similar to GDB's split of breakpoints vs
    breakpoint locations, and keeps track of multiple breakpoints
    referencing the same address/location, for the case of an internal
    GDBserver breakpoint or a tracepoint being set at the same address as
    a GDB breakpoint.  But, it only allows GDB to ever contribute one
    reference to a software breakpoint location.  IOW, if gdbserver sees a
    Z0 packet for the same address where it already had a GDB breakpoint
    set, then GDBserver won't create another high-level GDB breakpoint.
    
    However, mem-break.c only tracks GDB Z0 breakpoints.  The same logic
    should apply to all kinds of Zx packets.  Currently, gdbserver passes
    down each duplicate Zx (other than Z0) request directly to the
    target->insert_point routine.  The x86 watchpoint support itself
    refcounts watchpoint / hw breakpoint requests, to handle overlapping
    watchpoints, and save debug registers.  But that code doesn't (and
    really shouldn't) handle the duplicate requests, assuming that for
    each insert there will be a corresponding remove.
    
    So the fix is to generalize mem-break.c to track all kinds of Zx
    breakpoints, and filter out duplicates.  As mentioned, this ends up
    adding support for target-side conditions on hardware breakpoints and
    watchpoints too (though GDB itself doesn't support the latter yet).
    
    Probably the least obvious change in the patch is that it kind of
    turns the breakpoint insert/remove APIs inside out.  Before, the
    target methods were only called for GDB breakpoints.  The internal
    breakpoint set/delete methods inserted memory breakpoints directly
    bypassing the insert/remove target methods.  That's not good when the
    target should use a debug API to set software breakpoints, instead of
    relying on GDBserver patching memory with breakpoint instructions, as
    is the case of NTO.
    
    Now removal/insertion of all kinds of breakpoints/watchpoints, either
    internal, or from GDB, always go through the target methods.  The
    insert_point/remove_point methods no longer get passed a Z packet
    type, but an internal/raw breakpoint type.  They're also passed a
    pointer to the raw breakpoint itself (note that's still opaque outside
    mem-break.c), so that insert_memory_breakpoint /
    remove_memory_breakpoint have access to the breakpoint's shadow
    buffer.  I first tried passing down a new structure based on GDB's
    "struct bp_target_info" (actually with that name exactly), but then
    decided against it as unnecessary complication.
    
    As software/memory breakpoints work by poking at memory, when setting
    a GDB Z0 breakpoint (but not internal breakpoints, as those can assume
    the conditions are already right), we need to tell the target to
    prepare to access memory (which on Linux means stop threads).  If that
    operation fails, we need to return error to GDB.  Seeing an error, if
    this is the first breakpoint of that type that GDB tries to insert,
    GDB would then assume the breakpoint type is supported, but it may
    actually not be.  So we need to check whether the type is supported at
    all before preparing to access memory.  And to solve that, the patch
    adds a new target->supports_z_point_type method that is called before
    actually trying to insert the breakpoint.
    
    Other than that, hopefully the change is more or less obvious.
    
    New test added that exercises the hbreak2.exp regression in a more
    direct way, without relying on a breakpoint re-set happening before
    main is reached.
    
    Tested by building GDBserver for:
    
     aarch64-linux-gnu
     arm-linux-gnueabihf
     i686-pc-linux-gnu
     i686-w64-mingw32
     m68k-linux-gnu
     mips-linux-gnu
     mips-uclinux
     nios2-linux-gnu
     powerpc-linux-gnu
     sh-linux-gnu
     tilegx-unknown-linux-gnu
     x86_64-redhat-linux
     x86_64-w64-mingw32
    
    And also regression tested on x86_64 Fedora 20.
    
    gdb/gdbserver/
    2014-05-20  Pedro Alves  <palves@redhat.com>
    
    	* linux-aarch64-low.c (aarch64_insert_point)
    	(aarch64_remove_point): No longer check whether the type is
    	supported here.  Adjust to new interface.
    	(the_low_target): Install aarch64_supports_z_point_type as
    	supports_z_point_type method.
    	* linux-arm-low.c (raw_bkpt_type_to_arm_hwbp_type): New function.
    	(arm_linux_hw_point_initialize): Take an enum raw_bkpt_type
    	instead of a Z packet char.  Adjust.
    	(arm_supports_z_point_type): New function.
    	(arm_insert_point, arm_remove_point): Adjust to new interface.
    	(the_low_target): Install arm_supports_z_point_type.
    	* linux-crisv32-low.c (cris_supports_z_point_type): New function.
    	(cris_insert_point, cris_remove_point): Adjust to new interface.
    	Don't check whether the type is supported here.
    	(the_low_target): Install cris_supports_z_point_type.
    	* linux-low.c (linux_supports_z_point_type): New function.
    	(linux_insert_point, linux_remove_point): Adjust to new interface.
    	* linux-low.h (struct linux_target_ops) <insert_point,
    	remove_point>: Take an enum raw_bkpt_type instead of a char.  Add
    	raw_breakpoint pointer parameter.
    	<supports_z_point_type>: New method.
    	* linux-mips-low.c (mips_supports_z_point_type): New function.
    	(mips_insert_point, mips_remove_point): Adjust to new interface.
    	Use mips_supports_z_point_type.
    	(the_low_target): Install mips_supports_z_point_type.
    	* linux-ppc-low.c (the_low_target): Install NULL as
    	supports_z_point_type method.
    	* linux-s390-low.c (the_low_target): Install NULL as
    	supports_z_point_type method.
    	* linux-sparc-low.c (the_low_target): Install NULL as
    	supports_z_point_type method.
    	* linux-x86-low.c (x86_supports_z_point_type): New function.
    	(x86_insert_point): Adjust to new insert_point interface.  Use
    	insert_memory_breakpoint.  Adjust to new
    	i386_low_insert_watchpoint interface.
    	(x86_remove_point): Adjust to remove_point interface.  Use
    	remove_memory_breakpoint.  Adjust to new
    	i386_low_remove_watchpoint interface.
    	(the_low_target): Install x86_supports_z_point_type.
    	* lynx-low.c (lynx_target_ops): Install NULL as
    	supports_z_point_type callback.
    	* nto-low.c (nto_supports_z_point_type): New.
    	(nto_insert_point, nto_remove_point): Adjust to new interface.
    	(nto_target_ops): Install nto_supports_z_point_type.
    	* mem-break.c: Adjust intro comment.
    	(struct raw_breakpoint) <raw_type, size>: New fields.
    	<inserted>: Update comment.
    	<shlib_disabled>: Delete field.
    	(enum bkpt_type) <gdb_breakpoint>: Delete value.
    	<gdb_breakpoint_Z0, gdb_breakpoint_Z1, gdb_breakpoint_Z2,
    	gdb_breakpoint_Z3, gdb_breakpoint_Z4>: New values.
    	(raw_bkpt_type_to_target_hw_bp_type): New function.
    	(find_enabled_raw_code_breakpoint_at): New function.
    	(find_raw_breakpoint_at): New type and size parameters.  Use them.
    	(insert_memory_breakpoint): New function, based off
    	set_raw_breakpoint_at.
    	(remove_memory_breakpoint): New function.
    	(set_raw_breakpoint_at): Reimplement.
    	(set_breakpoint): New, based on set_breakpoint_at.
    	(set_breakpoint_at): Reimplement.
    	(delete_raw_breakpoint): Go through the_target->remove_point
    	instead of assuming memory breakpoints.
    	(find_gdb_breakpoint_at): Delete.
    	(Z_packet_to_bkpt_type, Z_packet_to_raw_bkpt_type): New functions.
    	(find_gdb_breakpoint): New function.
    	(set_gdb_breakpoint_at): Delete.
    	(z_type_supported): New function.
    	(set_gdb_breakpoint_1): New function, loosely based off
    	set_gdb_breakpoint_at.
    	(check_gdb_bp_preconditions, set_gdb_breakpoint): New functions.
    	(delete_gdb_breakpoint_at): Delete.
    	(delete_gdb_breakpoint_1): New function, loosely based off
    	delete_gdb_breakpoint_at.
    	(delete_gdb_breakpoint): New function.
    	(clear_gdb_breakpoint_conditions): Rename to ...
    	(clear_breakpoint_conditions): ... this.  Don't handle a NULL
    	breakpoint.
    	(add_condition_to_breakpoint): Make static.
    	(add_breakpoint_condition): Take a struct breakpoint pointer
    	instead of an address.  Adjust.
    	(gdb_condition_true_at_breakpoint): Rename to ...
    	(gdb_condition_true_at_breakpoint_z_type): ... this, and add
    	z_type parameter.
    	(gdb_condition_true_at_breakpoint): Reimplement.
    	(add_breakpoint_commands): Take a struct breakpoint pointer
    	instead of an address.  Adjust.
    	(gdb_no_commands_at_breakpoint): Rename to ...
    	(gdb_no_commands_at_breakpoint_z_type): ... this.  Add z_type
    	parameter.  Return true if no breakpoint was found.  Change debug
    	output.
    	(gdb_no_commands_at_breakpoint): Reimplement.
    	(run_breakpoint_commands): Rename to ...
    	(run_breakpoint_commands_z_type): ... this.  Add z_type parameter,
    	and change return type to boolean.
    	(run_breakpoint_commands): New function.
    	(gdb_breakpoint_here): Also check for Z1 breakpoints.
    	(uninsert_raw_breakpoint): Don't try to reinsert a disabled
    	breakpoint.  Go through the_target->remove_point instead of
    	assuming memory breakpoint.
    	(uninsert_breakpoints_at, uninsert_all_breakpoints): Uninsert
    	software and hardware breakpoints.
    	(reinsert_raw_breakpoint): Go through the_target->insert_point
    	instead of assuming memory breakpoint.
    	(reinsert_breakpoints_at, reinsert_all_breakpoints): Reinsert
    	software and hardware breakpoints.
    	(check_breakpoints, breakpoint_here, breakpoint_inserted_here):
    	Check both software and hardware breakpoints.
    	(validate_inserted_breakpoint): Assert the breakpoint is a
    	software breakpoint.  Set the inserted flag to -1 instead of
    	setting shlib_disabled.
    	(delete_disabled_breakpoints): Adjust.
    	(validate_breakpoints): Only validate software breakpoints.
    	Adjust to inserted flag change.
    	(check_mem_read, check_mem_write): Skip breakpoint types other
    	than software breakpoints.  Adjust to inserted flag change.
    	* mem-break.h (enum raw_bkpt_type): New enum.
    	(raw_breakpoint, struct process_info): Forward declare.
    	(Z_packet_to_target_hw_bp_type): Delete declaration.
    	(raw_bkpt_type_to_target_hw_bp_type, Z_packet_to_raw_bkpt_type)
    	(set_gdb_breakpoint, delete_gdb_breakpoint)
    	(clear_breakpoint_conditions): New declarations.
    	(set_gdb_breakpoint_at, clear_gdb_breakpoint_conditions): Delete.
    	(breakpoint_inserted_here): Update comment.
    	(add_breakpoint_condition, add_breakpoint_commands): Replace
    	address parameter with a breakpoint pointer parameter.
    	(gdb_breakpoint_here): Update comment.
    	(delete_gdb_breakpoint_at): Delete.
    	(insert_memory_breakpoint, remove_memory_breakpoint): Declare.
    	* server.c (process_point_options): Take a struct breakpoint
    	pointer instead of an address.  Adjust.
    	(process_serial_event) <Z/z packets>: Use set_gdb_breakpoint and
    	delete_gdb_breakpoint.
    	* spu-low.c (spu_target_ops): Install NULL as
    	supports_z_point_type method.
    	* target.h: Include mem-break.h.
    	(struct target_ops) <prepare_to_access_memory>: Update comment.
    	<supports_z_point_type>: New field.
    	<insert_point, remove_point>: Take an enum raw_bkpt_type argument
    	instead of a char.  Also take a raw breakpoint pointer.
    	* win32-arm-low.c (the_low_target): Install NULL as
    	supports_z_point_type.
    	* win32-i386-low.c (i386_supports_z_point_type): New function.
    	(i386_insert_point, i386_remove_point): Adjust to new interface.
    	(the_low_target): Install i386_supports_z_point_type.
    	* win32-low.c (win32_supports_z_point_type): New function.
    	(win32_insert_point, win32_remove_point): Adjust to new interface.
    	(win32_target_ops): Install win32_supports_z_point_type.
    	* win32-low.h (struct win32_target_ops):
    	<supports_z_point_type>: New method.
    	<insert_point, remove_point>: Take an enum raw_bkpt_type argument
    	instead of a char.  Also take a raw breakpoint pointer.
    
    gdb/testsuite/
    2014-05-20  Pedro Alves  <palves@redhat.com>
    
    	* gdb.base/break-idempotent.c: New file.
    	* gdb.base/break-idempotent.exp: New file.

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=932539e3ab93db990ef61c80bbf78a4f2fdcc60b

commit 932539e3ab93db990ef61c80bbf78a4f2fdcc60b
Author: Pedro Alves <palves@redhat.com>
Date:   Tue May 20 18:24:27 2014 +0100

    [GDBserver] Move Z packet defines and type convertion routines to shared code.
    
    The Aarch64, MIPS and x86 Linux backends all have Z packet number
    defines and corresponding protocol number to internal type convertion
    routines.  Factor them all out to gdbserver's core code, so we only
    have one shared copy.
    
    Tested on x86_64 Fedora 20, and also cross built for aarch64-linux-gnu
    and mips-linux-gnu.
    
    gdb/gdbserver/
    2014-05-20  Pedro Alves  <palves@redhat.com>
    
    	* mem-break.h: Include break-common.h.
    	(Z_PACKET_SW_BP, Z_PACKET_HW_BP, Z_PACKET_WRITE_WP)
    	(Z_PACKET_READ_WP, Z_PACKET_ACCESS_WP): New defines.
    	(Z_packet_to_target_hw_bp_type): New declaration.
    	* mem-break.c (Z_packet_to_target_hw_bp_type): New function.
    	* i386-low.c (Z_PACKET_HW_BP, Z_PACKET_WRITE_WP, Z_PACKET_READ_WP)
    	(Z_PACKET_ACCESS_WP): Delete macros.
    	(Z_packet_to_hw_type): Delete function.
    	* i386-low.h: Don't include break-common.h here.
    	(Z_packet_to_hw_type): Delete declaration.
    	* linux-x86-low.c (x86_insert_point, x86_insert_point): Call
    	Z_packet_to_target_hw_bp_type instead of Z_packet_to_hw_type.
    	* win32-i386-low.c (i386_insert_point, i386_remove_point): Call
    	Z_packet_to_target_hw_bp_type instead of Z_packet_to_hw_type.
    	* linux-aarch64-low.c: Don't include break-common.h here.
    	(Z_PACKET_SW_BP, Z_PACKET_HW_BP, Z_PACKET_WRITE_WP)
    	(Z_PACKET_READ_WP, Z_PACKET_ACCESS_WP): Delete macros.
    	(Z_packet_to_target_hw_bp_type): Delete function.
    	* linux-mips-low.c (rsp_bp_type_to_target_hw_bp_type): Delete
    	function.
    	(mips_insert_point, mips_remove_point): Use
    	Z_packet_to_target_hw_bp_type.

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4ff0d3d82d37a57507c147886ce9f9bfb7495b99

commit 4ff0d3d82d37a57507c147886ce9f9bfb7495b99
Author: Pedro Alves <palves@redhat.com>
Date:   Tue May 20 18:24:27 2014 +0100

    [GDBserver][AArch64] Make watchpoint support use target_hw_bp_type.
    
    This makes linux-aarch64-low.c use target_hw_bp_type, like gdb's
    aarch64-linux-nat.c.  The original motivation is decoupling
    insert_point/remove_point from Z packet numbers, but I think making
    the files a little bit more similar is a good thing on its own right.
    Ideally we'd merge these files even...  The
    aarch64_point_encode_ctrl_reg change is taken straight from GDB's
    copy.
    
    I confirmed with a cross compiler that this builds, but it's otherwise
    untested.
    
    gdb/gdbserver/
    2014-05-20  Pedro Alves  <palves@redhat.com>
    
    	* linux-aarch64-low.c: Include break-common.h.
    	(enum target_point_type): Delete.
    	(Z_packet_to_point_type): Rename to ...
    	(Z_packet_to_target_hw_bp_type): ... this, and return a
    	target_hw_bp_type instead.
    	(aarch64_show_debug_reg_state): Take an enum target_hw_bp_type
    	instead of an enum target_point_type.
    	(aarch64_point_encode_ctrl_reg): Likewise.  Compute type mask from
    	breakpoint type.
    	(aarch64_dr_state_insert_one_point)
    	(aarch64_dr_state_remove_one_point, aarch64_handle_breakpoint)
    	(aarch64_handle_aligned_watchpoint)
    	(aarch64_handle_unaligned_watchpoint, aarch64_handle_watchpoint):
    	Take an enum target_hw_bp_type instead of an enum
    	target_point_type.
    	(aarch64_supports_z_point_type): New function.
    	(aarch64_insert_point, aarch64_remove_point): Use it.  Adjust to
    	use Z_packet_to_target_hw_bp_type.

-----------------------------------------------------------------------

Summary of changes:
 gdb/gdbserver/ChangeLog                     |  200 ++++++++
 gdb/gdbserver/i386-low.c                    |   23 -
 gdb/gdbserver/i386-low.h                    |    6 -
 gdb/gdbserver/linux-aarch64-low.c           |  121 ++---
 gdb/gdbserver/linux-arm-low.c               |   73 ++-
 gdb/gdbserver/linux-crisv32-low.c           |   63 +--
 gdb/gdbserver/linux-low.c                   |   18 +-
 gdb/gdbserver/linux-low.h                   |    8 +-
 gdb/gdbserver/linux-mips-low.c              |   59 +--
 gdb/gdbserver/linux-ppc-low.c               |    1 +
 gdb/gdbserver/linux-s390-low.c              |    1 +
 gdb/gdbserver/linux-sparc-low.c             |    1 +
 gdb/gdbserver/linux-x86-low.c               |   76 ++--
 gdb/gdbserver/lynx-low.c                    |    1 +
 gdb/gdbserver/mem-break.c                   |  732 +++++++++++++++++++++------
 gdb/gdbserver/mem-break.h                   |   86 +++-
 gdb/gdbserver/nto-low.c                     |   49 ++-
 gdb/gdbserver/server.c                      |   57 +--
 gdb/gdbserver/spu-low.c                     |    1 +
 gdb/gdbserver/target.h                      |   23 +-
 gdb/gdbserver/win32-arm-low.c               |    1 +
 gdb/gdbserver/win32-i386-low.c              |   42 +-
 gdb/gdbserver/win32-low.c                   |   18 +-
 gdb/gdbserver/win32-low.h                   |    7 +-
 gdb/testsuite/ChangeLog                     |    5 +
 gdb/testsuite/gdb.base/break-idempotent.c   |   52 ++
 gdb/testsuite/gdb.base/break-idempotent.exp |  181 +++++++
 27 files changed, 1409 insertions(+), 496 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/break-idempotent.c
 create mode 100644 gdb/testsuite/gdb.base/break-idempotent.exp

Hey Pedro,

Sorry it took me so long to act on it, but this patch causes a regression on ppc-aix.

New behavior is:

    (gdb) b func.adb:12
    Breakpoint 1 at 0x10000a24: file func.adb, line 12.
    (gdb) b func.adb:13
    Breakpoint 2 at 0x10000a28: file func.adb, line 13.
    (gdb) run
    Starting program: /[...]/func

    Breakpoint 1, func () at func.adb:12
    12        Nested;   -- break #1
    (gdb) c
    Continuing.
    [Inferior 1 (process 4128872) exited with code 02]

The expected behavior is for GDB to stop at line 13 (which happens to be
line 14):

    (gdb) c
    Breakpoint 2, func () at func.adb:14
    14      end;


This is just a heads-up; now that the GDB 7.8 branch is around the corner, I can't continue pushing this back anymore, so I will try my best to finish my investigation. According to my notes, this may be related to software-single-stepping, but I haven't looked closely enough yet.

The "debug infrun" traces during the continue are...

Before:

    infrun: stop_pc = 0x100009ac
    infrun: software single step trap for process 13500574
    infrun: no stepping, continue
    infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 13500574] at 0x100009ac
    infrun: prepare_to_wait
    infrun: target_wait (-1, status) =
    infrun:   13500574 [process 13500574],
    infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
    infrun: infwait_normal_state
    infrun: TARGET_WAITKIND_STOPPED
    infrun: stop_pc = 0x10000a28
    infrun: BPSTAT_WHAT_STOP_NOISY
    infrun: stop_stepping

After:

    infrun: clear_proceed_status_thread (process 4128872)
    infrun: proceed (addr=0xffffffff, signal=GDB_SIGNAL_DEFAULT, step=0)
    infrun: resume (step=1, signal=GDB_SIGNAL_0), trap_expected=1, current thread [process 4128872] at 0x10000a24
    infrun: wait_for_inferior ()
    infrun: target_wait (-1, status) =
    infrun:   4128872 [process 4128872],
    infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
    infrun: infwait_normal_state
    infrun: TARGET_WAITKIND_STOPPED
    infrun: stop_pc = 0x100009ac
    infrun: no stepping, continue
    infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 4128872] at 0x100009ac
    infrun: prepare_to_wait
    infrun: target_wait (-1, status) =
    infrun:   4128872 [process 4128872],
    infrun:   status->kind = exited, status = 2
    infrun: infwait_normal_state
    infrun: TARGET_WAITKIND_EXITED
    [Inferior 1 (process 4128872) exited with code 02]
    infrun: stop_stepping

I will investigate further and try to suggest a fix.

Odd.  It's as if the breakpoint at 0x10000a28 never got inserted at all.

We should really have a "set debug breakpoint" knob for this kind of issue.  :-/

"set debug target" might help though, it shows the target_insert_breakpoint/target_remove_breakpoint calls.

> Odd.  It's as if the breakpoint at 0x10000a28 never got inserted at all.

I am on it, but just to keep you entertained... :-)

It appears to be exactly what's going on. I am wondering if that may be
related to the fact that we have a user breakpoint at the same address
as one of the SSS breakpoints.  That's all a guess, since I haven't
really looked into details at which breakpoint insertions correspond
to what. But notice how we insert the breakpoint at 0x10000a28 twice?

(gdb) c
>>> First, we insert the user breakpoints (the second one is an internal
>>> breakpoint on __pthread_init). The first user breakpoint is not
>>> inserted as we need to step out of it first.
target_insert_breakpoint (0x0000000010000a28, xxx) = 0
target_insert_breakpoint (0x00000000d03f3800, xxx) = 0
>>> Then we proceed with the step-out-of-breakpoint...
infrun: resume (step=1, signal=GDB_SIGNAL_0), trap_expected=1, current thread [process 15335610] at 0x10000a24
>>> That's when we insert the SSS breakpoints...
target_insert_breakpoint (0x0000000010000a28, xxx) = 0
target_insert_breakpoint (0x00000000100009ac, xxx) = 0
>>> ... then let the inferior resume...
target_resume (15335610, continue, 0)
infrun: wait_for_inferior ()
target_wait (-1, status, options={}) = 15335610,   status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: target_wait (-1, status) =
infrun:   15335610 [process 15335610],
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x100009ac
>>> At this point, we stopped at the second SSS breakpoint...
target_stopped_by_watchpoint () = 0
>>> That must be us removing the SSS breakpoints...
target_remove_breakpoint (0x0000000010000a28, xxx) = 0
target_remove_breakpoint (0x00000000100009ac, xxx) = 0
target_stopped_by_watchpoint () = 0
>>> We find that we're not done, so we resume....
infrun: no stepping, continue
>>> And thus insert the userr breakpoints again, except we're not
>>> inserting the second breakpoint?!? I bet this is because we think
>>> it's still inserted, but in fact it got removed by the SSS bp
>>> removal earlier.
target_insert_breakpoint (0x0000000010000a24, xxx) = 0
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 15335610] at 0x100009ac
target_resume (-1, continue, 0)
infrun: prepare_to_wait
target_wait (-1, status, options={}) = 15335610,   status->kind = exited, status = 2

On 05/29/2014 06:47 PM, brobecker at adacore dot com wrote:>>>> >>> And thus insert the userr breakpoints again, except we're not
>>>> >>> inserting the second breakpoint?!? I bet this is because we think
>>>> >>> it's still inserted, but in fact it got removed by the SSS bp
>>>> >>> removal earlier.

Yeah, sounds like that's it, because removing a breakpoint
bypasses the shadow buffer overlaying (breakpoint_xfer_memory):

int
default_memory_remove_breakpoint (struct gdbarch *gdbarch,
				  struct bp_target_info *bp_tgt)
{
  return target_write_raw_memory (bp_tgt->placed_address, bp_tgt->shadow_contents,
				  bp_tgt->placed_size);
}

This is the sort of thing that'd be fixed if software
single-step breakpoints were in the generic global location
list framework, but, they're not.

I guess the simplest is to do something like:

int
deprecated_remove_raw_breakpoint (struct gdbarch *gdbarch, void *bp)
{
  struct bp_target_info *bp_tgt = bp;
-  int ret;
+  int ret = 0;

-  ret = target_remove_breakpoint (gdbarch, bp_tgt);
+  if (!software_breakpoint_inserted_here_p (bp_tgt->placed_aspace, 
+                                           bp_tgt->placed_address)
+    ret = target_remove_breakpoint (gdbarch, bp_tgt);
  xfree (bp_tgt);

  return ret;
}

But note software_breakpoint_inserted_here_p checks for software single-step
breakpoints too, so obviously a little refactoring is necessary.

> This is the sort of thing that'd be fixed if software
> single-step breakpoints were in the generic global location
> list framework, but, they're not.
> 
> I guess the simplest is to do something like:

I am testing a patch on AIX as we speak. Mine is based on the same
principles, but avoids inserting it in the first place.

I'd like to run the patch on Linux as well. I will do one run in
normal mode, without software single-step. But ISTR that you
implemented SSS for x86_64-linux as well? If possible, I can
also do a run to compare SSS results.

> But ISTR that you implemented SSS for x86_64-linux as well?
> If possible, I can also do a run to compare SSS results.

Yep, it's here:

 git@github.com:palves/gdb.git x86_software_single_step

I just rebased it on current mainline now, and regtested it to make sure it's still sane.

I opened bug 17000 for this regression now.  Closing this one.

> I opened bug 17000 for this regression now.  Closing this one.

Sounds good! Thanks for doing that, Pedro.

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  0718a8da7b359f184e1b3a866645cc2f9611771b (commit)
      from  89a1c21a1db9c2b7add8958531ccc3ff692a49eb (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=0718a8da7b359f184e1b3a866645cc2f9611771b

commit 0718a8da7b359f184e1b3a866645cc2f9611771b
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Aug 19 18:18:31 2014 +0100

    Remove stale comment
    
    This comment is no longer true for watchpoints since commit 31e77af2
    (PR breakpoints/7143 - Watchpoint does not trigger when first set).
    
    gdb/testsuite/
    
    	* gdb.base/watchpoint-hw-hit-once.c (main): Update comment.

-----------------------------------------------------------------------

Summary of changes:
 gdb/testsuite/ChangeLog                         |    4 ++++
 gdb/testsuite/gdb.base/watchpoint-hw-hit-once.c |    2 +-
 2 files changed, 5 insertions(+), 1 deletions(-)