From ac9c87f8ecce2dfc0ac6067eb26524c0027a9a83 Mon Sep 17 00:00:00 2001 From: Daniel Jacobowitz Date: Thu, 23 Mar 2006 21:49:58 +0000 Subject: Update g/G packet support. --- gdb/arm-tdep.c | 2 - gdb/available.c | 17 +++--- gdb/gdbarch.c | 32 ----------- gdb/gdbarch.h | 10 ---- gdb/gdbarch.sh | 5 -- gdb/remote.c | 175 +++++++++++++++++++++++++++++++++++++------------------- 6 files changed, 125 insertions(+), 116 deletions(-) diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 8a086907233..f8f4dd6ffbb 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -2824,8 +2824,6 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_gdbarch_sp_regnum (gdbarch, ARM_SP_REGNUM); set_gdbarch_pc_regnum (gdbarch, ARM_PC_REGNUM); set_gdbarch_num_regs (gdbarch, NUM_GREGS + NUM_FREGS + NUM_SREGS); - set_gdbarch_remote_num_g_packet_regs (gdbarch, - NUM_GREGS + NUM_FREGS + NUM_SREGS); set_gdbarch_register_type (gdbarch, arm_register_type); if (info.feature_set) diff --git a/gdb/available.c b/gdb/available.c index 0fa69a5dbf2..def0c02b4ed 100644 --- a/gdb/available.c +++ b/gdb/available.c @@ -49,12 +49,10 @@ - Handle unexpected changes to _num_regs. - Call record_available_features from _gdbarch_init. - Do not override the default _register_byte - - Provide gdbarch_remote_num_g_packet_regs -*/ -/* FIXME: Everywhere we call internal_error from this file leads to a failure - to initialize a gdbarch, which leads to later failures when we expect - e.g. current_regcache to have been initialized. */ + (WARNING: This list is out of date and should be redone before submission. + And moved into gdbint.texi.) +*/ @@ -662,17 +660,20 @@ available_register_name (struct gdbarch *gdbarch, int regnum) } /* Return the target-supplied register of target-described register - REGNUM, if the feature set for GDBARCH describes that register. - Otherwise return REGNUM (the legacy 1:1 mapping). */ + REGNUM, or -1 if the register can not be accessed. */ int available_register_target_regnum (struct gdbarch *gdbarch, int regnum) { struct gdb_available_register *reg; + /* If there is no feature set, use the legacy 1:1 mapping. */ + if (gdbarch_feature_set (gdbarch) == NULL) + return regnum; + reg = find_register (gdbarch_feature_set (gdbarch), regnum); if (reg == NULL) - return regnum; + return -1; return reg->protocol_number; } diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 80aebec3af0..c3959db7c65 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -153,7 +153,6 @@ struct gdbarch gdbarch_pseudo_register_write_ftype *pseudo_register_write; int num_regs; int num_pseudo_regs; - int remote_num_g_packet_regs; int sp_regnum; int pc_regnum; int ps_regnum; @@ -282,7 +281,6 @@ struct gdbarch startup_gdbarch = 0, /* pseudo_register_write */ 0, /* num_regs */ 0, /* num_pseudo_regs */ - 0, /* remote_num_g_packet_regs */ -1, /* sp_regnum */ -1, /* pc_regnum */ -1, /* ps_regnum */ @@ -543,7 +541,6 @@ verify_gdbarch (struct gdbarch *current_gdbarch) if (current_gdbarch->num_regs == -1) fprintf_unfiltered (log, "\n\tnum_regs"); /* Skip verify of num_pseudo_regs, invalid_p == 0 */ - /* Skip verify of remote_num_g_packet_regs, has predicate */ /* Skip verify of sp_regnum, invalid_p == 0 */ /* Skip verify of pc_regnum, invalid_p == 0 */ /* Skip verify of ps_regnum, invalid_p == 0 */ @@ -1488,12 +1485,6 @@ gdbarch_dump (struct gdbarch *current_gdbarch, struct ui_file *file) fprintf_unfiltered (file, "gdbarch_dump: regset_from_core_section = <0x%lx>\n", (long) current_gdbarch->regset_from_core_section); - fprintf_unfiltered (file, - "gdbarch_dump: gdbarch_remote_num_g_packet_regs_p() = %d\n", - gdbarch_remote_num_g_packet_regs_p (current_gdbarch)); - fprintf_unfiltered (file, - "gdbarch_dump: remote_num_g_packet_regs = %s\n", - paddr_d (current_gdbarch->remote_num_g_packet_regs)); fprintf_unfiltered (file, "gdbarch_dump: remote_translate_xfer_address = <0x%lx>\n", (long) current_gdbarch->remote_translate_xfer_address); @@ -2098,29 +2089,6 @@ set_gdbarch_num_pseudo_regs (struct gdbarch *gdbarch, gdbarch->num_pseudo_regs = num_pseudo_regs; } -int -gdbarch_remote_num_g_packet_regs_p (struct gdbarch *gdbarch) -{ - gdb_assert (gdbarch != NULL); - return gdbarch->remote_num_g_packet_regs != 0; -} - -int -gdbarch_remote_num_g_packet_regs (struct gdbarch *gdbarch) -{ - gdb_assert (gdbarch != NULL); - if (gdbarch_debug >= 2) - fprintf_unfiltered (gdb_stdlog, "gdbarch_remote_num_g_packet_regs called\n"); - return gdbarch->remote_num_g_packet_regs; -} - -void -set_gdbarch_remote_num_g_packet_regs (struct gdbarch *gdbarch, - int remote_num_g_packet_regs) -{ - gdbarch->remote_num_g_packet_regs = remote_num_g_packet_regs; -} - int gdbarch_sp_regnum (struct gdbarch *gdbarch) { diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index 6a4d6885ab1..dc1b1a99adf 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -356,16 +356,6 @@ extern void set_gdbarch_num_pseudo_regs (struct gdbarch *gdbarch, int num_pseudo #define NUM_PSEUDO_REGS (gdbarch_num_pseudo_regs (current_gdbarch)) #endif -/* The number of registers fetched or stored using this target's - traditional g/G packet. - FIXME: Could we do without this by asking the target for a - g packet, and just seeing what's there? We surely could! */ - -extern int gdbarch_remote_num_g_packet_regs_p (struct gdbarch *gdbarch); - -extern int gdbarch_remote_num_g_packet_regs (struct gdbarch *gdbarch); -extern void set_gdbarch_remote_num_g_packet_regs (struct gdbarch *gdbarch, int remote_num_g_packet_regs); - /* GDB's standard (or well known) register numbers. These can map onto a real register or a pseudo (computed) register or not be defined at all (-1). diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index 6b022d49d84..39914dd8896 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -433,11 +433,6 @@ v:=:int:num_regs:::0:-1 # These pseudo-registers may be aliases for other registers, # combinations of other registers, or they may be computed by GDB. v:=:int:num_pseudo_regs:::0:0::0 -# The number of registers fetched or stored using this target's -# traditional g/G packet. -# FIXME: Could we do without this by asking the target for a -# g packet, and just seeing what's there? We surely could! -V::int:remote_num_g_packet_regs # GDB's standard (or well known) register numbers. These can map onto # a real register or a pseudo (computed) register or not be defined at diff --git a/gdb/remote.c b/gdb/remote.c index 1a01cc0b748..0862f851fa2 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -241,19 +241,26 @@ get_remote_state (void) return gdbarch_data (current_gdbarch, remote_gdbarch_data_handle); } -static void * -init_remote_state (struct gdbarch *gdbarch) +static int +compare_pnums (const void *lhs_, const void *rhs_) { - int regnum; - struct remote_state *rs = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct remote_state); - int num_g_regs; + const struct packet_reg * const *lhs = lhs_; + const struct packet_reg * const *rhs = rhs_; - if (gdbarch_remote_num_g_packet_regs_p (gdbarch)) - num_g_regs = gdbarch_remote_num_g_packet_regs (gdbarch); + if ((*lhs)->pnum < (*rhs)->pnum) + return -1; + else if ((*lhs)->pnum == (*rhs)->pnum) + return 0; else - num_g_regs = NUM_REGS; + return 1; +} - rs->sizeof_g_packet = 0; +static void * +init_remote_state (struct gdbarch *gdbarch) +{ + int regnum, num_remote_regs, offset; + struct remote_state *rs = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct remote_state); + struct packet_reg **remote_regs; /* Assume a 1:1 regnum<->pnum table unless the available registers interface informs us otherwise. */ @@ -263,15 +270,31 @@ init_remote_state (struct gdbarch *gdbarch) struct packet_reg *r = &rs->regs[regnum]; r->pnum = available_register_target_regnum (gdbarch, regnum); r->regnum = regnum; - r->offset = DEPRECATED_REGISTER_BYTE (regnum); - r->in_g_packet = (regnum < num_g_regs); - /* ...name = REGISTER_NAME (regnum); */ + } - /* Compute packet size by accumulating the size of all registers. */ - if (r->in_g_packet) - rs->sizeof_g_packet += register_size (current_gdbarch, regnum); + /* Define the g/G packet format as the contents of each register + with a remote protocol number, in order of ascending protocol + number. */ + + remote_regs = alloca (NUM_REGS * sizeof (struct packet_reg *)); + for (num_remote_regs = 0, regnum = 0; regnum < NUM_REGS; regnum++) + if (rs->regs[regnum].pnum != -1) + remote_regs[num_remote_regs++] = &rs->regs[regnum]; + + qsort (remote_regs, num_remote_regs, sizeof (struct packet_reg *), + compare_pnums); + + for (regnum = 0, offset = 0; regnum < num_remote_regs; regnum++) + { + remote_regs[regnum]->in_g_packet = 1; + remote_regs[regnum]->offset = offset; + offset += register_size (current_gdbarch, remote_regs[regnum]->regnum); } + /* Record the maximum possible size of the g packet - it may turn out + to be smaller. */ + rs->sizeof_g_packet = offset; + /* Default maximum number of characters in a packet body. Many remote stubs have a hardwired buffer size of 400 bytes (c.f. BUFMAX in m68k-stub.c and i386-stub.c). BUFMAX-1 is used @@ -3052,6 +3075,9 @@ fetch_register_using_p (struct packet_reg *reg) if (remote_protocol_packets[PACKET_p].support == PACKET_DISABLE) return 0; + if (reg->pnum == -1) + return 0; + p = buf; *p++ = 'p'; p += hexnumstr (p, reg->pnum); @@ -3089,10 +3115,6 @@ fetch_register_using_p (struct packet_reg *reg) return 1; } -/* Number of bytes of registers this stub implements. */ - -static int register_bytes_found; - /* Fetch the registers included in the target's 'g' packet. */ static void @@ -3100,20 +3122,46 @@ fetch_registers_using_g (void) { struct remote_state *rs = get_remote_state (); char *buf = alloca (rs->remote_packet_size); - int i; + int i, buf_len; char *p; - char *regs = alloca (rs->sizeof_g_packet); + char *regs; set_thread (PIDGET (inferior_ptid), 1); sprintf (buf, "g"); remote_send (buf, rs->remote_packet_size); - /* Save the size of the packet sent to us by the target. Its used + buf_len = strlen (buf); + + /* Sanity check the received packet. */ + if (buf_len > 2 * rs->sizeof_g_packet) + error (_("remote 'g' packet reply is too large: %s"), buf); + if (buf_len % 2 != 0) + error (_("Remote 'g' packet reply is of odd length: %s"), buf); + if (REGISTER_BYTES_OK_P () && !REGISTER_BYTES_OK (i)) + error (_("Remote 'g' packet reply is too short: %s"), buf); + + /* Save the size of the packet sent to us by the target. It is used as a heuristic when determining the max size of packets that the target can safely receive. */ - if ((rs->actual_register_packet_size) == 0) - (rs->actual_register_packet_size) = strlen (buf); + if (rs->actual_register_packet_size == 0) + rs->actual_register_packet_size = buf_len; + + /* If this is smaller than we guessed the 'g' packet would be, + update our records. A 'g' reply that doesn't include a register's + value implies either that the register is not available, or that + the 'p' packet must be used. */ + if (buf_len < 2 * rs->sizeof_g_packet) + { + rs->sizeof_g_packet = buf_len / 2; + + for (i = 0; i < NUM_REGS; i++) + if (rs->regs[i].in_g_packet + && rs->regs[i].offset >= rs->sizeof_g_packet) + rs->regs[i].in_g_packet = 0; + } + + regs = alloca (rs->sizeof_g_packet); /* Unimplemented registers read as all bits zero. */ memset (regs, 0, rs->sizeof_g_packet); @@ -3139,15 +3187,11 @@ fetch_registers_using_g (void) p = buf; for (i = 0; i < rs->sizeof_g_packet; i++) { - if (p[0] == 0) - break; - if (p[1] == 0) - { - warning (_("Remote reply is of odd length: %s"), buf); - /* Don't change register_bytes_found in this case, and don't - print a second warning. */ - goto supply_them; - } + if (p[0] == 0 || p[1] == 0) + /* This shouldn't happen - we adjusted sizeof_g_packet above. */ + internal_error (__FILE__, __LINE__, + "unexpected end of 'g' packet reply"); + if (p[0] == 'x' && p[1] == 'x') regs[i] = 0; /* 'x' */ else @@ -3155,15 +3199,6 @@ fetch_registers_using_g (void) p += 2; } - if (i != register_bytes_found) - { - register_bytes_found = i; - if (REGISTER_BYTES_OK_P () - && !REGISTER_BYTES_OK (i)) - warning (_("Remote reply is too short: %s"), buf); - } - - supply_them: { int i; for (i = 0; i < NUM_REGS; i++) @@ -3172,11 +3207,9 @@ fetch_registers_using_g (void) if (r->in_g_packet) { if (r->offset * 2 >= strlen (buf)) - /* A short packet that didn't include the register's - value, this implies that the register is zero (and - not that the register is unavailable). Supply that - zero value. */ - regcache_raw_supply (current_regcache, r->regnum, NULL); + /* This shouldn't happen - we adjusted in_g_packet above. */ + internal_error (__FILE__, __LINE__, + "unexpected end of 'g' packet reply"); else if (buf[r->offset * 2] == 'x') { gdb_assert (r->offset * 2 < strlen (buf)); @@ -3206,14 +3239,24 @@ remote_fetch_registers (int regnum) struct packet_reg *reg = packet_reg_from_regnum (rs, regnum); gdb_assert (reg != NULL); + /* If this register might be in the 'g' packet, try that first - + we are likely to read more than one register. If this is the + first 'g' packet, we might be overly optimistic about its + contents, so fall back to 'p'. */ + if (reg->in_g_packet) + { + fetch_registers_using_g (); + if (reg->in_g_packet) + return; + } + if (fetch_register_using_p (reg)) return; - if (!reg->in_g_packet) - error (_("Protocol error: register \"%s\" not supported by stub"), - gdbarch_register_name (current_gdbarch, reg->regnum)); + /* This register is not available. */ + regcache_raw_supply (current_regcache, reg->regnum, NULL); + set_register_cached (reg->regnum, -1); - fetch_registers_using_g (); return; } @@ -3222,8 +3265,11 @@ remote_fetch_registers (int regnum) for (i = 0; i < NUM_REGS; i++) if (!rs->regs[i].in_g_packet) if (!fetch_register_using_p (&rs->regs[i])) - error (_("Protocol error: register \"%s\" not supported by stub"), - gdbarch_register_name (current_gdbarch, rs->regs[i].regnum)); + { + /* This register is not available. */ + regcache_raw_supply (current_regcache, i, NULL); + set_register_cached (i, -1); + } } /* Prepare to store registers. Since we may send them all (using a @@ -3267,6 +3313,9 @@ store_register_using_P (struct packet_reg *reg) if (remote_protocol_packets[PACKET_P].support == PACKET_DISABLE) return 0; + if (reg->pnum == -1) + return 0; + xsnprintf (buf, rs->remote_packet_size, "P%s=", phex_nz (reg->pnum, 0)); p = buf + strlen (buf); regcache_raw_collect (current_regcache, reg->regnum, regp); @@ -3317,8 +3366,9 @@ store_registers_using_G () buf = alloca (rs->remote_packet_size); p = buf; *p++ = 'G'; - /* remote_prepare_to_store insures that register_bytes_found gets set. */ - bin2hex (regs, p, register_bytes_found); + /* remote_prepare_to_store insures that rs->sizeof_g_packet gets + updated. */ + bin2hex (regs, p, rs->sizeof_g_packet); remote_send (buf, rs->remote_packet_size); } @@ -3338,12 +3388,19 @@ remote_store_registers (int regnum) struct packet_reg *reg = packet_reg_from_regnum (rs, regnum); gdb_assert (reg != NULL); + /* Always prefer to store registers using the 'P' packet if + possible; we often change only a small number of registers. + Sometimes we change a larger number; we'd need help from a + higher layer to know to use 'G'. */ if (store_register_using_P (reg)) return; + /* For now, don't complain if we have no way to write the + register. GDB loses track of unavailable registers too + easily. Some day, this may be an error. We don't have + any way to read the register, either... */ if (!reg->in_g_packet) - error (_("Protocol error: register \"%s\" not supported by stub"), - gdbarch_register_name (current_gdbarch, reg->regnum)); + return; store_registers_using_G (); return; @@ -3354,8 +3411,8 @@ remote_store_registers (int regnum) for (i = 0; i < NUM_REGS; i++) if (!rs->regs[i].in_g_packet) if (!store_register_using_P (&rs->regs[i])) - error (_("Protocol error: register \"%s\" not supported by stub"), - gdbarch_register_name (current_gdbarch, rs->regs[i].regnum)); + /* See above for why we do not issue an error here. */ + continue; } -- cgit v1.2.1