From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39611 invoked by alias); 2 Oct 2017 15:15:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 39494 invoked by uid 89); 2 Oct 2017 15:15:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Oct 2017 15:15:16 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9EB3713AB2 for ; Mon, 2 Oct 2017 15:15:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9EB3713AB2 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from cascais.lan (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA11282A28 for ; Mon, 2 Oct 2017 15:15:14 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 3/3] Fix "Remote 'g' packet reply is too long" problems with multiple inferiors Date: Mon, 02 Oct 2017 15:15:00 -0000 Message-Id: <1506957311-30028-4-git-send-email-palves@redhat.com> In-Reply-To: <1506957311-30028-1-git-send-email-palves@redhat.com> References: <1506957311-30028-1-git-send-email-palves@redhat.com> X-SW-Source: 2017-10/txt/msg00020.txt.bz2 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: 2017-10-02 Pedro Alves * 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. (remote_parse_stop_reply): Use the stopped thread's architecture, not the current inferior's. (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. * target.c (default_thread_architecture): Use the sepecified inferior's architecture, instead of the current inferior's architecture (via target_gdbarch). gdb/testsuite/ChangeLog: 2017-10-02 Pedro Alves * gdb.multi/hangout.c: Include . (hangout_loop): New function. (main): Call alarm. Call hangout_loop in a loop. * gdb.multi/hello.c: Include . (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. --- gdb/remote.c | 92 ++++++++++++++++++++++++---------- gdb/target.c | 4 +- gdb/testsuite/gdb.multi/hangout.c | 14 ++++++ gdb/testsuite/gdb.multi/hello.c | 15 +++++- gdb/testsuite/gdb.multi/multi-arch.exp | 24 +++++++++ 5 files changed, 121 insertions(+), 28 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 54d810f..b6a81a2 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 (); + struct 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 (); + struct remote_arch_state *rsa = get_remote_arch_state (target_gdbarch ()); long what_they_get; if (config->fixed_p) @@ -6835,7 +6837,8 @@ 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; + struct gdbarch *reply_arch = NULL; ULONGEST addr; const char *p; int skipregs = 0; @@ -7015,9 +7018,43 @@ 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. */ + if (inf == NULL) + { + p = strchrnul (p1 + 1, ';'); + p++; + continue; + } + + reply_arch = inf->gdbarch; + rsa = get_remote_arch_state (reply_arch); + } + + struct packet_reg *reg = packet_reg_from_pnum (reply_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 +7063,13 @@ Packet: '%s'\n"), cached_reg.num = reg->regnum; cached_reg.data = (gdb_byte *) - xmalloc (register_size (gdbarch, reg->regnum)); + xmalloc (register_size (reply_arch, reg->regnum)); p = p1 + 1; fieldsize = hex2bin (p, cached_reg.data, - register_size (gdbarch, reg->regnum)); + register_size (reply_arch, reg->regnum)); p += 2 * fieldsize; - if (fieldsize < register_size (gdbarch, reg->regnum)) + if (fieldsize < register_size (reply_arch, reg->regnum)) warning (_("Remote reply is too short: %s"), buf); VEC_safe_push (cached_reg_t, event->regcache, &cached_reg); @@ -7605,7 +7642,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 (); + struct remote_arch_state *rsa = get_remote_arch_state (gdbarch); int i, buf_len; char *p; char *regs; @@ -7739,7 +7776,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 +7785,7 @@ remote_fetch_registers (struct target_ops *ops, if (regnum >= 0) { - struct packet_reg *reg = packet_reg_from_regnum (rsa, regnum); + struct packet_reg *reg = packet_reg_from_regnum (gdbarch, rsa, regnum); gdb_assert (reg != NULL); @@ -7773,7 +7811,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 +7827,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 (); + struct remote_arch_state *rsa = get_remote_arch_state (regcache->arch ()); int i; /* Make sure the entire registers array is valid. */ @@ -7855,7 +7893,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 +7932,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 +7941,7 @@ remote_store_registers (struct target_ops *ops, if (regnum >= 0) { - struct packet_reg *reg = packet_reg_from_regnum (rsa, regnum); + struct packet_reg *reg = packet_reg_from_regnum (gdbarch, rsa, regnum); gdb_assert (reg != NULL); @@ -7926,7 +7965,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 +12692,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/target.c b/gdb/target.c index 733c086..cb66158 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -3131,7 +3131,9 @@ default_watchpoint_addr_within_range (struct target_ops *target, 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 44dfba6..7266a18 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 . */ #include +#include + +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 2c5bee9..7cc5f3c 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 . */ #include +#include 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 d73b4f7..733afa0 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" +} -- 2.5.5