From 28439a30acd9aaafe5d7dc75573b3629533cb873 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 29 May 2013 11:57:48 +0000 Subject: [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 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 * 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 * 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/ChangeLog | 9 +++++++++ gdb/remote.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f520669..395326b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,14 @@ 2013-05-29 Pedro Alves + * 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. + +2013-05-29 Pedro Alves + * aarch64-tdep.c: Don't include "features/aarch64-without-fpu.c". (_initialize_aarch64_tdep): Don't call initialize_tdesc_aarch64_without_fpu. 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;", -- 2.7.4