From patchwork Wed Apr 8 19:56:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 6103 Received: (qmail 56618 invoked by alias); 8 Apr 2015 19:56:40 -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 56547 invoked by uid 89); 8 Apr 2015 19:56:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg20.ericsson.net Received: from usevmg20.ericsson.net (HELO usevmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 08 Apr 2015 19:56:35 +0000 Received: from EUSAAHC008.ericsson.se (Unknown_Domain [147.117.188.96]) by usevmg20.ericsson.net (Symantec Mail Security) with SMTP id 83.C8.12456.76135255; Wed, 8 Apr 2015 15:47:19 +0200 (CEST) Received: from elxcz23q12-y4.mo.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.96) with Microsoft SMTP Server (TLS) id 14.3.210.2; Wed, 8 Apr 2015 15:56:24 -0400 From: Simon Marchi To: CC: Simon Marchi Subject: [PATCH 6/7] remote: Consider byte size when reading/writing memory Date: Wed, 8 Apr 2015 15:56:18 -0400 Message-ID: <1428522979-28709-7-git-send-email-simon.marchi@ericsson.com> In-Reply-To: <1428522979-28709-1-git-send-email-simon.marchi@ericsson.com> References: <1428522979-28709-1-git-send-email-simon.marchi@ericsson.com> MIME-Version: 1.0 X-IsSubscribed: yes Adapt code in remote.c to take into account memory byte size when reading/writing memory. A few variables are renamed and suffixed with _bytes or _octets. This way, it's more obvious if there is any place where we add or compare values in different units (which would be a mistake). gdb/ChangeLog: * common/rsp-low.c (needs_escaping): New. (remote_escape_output): Add byte_size parameter and refactor to use it. Rename parameters. * common/rsp-low.h (remote_escape_output): Add byte_size parameter and rename others. Update doc. * remote.c (align_for_efficient_write): New. (remote_write_bytes_aux): Add byte_size parameter and use it. Minor refactoring and variable renaming. (remote_xfer_partial): Get byte size and use it. (remote_read_bytes_1): Add byte_size parameter and use it. (remote_write_bytes): Same. (remote_xfer_live_readonly_partial): Same. (remote_read_bytes): Same. (remote_flash_write): Update call to remote_write_bytes_aux. (remote_write_qxfer): Update call to remote_escape_output. (remote_search_memory): Same. (remote_hostio_pwrite): Same. gdb/gdbserver/ChangeLog: * server.c (write_qxfer_response): Update call to remote_escape_output. --- gdb/common/rsp-low.c | 71 ++++++++++++++++------- gdb/common/rsp-low.h | 16 +++--- gdb/gdbserver/server.c | 4 +- gdb/remote.c | 153 +++++++++++++++++++++++++++---------------------- 4 files changed, 145 insertions(+), 99 deletions(-) diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c index d3d3d65..6e21208 100644 --- a/gdb/common/rsp-low.c +++ b/gdb/common/rsp-low.c @@ -146,38 +146,67 @@ bin2hex (const gdb_byte *bin, char *hex, int count) return i; } +static int needs_escaping (gdb_byte b) +{ + return b == '$' || b == '#' || b == '}' || b == '*'; +} + /* See rsp-low.h. */ int -remote_escape_output (const gdb_byte *buffer, int len, - gdb_byte *out_buf, int *out_len, - int out_maxlen) +remote_escape_output (const gdb_byte *buffer, int len_bytes, int byte_size, + gdb_byte *out_buf, int *out_len_bytes, + int out_maxlen_octets) { - int input_index, output_index; - - output_index = 0; - for (input_index = 0; input_index < len; input_index++) + int input_byte_index, output_octet_index, octet_index_in_byte; + + /* The number of octets we'll need to escape in that byte. */ + int number_escape_octets_needed; + + /* Try to copy integral target memory bytes until + (1) we run out of space or + (2) we copied all of them. */ + output_octet_index = 0; + for (input_byte_index = 0; + input_byte_index < len_bytes; + input_byte_index++) { - gdb_byte b = buffer[input_index]; - - if (b == '$' || b == '#' || b == '}' || b == '*') + /* Find out how many escape octets we need for this byte. */ + number_escape_octets_needed = 0; + for (octet_index_in_byte = 0; + octet_index_in_byte < byte_size; + octet_index_in_byte++) { - /* These must be escaped. */ - if (output_index + 2 > out_maxlen) - break; - out_buf[output_index++] = '}'; - out_buf[output_index++] = b ^ 0x20; + int idx = input_byte_index * byte_size + octet_index_in_byte; + gdb_byte b = buffer[idx]; + if (needs_escaping (b)) + number_escape_octets_needed++; } - else + + /* Check if we have room to fit this escaped byte. */ + if (output_octet_index + byte_size + number_escape_octets_needed > + out_maxlen_octets) + break; + + /* Copy the byte octet per octet, adding escapes. */ + for (octet_index_in_byte = 0; + octet_index_in_byte < byte_size; + octet_index_in_byte++) { - if (output_index + 1 > out_maxlen) - break; - out_buf[output_index++] = b; + int idx = input_byte_index * byte_size + octet_index_in_byte; + gdb_byte b = buffer[idx]; + if (needs_escaping(b)) + { + out_buf[output_octet_index++] = '}'; + out_buf[output_octet_index++] = b ^ 0x20; + } + else + out_buf[output_octet_index++] = b; } } - *out_len = input_index; - return output_index; + *out_len_bytes = input_byte_index; + return output_octet_index; } /* See rsp-low.h. */ diff --git a/gdb/common/rsp-low.h b/gdb/common/rsp-low.h index d62f67e..c20e88c 100644 --- a/gdb/common/rsp-low.h +++ b/gdb/common/rsp-low.h @@ -59,17 +59,17 @@ extern int hex2bin (const char *hex, gdb_byte *bin, int count); extern int bin2hex (const gdb_byte *bin, char *hex, int count); -/* Convert BUFFER, binary data at least LEN bytes long, into escaped - binary data in OUT_BUF. Set *OUT_LEN to the length of the data - encoded in OUT_BUF, and return the number of bytes in OUT_BUF - (which may be more than *OUT_LEN due to escape characters). The - total number of bytes in the output buffer will be at most +/* Convert BUFFER, binary data at least LEN_BYTES bytes long, into escaped + binary data in OUT_BUF. Only copy target memory bytes that fit completely + in OUT_BUF. Set *OUT_LEN_BYTES to the number of bytes from BUFFER + successfully encoded in OUT_BUF, and return the number of octets used in + OUT_BUF. The total number of octets in the output buffer will be at most OUT_MAXLEN. This function properly escapes '*', and so is suitable for the server side as well as the client. */ -extern int remote_escape_output (const gdb_byte *buffer, int len, - gdb_byte *out_buf, int *out_len, - int out_maxlen); +extern int remote_escape_output (const gdb_byte *buffer, int len_bytes, + int byte_size, gdb_byte *out_buf, + int *out_len_bytes, int out_maxlen_octets); /* Convert BUFFER, escaped data LEN bytes long, into binary data in OUT_BUF. Return the number of bytes written to OUT_BUF. diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 3408ef7..367379e 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -383,8 +383,8 @@ write_qxfer_response (char *buf, const void *data, int len, int is_more) else buf[0] = 'l'; - return remote_escape_output (data, len, (unsigned char *) buf + 1, &out_len, - PBUFSIZ - 2) + 1; + return remote_escape_output (data, len, 1, (unsigned char *) buf + 1, + &out_len, PBUFSIZ - 2) + 1; } /* Handle btrace enabling in BTS format. */ diff --git a/gdb/remote.c b/gdb/remote.c index dcd24c4..d9b4350 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -6639,6 +6639,16 @@ check_binary_download (CORE_ADDR addr) } } +/* Helper function to resize the payload, in order to try to get a good + alignment. We try to write an amount of data such that the next write will + start on an address aligned on REMOTE_ALIGN_WRITES. */ + +static int +align_for_efficient_write (int todo_bytes, CORE_ADDR memaddr) +{ + return ((memaddr + todo_bytes) & ~(REMOTE_ALIGN_WRITES - 1)) - memaddr; +} + /* Write memory data directly to the remote machine. This does not inform the data cache; the data cache uses this. HEADER is the starting part of the packet. @@ -6651,7 +6661,7 @@ check_binary_download (CORE_ADDR addr) The function creates packet of the form
,: - where encoding of is termined by PACKET_FORMAT. + where encoding of is terminated by PACKET_FORMAT. If USE_LENGTH is 0, then the field and the preceding comma are omitted. @@ -6662,28 +6672,27 @@ check_binary_download (CORE_ADDR addr) static enum target_xfer_status remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, - const gdb_byte *myaddr, ULONGEST len, - ULONGEST *xfered_len, char packet_format, + const gdb_byte *myaddr, ULONGEST len_bytes, + int byte_size, ULONGEST *xfered_len_bytes, char packet_format, int use_length) { struct remote_state *rs = get_remote_state (); char *p; char *plen = NULL; int plenlen = 0; - int todo; - int nr_bytes; - int payload_size; - int payload_length; - int header_length; + int todo_bytes; + int bytes_written; + int payload_capacity_octets; + int payload_length_octets; if (packet_format != 'X' && packet_format != 'M') internal_error (__FILE__, __LINE__, _("remote_write_bytes_aux: bad packet format")); - if (len == 0) + if (len_bytes == 0) return TARGET_XFER_EOF; - payload_size = get_memory_write_packet_size (); + payload_capacity_octets = get_memory_write_packet_size (); /* The packet buffer will be large enough for the payload; get_memory_packet_size ensures this. */ @@ -6692,13 +6701,12 @@ remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, /* Compute the size of the actual payload by subtracting out the packet header and footer overhead: "$M,:...#nn". */ - payload_size -= strlen ("$,:#NN"); + payload_capacity_octets -= strlen ("$,:#NN"); if (!use_length) /* The comma won't be used. */ - payload_size += 1; - header_length = strlen (header); - payload_size -= header_length; - payload_size -= hexnumlen (memaddr); + payload_capacity_octets += 1; + payload_capacity_octets -= strlen (header); + payload_capacity_octets -= hexnumlen (memaddr); /* Construct the packet excluding the data: "
,:". */ @@ -6709,28 +6717,29 @@ remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, if (packet_format == 'X') { /* Best guess at number of bytes that will fit. */ - todo = min (len, payload_size); + todo_bytes = min (len_bytes, payload_capacity_octets / byte_size); if (use_length) - payload_size -= hexnumlen (todo); - todo = min (todo, payload_size); + /* The size in the packet is expressed in octets, not target bytes. */ + payload_capacity_octets -= hexnumlen (todo_bytes * byte_size); + todo_bytes = min (todo_bytes, payload_capacity_octets / byte_size); } else { - /* Num bytes that will fit. */ - todo = min (len, payload_size / 2); + /* Number of bytes that will fit. */ + todo_bytes = min (len_bytes, (payload_capacity_octets / byte_size) / 2); if (use_length) - payload_size -= hexnumlen (todo); - todo = min (todo, payload_size / 2); + payload_capacity_octets -= hexnumlen (todo_bytes * byte_size); + todo_bytes = min (todo_bytes, (payload_capacity_octets / byte_size) / 2); } - if (todo <= 0) + if (todo_bytes <= 0) internal_error (__FILE__, __LINE__, _("minimum packet size too small to write data")); /* If we already need another packet, then try to align the end of this packet to a useful boundary. */ - if (todo > 2 * REMOTE_ALIGN_WRITES && todo < len) - todo = ((memaddr + todo) & ~(REMOTE_ALIGN_WRITES - 1)) - memaddr; + if (todo_bytes > 2 * REMOTE_ALIGN_WRITES && todo_bytes < len_bytes) + todo_bytes = align_for_efficient_write (todo_bytes, memaddr); /* Append "". */ memaddr = remote_address_masked (memaddr); @@ -6741,10 +6750,10 @@ remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, /* Append ",". */ *p++ = ','; - /* Append . Retain the location/size of . It may need to - be adjusted once the packet body has been created. */ + /* Append . Retain the location/size of the length. It may + need to be adjusted once the packet body has been created. */ plen = p; - plenlen = hexnumstr (p, (ULONGEST) todo); + plenlen = hexnumstr (p, (ULONGEST) todo_bytes * byte_size); p += plenlen; } @@ -6758,42 +6767,45 @@ remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, /* Binary mode. Send target system values byte by byte, in increasing byte addresses. Only escape certain critical characters. */ - payload_length = remote_escape_output (myaddr, todo, (gdb_byte *) p, - &nr_bytes, payload_size); + payload_length_octets = + remote_escape_output (myaddr, todo_bytes, byte_size, (gdb_byte *) p, + &bytes_written, payload_capacity_octets); /* If not all TODO bytes fit, then we'll need another packet. Make a second try to keep the end of the packet aligned. Don't do this if the packet is tiny. */ - if (nr_bytes < todo && nr_bytes > 2 * REMOTE_ALIGN_WRITES) + if (bytes_written < todo_bytes && bytes_written > 2 * REMOTE_ALIGN_WRITES) { - int new_nr_bytes; - - new_nr_bytes = (((memaddr + nr_bytes) & ~(REMOTE_ALIGN_WRITES - 1)) - - memaddr); - if (new_nr_bytes != nr_bytes) - payload_length = remote_escape_output (myaddr, new_nr_bytes, - (gdb_byte *) p, &nr_bytes, - payload_size); + int new_todo_bytes; + + new_todo_bytes = align_for_efficient_write (bytes_written, memaddr); + + if (new_todo_bytes != bytes_written) + payload_length_octets = + remote_escape_output (myaddr, new_todo_bytes, byte_size, + (gdb_byte *) p, &bytes_written, + payload_capacity_octets); } - p += payload_length; - if (use_length && nr_bytes < todo) + p += payload_length_octets; + if (use_length && bytes_written < todo_bytes) { /* Escape chars have filled up the buffer prematurely, and we have actually sent fewer bytes than planned. Fix-up the length field of the packet. Use the same number of characters as before. */ - plen += hexnumnstr (plen, (ULONGEST) nr_bytes, plenlen); + plen += hexnumnstr (plen, (ULONGEST) bytes_written * byte_size, + plenlen); *plen = ':'; /* overwrite \0 from hexnumnstr() */ } } else { /* Normal mode: Send target system values byte by byte, in - increasing byte addresses. Each byte is encoded as a two hex + increasing byte addresses. Each octet is encoded as a two hex value. */ - nr_bytes = bin2hex (myaddr, p, todo); - p += 2 * nr_bytes; + p += 2 * bin2hex (myaddr, p, todo_bytes * byte_size); + bytes_written = todo_bytes; } putpkt_binary (rs->buf, (int) (p - rs->buf)); @@ -6804,7 +6816,7 @@ remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, /* Return NR_BYTES, not TODO, in case escape chars caused us to send fewer bytes than we'd planned. */ - *xfered_len = (ULONGEST) nr_bytes; + *xfered_len_bytes = (ULONGEST) bytes_written; return TARGET_XFER_OK; } @@ -6820,7 +6832,7 @@ remote_write_bytes_aux (const char *header, CORE_ADDR memaddr, static enum target_xfer_status remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr, ULONGEST len, - ULONGEST *xfered_len) + int byte_size, ULONGEST *xfered_len) { char *packet_format = 0; @@ -6843,7 +6855,7 @@ remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr, ULONGEST len, } return remote_write_bytes_aux (packet_format, - memaddr, myaddr, len, xfered_len, + memaddr, myaddr, len, byte_size, xfered_len, packet_format[0], 1); } @@ -6859,20 +6871,21 @@ remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr, ULONGEST len, static enum target_xfer_status remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, ULONGEST len, - ULONGEST *xfered_len) + int byte_size, ULONGEST *xfered_len) { struct remote_state *rs = get_remote_state (); int max_buf_size; /* Max size of packet output buffer. */ char *p; - int todo; - int i; + int todo_bytes; + int decoded_octets; + int len_octets = len * byte_size; max_buf_size = get_memory_read_packet_size (); /* The packet buffer will be large enough for the payload; get_memory_packet_size ensures this. */ - /* Number if bytes that will fit. */ - todo = min (len, max_buf_size / 2); + /* Number of bytes that will fit. */ + todo_bytes = min (len, (max_buf_size / byte_size) / 2); /* Construct "m"","". */ memaddr = remote_address_masked (memaddr); @@ -6880,7 +6893,7 @@ remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, ULONGEST len, *p++ = 'm'; p += hexnumstr (p, (ULONGEST) memaddr); *p++ = ','; - p += hexnumstr (p, (ULONGEST) todo); + p += hexnumstr (p, (ULONGEST) todo_bytes * byte_size); *p = '\0'; putpkt (rs->buf); getpkt (&rs->buf, &rs->buf_size, 0); @@ -6891,9 +6904,9 @@ remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, ULONGEST len, /* Reply describes memory byte by byte, each byte encoded as two hex characters. */ p = rs->buf; - i = hex2bin (p, myaddr, todo); + decoded_octets = hex2bin (p, myaddr, todo_bytes * byte_size); /* Return what we have. Let higher layers handle partial reads. */ - *xfered_len = (ULONGEST) i; + *xfered_len = (ULONGEST) (decoded_octets / byte_size); return TARGET_XFER_OK; } @@ -6906,7 +6919,7 @@ remote_read_bytes_1 (CORE_ADDR memaddr, gdb_byte *myaddr, ULONGEST len, static enum target_xfer_status remote_xfer_live_readonly_partial (struct target_ops *ops, gdb_byte *readbuf, ULONGEST memaddr, ULONGEST len, - ULONGEST *xfered_len) + int byte_size, ULONGEST *xfered_len) { struct target_section *secp; struct target_section_table *table; @@ -6929,7 +6942,7 @@ remote_xfer_live_readonly_partial (struct target_ops *ops, gdb_byte *readbuf, if (memend <= p->endaddr) { /* Entire transfer is within this section. */ - return remote_read_bytes_1 (memaddr, readbuf, len, + return remote_read_bytes_1 (memaddr, readbuf, len, byte_size, xfered_len); } else if (memaddr >= p->endaddr) @@ -6941,7 +6954,7 @@ remote_xfer_live_readonly_partial (struct target_ops *ops, gdb_byte *readbuf, { /* This section overlaps the transfer. Just do half. */ len = p->endaddr - memaddr; - return remote_read_bytes_1 (memaddr, readbuf, len, + return remote_read_bytes_1 (memaddr, readbuf, len, byte_size, xfered_len); } } @@ -6957,7 +6970,8 @@ remote_xfer_live_readonly_partial (struct target_ops *ops, gdb_byte *readbuf, static enum target_xfer_status remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr, - gdb_byte *myaddr, ULONGEST len, ULONGEST *xfered_len) + gdb_byte *myaddr, ULONGEST len, int byte_size, + ULONGEST *xfered_len) { if (len == 0) return TARGET_XFER_EOF; @@ -6995,7 +7009,7 @@ remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr, /* This goes through the topmost target again. */ res = remote_xfer_live_readonly_partial (ops, myaddr, memaddr, - len, xfered_len); + len, byte_size, xfered_len); if (res == TARGET_XFER_OK) return TARGET_XFER_OK; else @@ -7017,7 +7031,7 @@ remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr, } } - return remote_read_bytes_1 (memaddr, myaddr, len, xfered_len); + return remote_read_bytes_1 (memaddr, myaddr, len, byte_size, xfered_len); } @@ -7103,7 +7117,7 @@ remote_flash_write (struct target_ops *ops, ULONGEST address, &saved_remote_timeout); remote_timeout = remote_flash_timeout; - ret = remote_write_bytes_aux ("vFlashWrite:", address, data, length, + ret = remote_write_bytes_aux ("vFlashWrite:", address, data, length, 1, xfered_len,'X', 0); do_cleanups (back_to); @@ -8775,7 +8789,7 @@ remote_write_qxfer (struct target_ops *ops, const char *object_name, /* Escape as much data as fits into rs->buf. */ buf_len = remote_escape_output - (writebuf, len, (gdb_byte *) rs->buf + i, &max_size, max_size); + (writebuf, len, 1, (gdb_byte *) rs->buf + i, &max_size, max_size); if (putpkt_binary (rs->buf, i + buf_len) < 0 || getpkt_sane (&rs->buf, &rs->buf_size, 0) < 0 @@ -8886,6 +8900,7 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object, int i; char *p2; char query_type; + int byte_size = gdbarch_memory_byte_size (target_gdbarch ()); set_remote_traceframe (); set_general_thread (inferior_ptid); @@ -8902,9 +8917,11 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object, return TARGET_XFER_EOF; if (writebuf != NULL) - return remote_write_bytes (offset, writebuf, len, xfered_len); + return remote_write_bytes (offset, writebuf, len, byte_size, + xfered_len); else - return remote_read_bytes (ops, offset, readbuf, len, xfered_len); + return remote_read_bytes (ops, offset, readbuf, len, byte_size, + xfered_len); } /* Handle SPU memory using qxfer packets. */ @@ -9137,7 +9154,7 @@ remote_search_memory (struct target_ops* ops, /* Escape as much data as fits into rs->buf. */ escaped_pattern_len = - remote_escape_output (pattern, pattern_len, (gdb_byte *) rs->buf + i, + remote_escape_output (pattern, pattern_len, 1, (gdb_byte *) rs->buf + i, &used_pattern_len, max_size); /* Bail if the pattern is too large. */ @@ -9922,7 +9939,7 @@ remote_hostio_pwrite (struct target_ops *self, remote_buffer_add_int (&p, &left, offset); remote_buffer_add_string (&p, &left, ","); - p += remote_escape_output (write_buf, len, (gdb_byte *) p, &out_len, + p += remote_escape_output (write_buf, len, 1, (gdb_byte *) p, &out_len, get_remote_packet_size () - (p - rs->buf)); return remote_hostio_send_command (p - rs->buf, PACKET_vFile_pwrite,