[PATCH] gdbserver: Handle 'v' packet while processing qSymbol.
Marcin Kościelnicki
koriakin@0x04.net
Tue Mar 29 21:49:00 GMT 2016
Ping.
On 12/03/16 16:02, Marcin Kościelnicki wrote:
> On powerpc64, qSymbol query may require gdb to read a function
> descriptor, sending a vFile packet to gdbserver. Thus, we need
> to handle 'v' packet in look_up_one_symbol.
>
> vFile replies may be quite long, and require reallocating own_buf.
> Since handle_v_requests assumes the buffer is the static global own_buf
> from server.c and reallocates it, we need to make own_buf global and
> use it from look_up_one_symbol instead of using our own auto variable.
> I've also done the same change in relocate_instruction, just in case.
>
> On gdb side, in remote_check_symbols, rs->buf may be clobbered by vFile
> handling, yet we need its contents for the reply (the symbol name is
> stored there). Allocate a new buffer instead.
>
> This broke fast tracepoints on powerpc64, due to errors in reading IPA
> symbols.
>
> gdb/ChangeLog:
>
> * remote.c (xfreep): New function.
> (remote_check_symbols): Allocate own buffer for reply.
>
> gdbserver/ChangeLog:
>
> * remote-utils.c (look_up_one_symbol): Remove own_buf, handle 'v'
> packets.
> (relocate_instruction): Remove own_buf.
> * server.c (own_buf): Make global.
> (handle_v_requests): Make global.
> * server.h (own_buf): New declaration.
> (handle_v_requests): New prototype.
> ---
>
> gdb/ChangeLog | 5 +++++
> gdb/gdbserver/ChangeLog | 10 ++++++++++
> gdb/gdbserver/remote-utils.c | 41 +++++++++++++++++++++++++++--------------
> gdb/gdbserver/server.c | 4 ++--
> gdb/gdbserver/server.h | 4 ++++
> gdb/remote.c | 22 +++++++++++++++++-----
> 6 files changed, 65 insertions(+), 21 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index febe960..41f826e 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-03-12 Marcin Kościelnicki <koriakin@0x04.net>
> +
> + * remote.c (xfreep): New function.
> + (remote_check_symbols): Allocate own buffer for reply.
> +
> 2016-03-11 Marcin Kościelnicki <koriakin@0x04.net>
>
> * s390-linux-tdep.c (s390_ax_pseudo_register_collect): New function.
> diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
> index dda87b0..7ce2690 100644
> --- a/gdb/gdbserver/ChangeLog
> +++ b/gdb/gdbserver/ChangeLog
> @@ -1,3 +1,13 @@
> +2016-03-12 Marcin Kościelnicki <koriakin@0x04.net>
> +
> + * remote-utils.c (look_up_one_symbol): Remove own_buf, handle 'v'
> + packets.
> + (relocate_instruction): Remove own_buf.
> + * server.c (own_buf): Make global.
> + (handle_v_requests): Make global.
> + * server.h (own_buf): New declaration.
> + (handle_v_requests): New prototype.
> +
> 2016-03-09 Marcin Kościelnicki <koriakin@0x04.net>
>
> * linux-ppc-low.c (ppc_supports_tracepoints): New function.
> diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
> index e751473..24f92c2 100644
> --- a/gdb/gdbserver/remote-utils.c
> +++ b/gdb/gdbserver/remote-utils.c
> @@ -1462,7 +1462,7 @@ clear_symbol_cache (struct sym_cache **symcache_p)
> int
> look_up_one_symbol (const char *name, CORE_ADDR *addrp, int may_ask_gdb)
> {
> - char own_buf[266], *p, *q;
> + char *p, *q;
> int len;
> struct sym_cache *sym;
> struct process_info *proc;
> @@ -1499,21 +1499,35 @@ look_up_one_symbol (const char *name, CORE_ADDR *addrp, int may_ask_gdb)
> main loop. For now, this is an adequate approximation; allow
> GDB to read from memory while it figures out the address of the
> symbol. */
> - while (own_buf[0] == 'm')
> + while (1)
> {
> - CORE_ADDR mem_addr;
> - unsigned char *mem_buf;
> - unsigned int mem_len;
> + if (own_buf[0] == 'm')
> + {
> + CORE_ADDR mem_addr;
> + unsigned char *mem_buf;
> + unsigned int mem_len;
>
> - decode_m_packet (&own_buf[1], &mem_addr, &mem_len);
> - mem_buf = (unsigned char *) xmalloc (mem_len);
> - if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0)
> - bin2hex (mem_buf, own_buf, mem_len);
> + decode_m_packet (&own_buf[1], &mem_addr, &mem_len);
> + mem_buf = (unsigned char *) xmalloc (mem_len);
> + if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0)
> + bin2hex (mem_buf, own_buf, mem_len);
> + else
> + write_enn (own_buf);
> + free (mem_buf);
> + if (putpkt (own_buf) < 0)
> + return -1;
> + }
> + else if (own_buf[0] == 'v')
> + {
> + int new_len = -1;
> + handle_v_requests (own_buf, len, &new_len);
> + if (new_len != -1)
> + putpkt_binary (own_buf, new_len);
> + else
> + putpkt (own_buf);
> + }
> else
> - write_enn (own_buf);
> - free (mem_buf);
> - if (putpkt (own_buf) < 0)
> - return -1;
> + break;
> len = getpkt (own_buf);
> if (len < 0)
> return -1;
> @@ -1561,7 +1575,6 @@ look_up_one_symbol (const char *name, CORE_ADDR *addrp, int may_ask_gdb)
> int
> relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc)
> {
> - char own_buf[266];
> int len;
> ULONGEST written = 0;
>
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index ef715e7..9c50929 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -119,7 +119,7 @@ int disable_packet_qfThreadInfo;
> static struct target_waitstatus last_status;
> static ptid_t last_ptid;
>
> -static char *own_buf;
> +char *own_buf;
> static unsigned char *mem_buf;
>
> /* A sub-class of 'struct notif_event' for stop, holding information
> @@ -2935,7 +2935,7 @@ handle_v_kill (char *own_buf)
> }
>
> /* Handle all of the extended 'v' packets. */
> -static void
> +void
> handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
> {
> if (!disable_packet_vCont)
> diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
> index 3d78fb3..51b2191 100644
> --- a/gdb/gdbserver/server.h
> +++ b/gdb/gdbserver/server.h
> @@ -82,6 +82,8 @@ extern int disable_packet_Tthread;
> extern int disable_packet_qC;
> extern int disable_packet_qfThreadInfo;
>
> +extern char *own_buf;
> +
> extern int run_once;
> extern int multi_process;
> extern int report_fork_events;
> @@ -113,6 +115,8 @@ typedef int gdb_fildes_t;
> #include "event-loop.h"
>
> /* Functions from server.c. */
> +extern void handle_v_requests (char *own_buf, int packet_len,
> + int *new_packet_len);
> extern int handle_serial_event (int err, gdb_client_data client_data);
> extern int handle_target_event (int err, gdb_client_data client_data);
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index f09a06e..5f3d9fd 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -4321,6 +4321,16 @@ init_all_packet_configs (void)
> }
> }
>
> +/* Helper function for cleanup below. Argument is a pointer to a void *
> + pointing to a buffer that needs to be freed. */
> +
> +static void
> +xfreep (void *arg)
> +{
> + void **pptr = (void **) arg;
> + xfree (*pptr);
> +}
> +
> /* Symbol look-up. */
>
> static void
> @@ -4329,6 +4339,7 @@ remote_check_symbols (void)
> struct remote_state *rs = get_remote_state ();
> char *msg, *reply, *tmp;
> int end;
> + long reply_size;
> struct cleanup *old_chain;
>
> /* The remote side has no concept of inferiors that aren't running
> @@ -4350,13 +4361,15 @@ remote_check_symbols (void)
> because we need both at the same time. */
> msg = (char *) xmalloc (get_remote_packet_size ());
> old_chain = make_cleanup (xfree, msg);
> + reply = (char *) xmalloc (get_remote_packet_size ());
> + make_cleanup (xfreep, &reply);
> + reply_size = get_remote_packet_size ();
>
> /* Invite target to request symbol lookups. */
>
> putpkt ("qSymbol::");
> - getpkt (&rs->buf, &rs->buf_size, 0);
> - packet_ok (rs->buf, &remote_protocol_packets[PACKET_qSymbol]);
> - reply = rs->buf;
> + getpkt (&reply, &reply_size, 0);
> + packet_ok (reply, &remote_protocol_packets[PACKET_qSymbol]);
>
> while (startswith (reply, "qSymbol:"))
> {
> @@ -4384,8 +4397,7 @@ remote_check_symbols (void)
> }
>
> putpkt (msg);
> - getpkt (&rs->buf, &rs->buf_size, 0);
> - reply = rs->buf;
> + getpkt (&reply, &reply_size, 0);
> }
>
> do_cleanups (old_chain);
>
More information about the Gdb-patches
mailing list