This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[COMMIT PATCH] [remote] Insert breakpoints in the right process.


I noticed that gdb.multi/multi-arch.exp wasn't passing with
extended-remote GDBserver with my pending multi-process+multi-arch
series anymore on current mainline, while it used to pass before:

 (gdb) run
 Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.multi/ma-hangout
 Process /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.multi/ma-hangout created; pid = 32067
 Warning:
 Cannot insert breakpoint 2.
 Error accessing memory address 0x4005c2: Unknown error -1.
 Cannot insert breakpoint -1.
 Temporarily disabling shared library breakpoints:
 breakpoint #-1

 (gdb) FAIL: gdb.multi/multi-arch.exp: starting inferior 2


Investigating manually, I found an easy way to reproduce.  You just
need breakpoints on distinct inferiors, and a way to have GDB install
them in one go:

 (gdb) set breakpoint always-inserted on
 (gdb) info breakpoints
 Num     Type           Disp Enb Address            What
 2       breakpoint     del  n   <MULTIPLE>
 2.1                         y     0x00000000004005c2 in main at ../../../src/gdb/testsuite/gdb.multi/hello.c:40 inf 1
 2.2                         y     0x08048475         in main at ../../../src/gdb/testsuite/gdb.multi/hangout.c:22 inf 2
 (gdb) enable 2
 Warning:
 Cannot insert breakpoint 2.
 Error accessing memory address 0x4005c2: Unknown error -1.

And turning on remote debugging, we see:

 (gdb) set debug remote 1
 (gdb) disable 2
 (gdb) enable 2
 Sending packet: $Z0,4005c2,1#71...Packet received: E01
 Sending packet: $Z0,8048475,1#87...Packet received: OK
 Warning:
 Cannot insert breakpoint 2.
 Error accessing memory address 0x4005c2: Unknown error -1.

Notice that each of those Z0 breakpoints should be set in different
processes.  However, no Hg packet to select a process has been sent in
between, so GDBserver tries to plant both on the same process that
happens to be current.  The first Z0 then not so surprisingly fails.
IOW, the blame is on GDB, for telling GDBserver to plant both
breakpoints in the same process.

remote.c has a lazy scheme where it keeps a local cache of the
remote's selected general thread, and delays updating it on the remote
side until necessary (memory/register reads/writes, etc.).  This is
done to reduce RSP traffic.  The bug is that the Zx breakpoint
insert/remove methods weren't committing the selected thread/process
back to the remote side:

 Breakpoint 3, remote_insert_breakpoint (gdbarch=0x1383ae0, bp_tgt=0x140c2b0) at ../../src/gdb/remote.c:8148
 8148      if (remote_protocol_packets[PACKET_Z0].support != PACKET_DISABLE)
 (top-gdb) p inferior_ptid
 $3 = {pid = 3670, lwp = 0, tid = 3670}
 (top-gdb) p general_thread
 $4 = {pid = 3671, lwp = 0, tid = 3671}

IOW, a call to set_general_process is missing.

I did some auditing over remote.c, and added calls to all places I
found missing it.

This only used to work by chance before.  breakpoint.c switches to a
thread of the target process before installing a breakpoint location.
That calls switch_to_thread.  Before:

 2012-07-27  Yao Qi  <yao@codesourcery.com>

         * thread.c (switch_to_thread): Don't call registers_changed.

that caused the register caches to all be flushed and refetched before
installing the breakpoint location.  Given fetching registers commits
the remote general thread (with Hg), masking out the latent bug.

Tested on x86_64 Fedora 17 with GDBserver.

gdb/
2013-05-29  Pedro Alves  <palves@redhat.com>

	* remote.c (remote_insert_breakpoint, remote_remove_breakpoint)
	(remote_insert_watchpoint, remote_remove_watchpoint)
	(remote_insert_hw_breakpoint, remote_remove_hw_breakpoint)
	(remote_verify_memory, compare_sections_command)
	(remote_search_memory): Set the general process/thread on the
	remote side.
---
 gdb/remote.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/gdb/remote.c b/gdb/remote.c
index 658fae2..9a529cf 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8153,6 +8153,11 @@ remote_insert_breakpoint (struct gdbarch *gdbarch,
       int bpsize;
       struct condition_list *cond = NULL;
 
+      /* Make sure the remote is pointing at the right process, if
+	 necessary.  */
+      if (!gdbarch_has_global_breakpoints (target_gdbarch ()))
+	set_general_process ();
+
       gdbarch_remote_breakpoint_from_pc (gdbarch, &addr, &bpsize);
 
       rs = get_remote_state ();
@@ -8203,6 +8208,11 @@ remote_remove_breakpoint (struct gdbarch *gdbarch,
       char *p = rs->buf;
       char *endbuf = rs->buf + get_remote_packet_size ();
 
+      /* Make sure the remote is pointing at the right process, if
+	 necessary.  */
+      if (!gdbarch_has_global_breakpoints (target_gdbarch ()))
+	set_general_process ();
+
       *(p++) = 'z';
       *(p++) = '0';
       *(p++) = ',';
@@ -8252,6 +8262,11 @@ remote_insert_watchpoint (CORE_ADDR addr, int len, int type,
   if (remote_protocol_packets[PACKET_Z0 + packet].support == PACKET_DISABLE)
     return 1;
 
+  /* Make sure the remote is pointing at the right process, if
+     necessary.  */
+  if (!gdbarch_has_global_breakpoints (target_gdbarch ()))
+    set_general_process ();
+
   xsnprintf (rs->buf, endbuf - rs->buf, "Z%x,", packet);
   p = strchr (rs->buf, '\0');
   addr = remote_address_masked (addr);
@@ -8296,6 +8311,11 @@ remote_remove_watchpoint (CORE_ADDR addr, int len, int type,
   if (remote_protocol_packets[PACKET_Z0 + packet].support == PACKET_DISABLE)
     return -1;
 
+  /* Make sure the remote is pointing at the right process, if
+     necessary.  */
+  if (!gdbarch_has_global_breakpoints (target_gdbarch ()))
+    set_general_process ();
+
   xsnprintf (rs->buf, endbuf - rs->buf, "z%x,", packet);
   p = strchr (rs->buf, '\0');
   addr = remote_address_masked (addr);
@@ -8399,6 +8419,11 @@ remote_insert_hw_breakpoint (struct gdbarch *gdbarch,
   if (remote_protocol_packets[PACKET_Z1].support == PACKET_DISABLE)
     return -1;
 
+  /* Make sure the remote is pointing at the right process, if
+     necessary.  */
+  if (!gdbarch_has_global_breakpoints (target_gdbarch ()))
+    set_general_process ();
+
   rs = get_remote_state ();
   p = rs->buf;
   endbuf = rs->buf + get_remote_packet_size ();
@@ -8452,6 +8477,11 @@ remote_remove_hw_breakpoint (struct gdbarch *gdbarch,
   if (remote_protocol_packets[PACKET_Z1].support == PACKET_DISABLE)
     return -1;
 
+  /* Make sure the remote is pointing at the right process, if
+     necessary.  */
+  if (!gdbarch_has_global_breakpoints (target_gdbarch ()))
+    set_general_process ();
+
   *(p++) = 'z';
   *(p++) = '1';
   *(p++) = ',';
@@ -8515,6 +8545,9 @@ remote_verify_memory (struct target_ops *ops,
   unsigned long host_crc, target_crc;
   char *tmp;
 
+  /* Make sure the remote is pointing at the right process.  */
+  set_general_process ();
+
   /* FIXME: assumes lma can fit into long.  */
   xsnprintf (rs->buf, get_remote_packet_size (), "qCRC:%lx,%lx",
 	     (long) lma, (long) size);
@@ -8559,6 +8592,9 @@ compare_sections_command (char *args, int from_tty)
   if (!exec_bfd)
     error (_("command cannot be used without an exec file"));
 
+  /* Make sure the remote is pointing at the right process.  */
+  set_general_process ();
+
   for (s = exec_bfd->sections; s; s = s->next)
     {
       if (!(s->flags & SEC_LOAD))
@@ -8985,6 +9021,9 @@ remote_search_memory (struct target_ops* ops,
 				   pattern, pattern_len, found_addrp);
     }
 
+  /* Make sure the remote is pointing at the right process.  */
+  set_general_process ();
+
   /* Insert header.  */
   i = snprintf (rs->buf, max_size, 
 		"qSearch:memory:%s;%s;",


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]