From patchwork Thu Aug 27 12:07:54 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 8475 Received: (qmail 90641 invoked by alias); 27 Aug 2015 12:08:00 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 90574 invoked by uid 89); 27 Aug 2015 12:07:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 27 Aug 2015 12:07:57 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 39EA18E239 for ; Thu, 27 Aug 2015 12:07:56 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7RC7sVo001736; Thu, 27 Aug 2015 08:07:55 -0400 Message-ID: <55DEFD9A.4020904@redhat.com> Date: Thu, 27 Aug 2015 13:07:54 +0100 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Gary Benson CC: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: remove packet size limit References: <1440541673-31794-1-git-send-email-palves@redhat.com> <20150826084356.GA26028@blade.nx> In-Reply-To: <20150826084356.GA26028@blade.nx> On 08/26/2015 09:43 AM, Gary Benson wrote: > Pedro Alves wrote: >> The remote packet buffer size is currently capped to 16384 mostly for >> historical reasons, related to use of alloca. Stop using alloca and >> remove the limitation. > > Nice. > >> diff --git a/gdb/remote.c b/gdb/remote.c >> index e019277..e00e67a 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -920,6 +920,16 @@ struct memory_packet_config >> int fixed_p; >> }; >> >> +/* The default max memory-write-packet-size. The 16k is historical. >> + (It came from older GDB's using alloca for buffers and the >> + knowledge (folk law?) that some hosts don't cope very well with > > It's "folklore". Thanks, pushed with that change. From a5c0808e221c0e8600561c2c9664c920d808a887 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 27 Aug 2015 13:03:15 +0100 Subject: [PATCH] gdb: remove packet size limit The remote packet buffer size is currently capped to 16384 mostly for historical reasons, related to use of alloca. Stop using alloca and remove the limitation. Tested on x86_64 Fedora 20. gdb/ChangeLog: 2015-08-27 Pedro Alves * remote.c (DEFAULT_MAX_MEMORY_PACKET_SIZE) (MIN_MEMORY_PACKET_SIZE): New. (MAX_REMOTE_PACKET_SIZE, MIN_REMOTE_PACKET_SIZE): Delete. (get_memory_packet_size): Adjust. No longer limit the max packet size. (set_memory_packet_size): Adjust, and remove dead code. (remote_check_symbols): Use xmalloc and a cleanup instead of alloca. (remote_packet_size): No longer cap the packet size. (putpkt_binary): Use xmalloc and a cleanup instead of alloca. --- gdb/ChangeLog | 13 +++++++++++ gdb/remote.c | 71 ++++++++++++++++++++++++++++++----------------------------- 2 files changed, 49 insertions(+), 35 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0323b06..22005ac 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2015-08-27 Pedro Alves + + * remote.c (DEFAULT_MAX_MEMORY_PACKET_SIZE) + (MIN_MEMORY_PACKET_SIZE): New. + (MAX_REMOTE_PACKET_SIZE, MIN_REMOTE_PACKET_SIZE): Delete. + (get_memory_packet_size): Adjust. No longer limit the max packet + size. + (set_memory_packet_size): Adjust, and remove dead code. + (remote_check_symbols): Use xmalloc and a cleanup instead of + alloca. + (remote_packet_size): No longer cap the packet size. + (putpkt_binary): Use xmalloc and a cleanup instead of alloca. + 2015-08-26 Luis Machado * compile/compile.c (compile_to_object): Mention language in diff --git a/gdb/remote.c b/gdb/remote.c index e11cb5f..3afcaf4 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -921,6 +921,16 @@ struct memory_packet_config int fixed_p; }; +/* The default max memory-write-packet-size. The 16k is historical. + (It came from older GDB's using alloca for buffers and the + knowledge (folklore?) that some hosts don't cope very well with + large alloca calls.) */ +#define DEFAULT_MAX_MEMORY_PACKET_SIZE 16384 + +/* The minimum remote packet size for memory transfers. Ensures we + can write at least one byte. */ +#define MIN_MEMORY_PACKET_SIZE 20 + /* Compute the current size of a read/write packet. Since this makes use of ``actual_register_packet_size'' the computation is dynamic. */ @@ -930,23 +940,11 @@ 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 (); - /* NOTE: The somewhat arbitrary 16k comes from the knowledge (folk - law?) that some hosts don't cope very well with large alloca() - calls. Eventually the alloca() code will be replaced by calls to - xmalloc() and make_cleanups() allowing this restriction to either - be lifted or removed. */ -#ifndef MAX_REMOTE_PACKET_SIZE -#define MAX_REMOTE_PACKET_SIZE 16384 -#endif - /* NOTE: 20 ensures we can write at least one byte. */ -#ifndef MIN_REMOTE_PACKET_SIZE -#define MIN_REMOTE_PACKET_SIZE 20 -#endif long what_they_get; if (config->fixed_p) { if (config->size <= 0) - what_they_get = MAX_REMOTE_PACKET_SIZE; + what_they_get = DEFAULT_MAX_MEMORY_PACKET_SIZE; else what_they_get = config->size; } @@ -965,10 +963,8 @@ get_memory_packet_size (struct memory_packet_config *config) && what_they_get > rsa->actual_register_packet_size) what_they_get = rsa->actual_register_packet_size; } - if (what_they_get > MAX_REMOTE_PACKET_SIZE) - what_they_get = MAX_REMOTE_PACKET_SIZE; - if (what_they_get < MIN_REMOTE_PACKET_SIZE) - what_they_get = MIN_REMOTE_PACKET_SIZE; + if (what_they_get < MIN_MEMORY_PACKET_SIZE) + what_they_get = MIN_MEMORY_PACKET_SIZE; /* Make sure there is room in the global buffer for this packet (including its trailing NUL byte). */ @@ -1005,15 +1001,16 @@ set_memory_packet_size (char *args, struct memory_packet_config *config) size = strtoul (args, &end, 0); if (args == end) error (_("Invalid %s (bad syntax)."), config->name); -#if 0 - /* Instead of explicitly capping the size of a packet to - MAX_REMOTE_PACKET_SIZE or dissallowing it, the user is - instead allowed to set the size to something arbitrarily - large. */ - if (size > MAX_REMOTE_PACKET_SIZE) - error (_("Invalid %s (too large)."), config->name); -#endif + + /* Instead of explicitly capping the size of a packet to or + disallowing it, the user is allowed to set the size to + something arbitrarily large. */ } + + /* So that the query shows the correct value. */ + if (size <= 0) + size = DEFAULT_MAX_MEMORY_PACKET_SIZE; + /* Extra checks? */ if (fixed_p && !config->fixed_p) { @@ -4008,6 +4005,7 @@ remote_check_symbols (void) char *msg, *reply, *tmp; struct bound_minimal_symbol sym; int end; + struct cleanup *old_chain; /* The remote side has no concept of inferiors that aren't running yet, it only knows about running processes. If we're connected @@ -4026,7 +4024,8 @@ remote_check_symbols (void) /* Allocate a message buffer. We can't reuse the input buffer in RS, because we need both at the same time. */ - msg = alloca (get_remote_packet_size ()); + msg = xmalloc (get_remote_packet_size ()); + old_chain = make_cleanup (xfree, msg); /* Invite target to request symbol lookups. */ @@ -4064,6 +4063,8 @@ remote_check_symbols (void) getpkt (&rs->buf, &rs->buf_size, 0); reply = rs->buf; } + + do_cleanups (old_chain); } static struct serial * @@ -4186,13 +4187,6 @@ remote_packet_size (const struct protocol_feature *feature, return; } - if (packet_size > MAX_REMOTE_PACKET_SIZE) - { - warning (_("limiting remote suggested packet size (%d bytes) to %d"), - packet_size, MAX_REMOTE_PACKET_SIZE); - packet_size = MAX_REMOTE_PACKET_SIZE; - } - /* Record the new maximum packet size. */ rs->explicit_packet_size = packet_size; } @@ -7784,7 +7778,8 @@ putpkt_binary (const char *buf, int cnt) struct remote_state *rs = get_remote_state (); int i; unsigned char csum = 0; - char *buf2 = alloca (cnt + 6); + char *buf2 = xmalloc (cnt + 6); + struct cleanup *old_chain = make_cleanup (xfree, buf2); int ch; int tcount = 0; @@ -7877,6 +7872,7 @@ putpkt_binary (const char *buf, int cnt) case '+': if (remote_debug) fprintf_unfiltered (gdb_stdlog, "Ack\n"); + do_cleanups (old_chain); return 1; case '-': if (remote_debug) @@ -7885,7 +7881,10 @@ putpkt_binary (const char *buf, int cnt) case SERIAL_TIMEOUT: tcount++; if (tcount > 3) - return 0; + { + do_cleanups (old_chain); + return 0; + } break; /* Retransmit buffer. */ case '$': { @@ -7972,6 +7971,8 @@ putpkt_binary (const char *buf, int cnt) } #endif } + + do_cleanups (old_chain); return 0; }