diff options
author | Pedro Alves <palves@redhat.com> | 2017-10-03 13:26:42 +0100 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2017-10-03 13:26:42 +0100 |
commit | 8bcea3ea32a55a3cfac29dde1c97083843047e45 (patch) | |
tree | 760a75c05232ff4b4bb10b69844e2ec1a4dbf128 | |
parent | a33e561329042abde7a22540be1a32714375b360 (diff) | |
download | binutils-gdb-users/palves/remote-multi-inferior-fixes.tar.gz |
Fix "Remote 'g' packet reply is too long" problems with multiple inferiorsusers/palves/remote-multi-inferior-fixes
When debugging two inferiors (or more) against gdbserver, and the
inferiors have different architectures, such as e.g., on x86_64
GNU/Linux and one inferior is 64-bit while the other is 32-bit, then
GDB can get confused with the different architectures in a couple
spots.
In both cases I ran into, GDB incorrectly ended up using the
architecture of whatever happens to be the selected inferior instead
of the architecture of some other given inferior:
#1 - When parsing the expedited registers in stop replies.
#2 - In the default implementation of the target_thread_architecture
target method.
These resulted in instances of the infamous "Remote 'g' packet reply
is too long" error. For example, with the test added in this commit,
we get:
~~~
Continuing.
Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): ad064000000000000[snip]
(gdb) FAIL: gdb.multi/multi-arch.exp: inf1 event with inf2 selected: continue to hello_loop
c
Continuing.
Truncated register 50 in remote 'g' packet
(gdb) PASS: gdb.multi/multi-arch.exp: inf2 event with inf1 selected: c
~~~
This commit fixes that.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* remote.c (get_remote_arch_state): New 'gdbarch' parameter. Use
it instead of target_gdbarch.
(get_remote_state, get_remote_packet_size): Adjust
get_remote_arch_state calls, passing down target_gdbarch
explicitly.
(packet_reg_from_regnum, packet_reg_from_pnum): New parameter
'gdbarch' and use it instead of target_gdbarch.
(get_memory_packet_size): Adjust get_remote_arch_state calls,
passing down target_gdbarch explicitly.
(struct stop_reply) <arch>: New field.
(remote_parse_stop_reply): Use the stopped thread's architecture,
not the current inferior's. Save the architecture in the
stop_reply.
(process_stop_reply): Use the stop reply's architecture.
(process_g_packet, remote_fetch_registers)
(remote_prepare_to_store, store_registers_using_G)
(remote_store_registers): Adjust get_remote_arch_state calls,
using the regcache's architecture.
(remote_get_trace_status): Adjust get_remote_arch_state calls,
passing down target_gdbarch explicitly.
* spu-multiarch.c (spu_thread_architecture): Defer to the target
beneath instead of calling target_gdbarch.
* target.c (default_thread_architecture): Use the specified
inferior's architecture, instead of the current inferior's
architecture (via target_gdbarch).
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* gdb.multi/hangout.c: Include <unistd.h>.
(hangout_loop): New function.
(main): Call alarm. Call hangout_loop in a loop.
* gdb.multi/hello.c: Include <unistd.h>.
(hello_loop): New function.
(main): Call alarm. Call hangout_loop in a loop.
* gdb.multi/multi-arch.exp: Test running to a breakpoint one
inferior with the other selected.
-rw-r--r-- | gdb/remote.c | 100 | ||||
-rw-r--r-- | gdb/spu-multiarch.c | 3 | ||||
-rw-r--r-- | gdb/target.c | 4 | ||||
-rw-r--r-- | gdb/testsuite/gdb.multi/hangout.c | 14 | ||||
-rw-r--r-- | gdb/testsuite/gdb.multi/hello.c | 15 | ||||
-rw-r--r-- | gdb/testsuite/gdb.multi/multi-arch.exp | 24 |
6 files changed, 130 insertions, 30 deletions
diff --git a/gdb/remote.c b/gdb/remote.c index 54d810f0543..9bc69c29d06 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -654,11 +654,11 @@ remote_get_noisy_reply () static struct gdbarch_data *remote_gdbarch_data_handle; static struct remote_arch_state * -get_remote_arch_state (void) +get_remote_arch_state (struct gdbarch *gdbarch) { - gdb_assert (target_gdbarch () != NULL); + gdb_assert (gdbarch != NULL); return ((struct remote_arch_state *) - gdbarch_data (target_gdbarch (), remote_gdbarch_data_handle)); + gdbarch_data (gdbarch, remote_gdbarch_data_handle)); } /* Fetch the global remote target state. */ @@ -671,7 +671,7 @@ get_remote_state (void) function which calls getpkt also needs to be mindful of changes to rs->buf, but this call limits the number of places which run into trouble. */ - get_remote_arch_state (); + get_remote_arch_state (target_gdbarch ()); return get_remote_state_raw (); } @@ -878,7 +878,7 @@ static long get_remote_packet_size (void) { struct remote_state *rs = get_remote_state (); - struct remote_arch_state *rsa = get_remote_arch_state (); + remote_arch_state *rsa = get_remote_arch_state (target_gdbarch ()); if (rs->explicit_packet_size) return rs->explicit_packet_size; @@ -887,9 +887,10 @@ get_remote_packet_size (void) } static struct packet_reg * -packet_reg_from_regnum (struct remote_arch_state *rsa, long regnum) +packet_reg_from_regnum (struct gdbarch *gdbarch, struct remote_arch_state *rsa, + long regnum) { - if (regnum < 0 && regnum >= gdbarch_num_regs (target_gdbarch ())) + if (regnum < 0 && regnum >= gdbarch_num_regs (gdbarch)) return NULL; else { @@ -901,11 +902,12 @@ packet_reg_from_regnum (struct remote_arch_state *rsa, long regnum) } static struct packet_reg * -packet_reg_from_pnum (struct remote_arch_state *rsa, LONGEST pnum) +packet_reg_from_pnum (struct gdbarch *gdbarch, struct remote_arch_state *rsa, + LONGEST pnum) { int i; - for (i = 0; i < gdbarch_num_regs (target_gdbarch ()); i++) + for (i = 0; i < gdbarch_num_regs (gdbarch); i++) { struct packet_reg *r = &rsa->regs[i]; @@ -1049,7 +1051,7 @@ static long get_memory_packet_size (struct memory_packet_config *config) { struct remote_state *rs = get_remote_state (); - struct remote_arch_state *rsa = get_remote_arch_state (); + remote_arch_state *rsa = get_remote_arch_state (target_gdbarch ()); long what_they_get; if (config->fixed_p) @@ -6361,6 +6363,9 @@ typedef struct stop_reply struct target_waitstatus ws; + /* The architecture associated with the expedited registers. */ + gdbarch *arch; + /* Expedited registers. This makes remote debugging a bit more efficient for those targets that provide critical registers as part of their normal status mechanism (as another roundtrip to @@ -6835,7 +6840,7 @@ strprefix (const char *p, const char *pend, const char *prefix) static void remote_parse_stop_reply (char *buf, struct stop_reply *event) { - struct remote_arch_state *rsa = get_remote_arch_state (); + remote_arch_state *rsa = NULL; ULONGEST addr; const char *p; int skipregs = 0; @@ -7015,9 +7020,47 @@ Packet: '%s'\n"), reason. */ if (p_temp == p1) { - struct packet_reg *reg = packet_reg_from_pnum (rsa, pnum); + /* If we haven't parsed the event's thread yet, find + it now, in order to find the architecture of the + reported expedited registers. */ + if (event->ptid == null_ptid) + { + const char *thr = strstr (p1 + 1, ";thread:"); + if (thr != NULL) + event->ptid = read_ptid (thr + strlen (";thread:"), + NULL); + else + event->ptid = magic_null_ptid; + } + + if (rsa == NULL) + { + inferior *inf = (event->ptid == null_ptid + ? NULL + : find_inferior_ptid (event->ptid)); + /* If this is the first time we learn anything + about this process, skip the registers + included in this packet, since we don't yet + know which architecture to use to parse them. + We'll determine the architecture later when + we process the stop reply and retrieve the + target description, via + remote_notice_new_inferior -> + post_create_inferior. */ + if (inf == NULL) + { + p = strchrnul (p1 + 1, ';'); + p++; + continue; + } + + event->arch = inf->gdbarch; + rsa = get_remote_arch_state (event->arch); + } + + packet_reg *reg + = packet_reg_from_pnum (event->arch, rsa, pnum); cached_reg_t cached_reg; - struct gdbarch *gdbarch = target_gdbarch (); if (reg == NULL) error (_("Remote sent bad register number %s: %s\n\ @@ -7026,13 +7069,13 @@ Packet: '%s'\n"), cached_reg.num = reg->regnum; cached_reg.data = (gdb_byte *) - xmalloc (register_size (gdbarch, reg->regnum)); + xmalloc (register_size (event->arch, reg->regnum)); p = p1 + 1; fieldsize = hex2bin (p, cached_reg.data, - register_size (gdbarch, reg->regnum)); + register_size (event->arch, reg->regnum)); p += 2 * fieldsize; - if (fieldsize < register_size (gdbarch, reg->regnum)) + if (fieldsize < register_size (event->arch, reg->regnum)) warning (_("Remote reply is too short: %s"), buf); VEC_safe_push (cached_reg_t, event->regcache, &cached_reg); @@ -7248,7 +7291,7 @@ process_stop_reply (struct stop_reply *stop_reply, if (stop_reply->regcache) { struct regcache *regcache - = get_thread_arch_regcache (ptid, target_gdbarch ()); + = get_thread_arch_regcache (ptid, stop_reply->arch); cached_reg_t *reg; int ix; @@ -7605,7 +7648,7 @@ process_g_packet (struct regcache *regcache) { struct gdbarch *gdbarch = get_regcache_arch (regcache); struct remote_state *rs = get_remote_state (); - struct remote_arch_state *rsa = get_remote_arch_state (); + remote_arch_state *rsa = get_remote_arch_state (gdbarch); int i, buf_len; char *p; char *regs; @@ -7739,7 +7782,8 @@ static void remote_fetch_registers (struct target_ops *ops, struct regcache *regcache, int regnum) { - struct remote_arch_state *rsa = get_remote_arch_state (); + struct gdbarch *gdbarch = get_regcache_arch (regcache); + remote_arch_state *rsa = get_remote_arch_state (gdbarch); int i; set_remote_traceframe (); @@ -7747,7 +7791,7 @@ remote_fetch_registers (struct target_ops *ops, if (regnum >= 0) { - struct packet_reg *reg = packet_reg_from_regnum (rsa, regnum); + packet_reg *reg = packet_reg_from_regnum (gdbarch, rsa, regnum); gdb_assert (reg != NULL); @@ -7773,7 +7817,7 @@ remote_fetch_registers (struct target_ops *ops, fetch_registers_using_g (regcache); - for (i = 0; i < gdbarch_num_regs (get_regcache_arch (regcache)); i++) + for (i = 0; i < gdbarch_num_regs (gdbarch); i++) if (!rsa->regs[i].in_g_packet) if (!fetch_register_using_p (regcache, &rsa->regs[i])) { @@ -7789,7 +7833,7 @@ remote_fetch_registers (struct target_ops *ops, static void remote_prepare_to_store (struct target_ops *self, struct regcache *regcache) { - struct remote_arch_state *rsa = get_remote_arch_state (); + remote_arch_state *rsa = get_remote_arch_state (regcache->arch ()); int i; /* Make sure the entire registers array is valid. */ @@ -7855,7 +7899,7 @@ static void store_registers_using_G (const struct regcache *regcache) { struct remote_state *rs = get_remote_state (); - struct remote_arch_state *rsa = get_remote_arch_state (); + remote_arch_state *rsa = get_remote_arch_state (regcache->arch ()); gdb_byte *regs; char *p; @@ -7894,7 +7938,8 @@ static void remote_store_registers (struct target_ops *ops, struct regcache *regcache, int regnum) { - struct remote_arch_state *rsa = get_remote_arch_state (); + struct gdbarch *gdbarch = regcache->arch (); + remote_arch_state *rsa = get_remote_arch_state (gdbarch); int i; set_remote_traceframe (); @@ -7902,7 +7947,7 @@ remote_store_registers (struct target_ops *ops, if (regnum >= 0) { - struct packet_reg *reg = packet_reg_from_regnum (rsa, regnum); + packet_reg *reg = packet_reg_from_regnum (gdbarch, rsa, regnum); gdb_assert (reg != NULL); @@ -7926,7 +7971,7 @@ remote_store_registers (struct target_ops *ops, store_registers_using_G (regcache); - for (i = 0; i < gdbarch_num_regs (get_regcache_arch (regcache)); i++) + for (i = 0; i < gdbarch_num_regs (gdbarch); i++) if (!rsa->regs[i].in_g_packet) if (!store_register_using_P (regcache, &rsa->regs[i])) /* See above for why we do not issue an error here. */ @@ -12653,7 +12698,8 @@ remote_get_trace_status (struct target_ops *self, struct trace_status *ts) if (packet_support (PACKET_qTStatus) == PACKET_DISABLE) return -1; - trace_regblock_size = get_remote_arch_state ()->sizeof_g_packet; + trace_regblock_size + = get_remote_arch_state (target_gdbarch ())->sizeof_g_packet; putpkt ("qTStatus"); diff --git a/gdb/spu-multiarch.c b/gdb/spu-multiarch.c index a935a72c305..e521c7e6ac2 100644 --- a/gdb/spu-multiarch.c +++ b/gdb/spu-multiarch.c @@ -121,7 +121,8 @@ spu_thread_architecture (struct target_ops *ops, ptid_t ptid) if (parse_spufs_run (ptid, &spufs_fd, &spufs_addr)) return spu_gdbarch (spufs_fd); - return target_gdbarch (); + target_ops *beneath = find_target_beneath (ops); + return beneath->to_thread_architecture (beneath, ptid); } /* Override the to_region_ok_for_hw_watchpoint routine. */ diff --git a/gdb/target.c b/gdb/target.c index aded0ba34f1..f2dccd23ced 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -3137,7 +3137,9 @@ default_watchpoint_addr_within_range (struct target_ops *target, static struct gdbarch * default_thread_architecture (struct target_ops *ops, ptid_t ptid) { - return target_gdbarch (); + inferior *inf = find_inferior_ptid (ptid); + gdb_assert (inf != NULL); + return inf->gdbarch; } static int diff --git a/gdb/testsuite/gdb.multi/hangout.c b/gdb/testsuite/gdb.multi/hangout.c index 44dfba68937..7266a188f85 100644 --- a/gdb/testsuite/gdb.multi/hangout.c +++ b/gdb/testsuite/gdb.multi/hangout.c @@ -16,14 +16,28 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <stdio.h> +#include <unistd.h> + +static void +hangout_loop (void) +{ +} int main(int argc, char *argv[]) { int i; + alarm (30); + for (i = 0; i < argc; ++i) { printf("Arg %d is %s\n", i, argv[i]); } + + while (1) + { + hangout_loop (); + usleep (20); + } } diff --git a/gdb/testsuite/gdb.multi/hello.c b/gdb/testsuite/gdb.multi/hello.c index 2c5bee99b5c..7cc5f3c32bf 100644 --- a/gdb/testsuite/gdb.multi/hello.c +++ b/gdb/testsuite/gdb.multi/hello.c @@ -16,6 +16,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <stdlib.h> +#include <unistd.h> short hglob = 1; @@ -37,15 +38,27 @@ hello(int x) return x + 45; } +static void +hello_loop (void) +{ +} + int main() { int tmpx; + alarm (30); + bar(); tmpx = hello(glob); commonfun(); glob = tmpx; commonfun(); -} + while (1) + { + hello_loop (); + usleep (20); + } +} diff --git a/gdb/testsuite/gdb.multi/multi-arch.exp b/gdb/testsuite/gdb.multi/multi-arch.exp index d73b4f7aeec..733afa098c0 100644 --- a/gdb/testsuite/gdb.multi/multi-arch.exp +++ b/gdb/testsuite/gdb.multi/multi-arch.exp @@ -96,3 +96,27 @@ if ![runto_main] then { gdb_test "info inferiors" \ "Executable.*${exec1}.*${exec2}.*" + +# Now select inferior 2, and trigger an event in inferior 1. This +# tries to check that GDB doesn't incorrectly uses the architecture of +# inferior 2 when parsing the expedited registers in a stop reply for +# inferior 1 (when remote debugging). + +gdb_test_no_output "set schedule-multiple on" + +with_test_prefix "inf1 event with inf2 selected" { + gdb_test "inferior 2" "Switching to inferior 2.*thread 2\.1.*main.*${srcfile2}.*" + gdb_test "b hello_loop" "Breakpoint .* at .*${srcfile1}.*" + gdb_test "c" " hello_loop.*" "continue to hello_loop" +} + +delete_breakpoints + +# Same, but the other way around: select inferior 1 and trigger an +# event in inferior 2. + +with_test_prefix "inf2 event with inf1 selected" { + gdb_test "inferior 1" "Switching to inferior 1.*thread 1\.1.*hello_loop.*${srcfile1}.*" + gdb_test "b hangout_loop" "Breakpoint .* at .*${srcfile2}.*" + gdb_test "c" " hangout_loop.*" "continue to hangout_loop" +} |