From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91385 invoked by alias); 29 Mar 2016 21:49:31 -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 91272 invoked by uid 89); 29 Mar 2016 21:49:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,KAM_ADVERT2,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=2016-03-09, 20160309, adequate, *to X-HELO: xyzzy.0x04.net Received: from xyzzy.0x04.net (HELO xyzzy.0x04.net) (159.100.250.38) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Mar 2016 21:49:20 +0000 Received: from hogfather.0x04.net (89-65-66-135.dynamic.chello.pl [89.65.66.135]) by xyzzy.0x04.net (Postfix) with ESMTPS id 1C6151A3BB for ; Tue, 29 Mar 2016 21:49:18 +0000 (UTC) Received: from [172.21.36.130] (unknown [88.214.186.105]) by hogfather.0x04.net (Postfix) with ESMTPSA id EC9935800F8 for ; Tue, 29 Mar 2016 23:49:17 +0200 (CEST) Subject: Re: [PATCH] gdbserver: Handle 'v' packet while processing qSymbol. To: gdb-patches@sourceware.org References: <1457794943-12954-1-git-send-email-koriakin@0x04.net> From: =?UTF-8?Q?Marcin_Ko=c5=9bcielnicki?= Message-ID: <56FAF85D.3080008@0x04.net> Date: Tue, 29 Mar 2016 21:49:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <1457794943-12954-1-git-send-email-koriakin@0x04.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg00539.txt.bz2 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 > + > + * remote.c (xfreep): New function. > + (remote_check_symbols): Allocate own buffer for reply. > + > 2016-03-11 Marcin Kościelnicki > > * 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 > + > + * 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 > > * 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); >