* [PATCH 3/7] Clarify doc about memory read/write and non-8-bits bytes
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
2015-04-08 19:56 ` [PATCH 1/7] Various cleanups in target read/write code Simon Marchi
2015-04-08 19:56 ` [PATCH 7/7] MI: Consider byte size when reading/writing memory Simon Marchi
@ 2015-04-08 19:56 ` Simon Marchi
2015-04-09 7:02 ` Eli Zaretskii
2015-04-08 19:56 ` [PATCH 5/7] target: consider byte size when reading/writing memory Simon Marchi
` (4 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2015-04-08 19:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
This patch modifies the manual to clarify the MI, RSP and Python APIs in
regard to reading/writing memory on architectures with non-8-bits bytes.
Care is taken to use the word byte when referring to one piece of the
smallest addressable size on the current architecture and the word octet
when referring to an 8-bits data piece. I try to avoid "word", because
it can be ambiguous.
For MI, -data-{read,write}-memory are not modified, since they are
deprecated.
gdb/doc/ChangeLog:
* gdb.texinfo (GDB/MI Data Manipulation): Clarify usage of
bytes and octets for -data-{read,write}-memory-bytes.
(Packets): Same for the m, M and X packets.
* python.texi (Inferiors In Python): Same for read_memory and
write_memory.
---
gdb/doc/gdb.texinfo | 41 ++++++++++++++++++++++-------------------
gdb/doc/python.texi | 5 +++--
2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d794893..e4dc24b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -29935,18 +29935,19 @@ where:
@table @samp
@item @var{address}
-An expression specifying the address of the first memory word to be
+An expression specifying the address of the first memory byte to be
read. Complex expressions containing embedded white space should be
quoted using the C convention.
@item @var{count}
-The number of bytes to read. This should be an integer literal.
+The number of target memory bytes to read. This should be an integer
+literal.
@item @var{byte-offset}
-The offsets in bytes relative to @var{address} at which to start
-reading. This should be an integer literal. This option is provided
-so that a frontend is not required to first evaluate address and then
-perform address arithmetics itself.
+The offset in target memory bytes relative to @var{address} at which to
+start reading. This should be an integer literal. This option is
+provided so that a frontend is not required to first evaluate address
+and then perform address arithmetics itself.
@end table
@@ -29979,8 +29980,9 @@ The start address of the memory block, as hexadecimal literal.
The end address of the memory block, as hexadecimal literal.
@item offset
-The offset of the memory block, as hexadecimal literal, relative to
-the start address passed to @code{-data-read-memory-bytes}.
+The offset of the memory block in target memory bytes, as hexadecimal
+literal, relative to the start address passed to
+@code{-data-read-memory-bytes}.
@item contents
The contents of the memory block, in hex.
@@ -30020,17 +30022,18 @@ where:
@table @samp
@item @var{address}
-An expression specifying the address of the first memory word to be
+An expression specifying the address of the first memory byte to be
written. Complex expressions containing embedded white space should be
quoted using the C convention.
@item @var{contents}
-The hex-encoded bytes to write.
+The hex-encoded bytes to write. It is an error if @var{contents} does
+not represent an integral number of target memory bytes.
@item @var{count}
-Optional argument indicating the number of bytes to be written. If @var{count}
-is greater than @var{contents}' length, @value{GDBN} will repeatedly
-write @var{contents} until it fills @var{count} bytes.
+Optional argument indicating the number of target bytes to be written.
+If @var{count} is greater than @var{contents}' length, @value{GDBN} will
+repeatedly write @var{contents} until it fills @var{count} bytes.
@end table
@@ -34665,7 +34668,7 @@ probes the target state as if a new connection was opened
@item m @var{addr},@var{length}
@cindex @samp{m} packet
-Read @var{length} bytes of memory starting at address @var{addr}.
+Read @var{length} octets of memory starting at address @var{addr}.
Note that @var{addr} may not be aligned to any particular boundary.
The stub need not use any particular size or alignment when gathering
@@ -34680,8 +34683,8 @@ suitable for accessing memory-mapped I/O devices.
Reply:
@table @samp
@item @var{XX@dots{}}
-Memory contents; each byte is transmitted as a two-digit hexadecimal
-number. The reply may contain fewer bytes than requested if the
+Memory contents; each octet is transmitted as a two-digit hexadecimal
+number. The reply may contain fewer octets than requested if the
server was able to read only part of the region of memory.
@item E @var{NN}
@var{NN} is errno
@@ -34689,8 +34692,8 @@ server was able to read only part of the region of memory.
@item M @var{addr},@var{length}:@var{XX@dots{}}
@cindex @samp{M} packet
-Write @var{length} bytes of memory starting at address @var{addr}.
-The data is given by @var{XX@dots{}}; each byte is transmitted as a two-digit
+Write @var{length} octets of memory starting at address @var{addr}.
+The data is given by @var{XX@dots{}}; each octet is transmitted as a two-digit
hexadecimal number.
Reply:
@@ -35005,7 +35008,7 @@ for success (@pxref{Stop Reply Packets})
@anchor{X packet}
@cindex @samp{X} packet
Write data to memory, where the data is transmitted in binary.
-Memory is specified by its address @var{addr} and number of bytes @var{length};
+Memory is specified by its address @var{addr} and number of octets @var{length};
@samp{@var{XX}@dots{}} is binary data (@pxref{Binary Data}).
Reply:
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 098d718..b31bce0 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2718,7 +2718,7 @@ return an empty tuple.
@findex Inferior.read_memory
@defun Inferior.read_memory (address, length)
-Read @var{length} bytes of memory from the inferior, starting at
+Read @var{length} target bytes of memory from the inferior, starting at
@var{address}. Returns a buffer object, which behaves much like an array
or a string. It can be modified and given to the
@code{Inferior.write_memory} function. In @code{Python} 3, the return
@@ -2731,7 +2731,8 @@ Write the contents of @var{buffer} to the inferior, starting at
@var{address}. The @var{buffer} parameter must be a Python object
which supports the buffer protocol, i.e., a string, an array or the
object returned from @code{Inferior.read_memory}. If given, @var{length}
-determines the number of bytes from @var{buffer} to be written.
+determines the number of target memory bytes from @var{buffer} to be
+written.
@end defun
@findex gdb.search_memory
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/7] remote: Consider byte size when reading/writing memory
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
` (5 preceding siblings ...)
2015-04-08 19:56 ` [PATCH 2/7] Cleanup some docs about memory write Simon Marchi
@ 2015-04-08 19:56 ` Simon Marchi
2015-04-09 8:20 ` [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Eli Zaretskii
7 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-08 19:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
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
<HEADER><ADDRESS>,<LENGTH>:<DATA>
- where encoding of <DATA> is termined by PACKET_FORMAT.
+ where encoding of <DATA> is terminated by PACKET_FORMAT.
If USE_LENGTH is 0, then the <LENGTH> 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<memaddr>,<len>:...#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: "<header><memaddr>,<len>:". */
@@ -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>". */
memaddr = remote_address_masked (memaddr);
@@ -6741,10 +6750,10 @@ remote_write_bytes_aux (const char *header, CORE_ADDR memaddr,
/* Append ",". */
*p++ = ',';
- /* Append <len>. Retain the location/size of <len>. It may need to
- be adjusted once the packet body has been created. */
+ /* Append <len_bytes>. 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>","<len>". */
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);
}
\f
@@ -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,
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/7] target: consider byte size when reading/writing memory
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
` (2 preceding siblings ...)
2015-04-08 19:56 ` [PATCH 3/7] Clarify doc about memory read/write and non-8-bits bytes Simon Marchi
@ 2015-04-08 19:56 ` Simon Marchi
2015-04-08 19:56 ` [PATCH 4/7] gdbarch: add memory_byte_size method Simon Marchi
` (3 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-08 19:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
If we are reading/writing from a memory object, the length represents
the number of "addresses" to read/write, so the byte size needs to be
taken into account when allocating memory on gdb's side.
gdb/ChangeLog:
* target.c (target_read): Consider byte size when reading from a
memory object.
(read_memory_robust): Same.
(read_whatever_is_readable): Same.
(target_write_with_progress): Consider byte size when
writing to a memory object.
* target.h (target_read): Update documentation.
(target_write): Add documentation.
---
gdb/target.c | 35 ++++++++++++++++++++++++++++-------
gdb/target.h | 26 ++++++++++++++++++++++----
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/gdb/target.c b/gdb/target.c
index bd9a0eb..d3ada16 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1589,6 +1589,15 @@ target_read (struct target_ops *ops,
ULONGEST offset, LONGEST len)
{
LONGEST xfered_total = 0;
+ int byte_size = 1;
+
+ /* If we are reading a memory object, find the length of a byte for that
+ architecture. */
+ if (object == TARGET_OBJECT_MEMORY
+ || object == TARGET_OBJECT_STACK_MEMORY
+ || object == TARGET_OBJECT_CODE_MEMORY
+ || object == TARGET_OBJECT_RAW_MEMORY)
+ byte_size = gdbarch_memory_byte_size (target_gdbarch());
while (xfered_total < len)
{
@@ -1596,7 +1605,7 @@ target_read (struct target_ops *ops,
enum target_xfer_status status;
status = target_read_partial (ops, object, annex,
- buf + xfered_total,
+ buf + xfered_total * byte_size,
offset + xfered_total, len - xfered_total,
&xfered_partial);
@@ -1639,6 +1648,7 @@ target_read (struct target_ops *ops,
static void
read_whatever_is_readable (struct target_ops *ops,
const ULONGEST begin, const ULONGEST end,
+ int byte_size,
VEC(memory_read_result_s) **result)
{
gdb_byte *buf = xmalloc (end - begin);
@@ -1705,7 +1715,7 @@ read_whatever_is_readable (struct target_ops *ops,
}
xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
- buf + (first_half_begin - begin),
+ buf + (first_half_begin - begin) * byte_size,
first_half_begin,
first_half_end - first_half_begin);
@@ -1741,8 +1751,9 @@ read_whatever_is_readable (struct target_ops *ops,
/* The [current_end, end) range has been read. */
LONGEST region_len = end - current_end;
- r.data = xmalloc (region_len);
- memcpy (r.data, buf + current_end - begin, region_len);
+ r.data = xmalloc (region_len * byte_size);
+ memcpy (r.data, buf + (current_end - begin) * byte_size,
+ region_len * byte_size);
r.begin = current_end;
r.end = end;
xfree (buf);
@@ -1769,6 +1780,7 @@ read_memory_robust (struct target_ops *ops,
const ULONGEST offset, const LONGEST len)
{
VEC(memory_read_result_s) *result = 0;
+ int byte_size = gdbarch_memory_byte_size (target_gdbarch ());
LONGEST xfered_total = 0;
while (xfered_total < len)
@@ -1794,7 +1806,7 @@ read_memory_robust (struct target_ops *ops,
else
{
LONGEST to_read = min (len - xfered_total, region_len);
- gdb_byte *buffer = (gdb_byte *)xmalloc (to_read);
+ gdb_byte *buffer = (gdb_byte *) xmalloc (to_read * byte_size);
LONGEST xfered_partial =
target_read (ops, TARGET_OBJECT_MEMORY, NULL,
@@ -1806,7 +1818,7 @@ read_memory_robust (struct target_ops *ops,
/* Got an error reading full chunk. See if maybe we can read
some subrange. */
xfree (buffer);
- read_whatever_is_readable (ops, offset + xfered_total,
+ read_whatever_is_readable (ops, offset + xfered_total, byte_size,
offset + xfered_total + to_read, &result);
xfered_total += to_read;
}
@@ -1836,6 +1848,15 @@ target_write_with_progress (struct target_ops *ops,
void (*progress) (ULONGEST, void *), void *baton)
{
LONGEST xfered_total = 0;
+ int byte_size = 1;
+
+ /* If we are writing to a memory object, find the length of a byte for that
+ architecture. */
+ if (object == TARGET_OBJECT_MEMORY
+ || object == TARGET_OBJECT_STACK_MEMORY
+ || object == TARGET_OBJECT_CODE_MEMORY
+ || object == TARGET_OBJECT_RAW_MEMORY)
+ byte_size = gdbarch_memory_byte_size (target_gdbarch ());
/* Give the progress callback a chance to set up. */
if (progress)
@@ -1847,7 +1868,7 @@ target_write_with_progress (struct target_ops *ops,
enum target_xfer_status status;
status = target_write_partial (ops, object, annex,
- (gdb_byte *) buf + xfered_total,
+ buf + xfered_total * byte_size,
offset + xfered_total, len - xfered_total,
&xfered_partial);
diff --git a/gdb/target.h b/gdb/target.h
index ad50f07..c72f8b4 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -259,12 +259,15 @@ typedef enum target_xfer_status
ULONGEST len,
ULONGEST *xfered_len);
-/* Request that OPS transfer up to LEN 8-bit bytes of the target's
- OBJECT. The OFFSET, for a seekable object, specifies the
- starting point. The ANNEX can be used to provide additional
+/* Request that OPS transfer up to LEN bytes of the target's OBJECT. When
+ reading from a memory object, the byte size is architecture dependent and
+ can be found using gdbarch_memory_byte_size. Otherwise, a byte is 1 octet
+ long. BUF should point to a buffer large enough to hold the read data,
+ taking into account the byte size. The OFFSET, for a seekable object,
+ specifies the starting point. The ANNEX can be used to provide additional
data-specific information to the target.
- Return the number of bytes actually transfered, or a negative error
+ Return the number of bytes actually transferred, or a negative error
code (an 'enum target_xfer_error' value) if the transfer is not
supported or otherwise fails. Return of a positive value less than
LEN indicates that no further transfer is possible. Unlike the raw
@@ -294,6 +297,21 @@ extern VEC(memory_read_result_s)* read_memory_robust (struct target_ops *ops,
const ULONGEST offset,
const LONGEST len);
+/* Request that OPS transfer up to LEN bytes from BUF to the target's
+ OBJECT. When writing to a memory object, the byte size is
+ architecture dependent and can be found using
+ gdbarch_memory_byte_size. Otherwise, a byte is 1 octet long. The
+ OFFSET, for a seekable object, specifies the starting point. The
+ ANNEX can be used to provide additional data-specific information to
+ the target.
+
+ Return the number of bytes actually transferred, or a negative error
+ code (an 'enum target_xfer_status' value) if the transfer is not
+ supported or otherwise fails. Return of a positive value less than
+ LEN indicates that no further transfer is possible. Unlike the raw
+ to_xfer_partial interface, callers of these functions do not need to
+ retry partial transfers. */
+
extern LONGEST target_write (struct target_ops *ops,
enum target_object object,
const char *annex, const gdb_byte *buf,
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 7/7] MI: Consider byte size when reading/writing memory
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
2015-04-08 19:56 ` [PATCH 1/7] Various cleanups in target read/write code Simon Marchi
@ 2015-04-08 19:56 ` Simon Marchi
2015-04-08 19:56 ` [PATCH 3/7] Clarify doc about memory read/write and non-8-bits bytes Simon Marchi
` (5 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-08 19:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
As a user of the target memory read/write interface, the MI code must
adjust its memory allocations to take into account the byte size of the
target.
gdb/ChangeLog:
mi/mi-main.c (mi_cmd_data_read_memory_bytes): Consider byte
size.
(mi_cmd_data_write_memory_bytes): Same.
---
gdb/mi/mi-main.c | 51 +++++++++++++++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 20 deletions(-)
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 2733e80..99caf67 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1595,6 +1595,7 @@ mi_cmd_data_read_memory_bytes (char *command, char **argv, int argc)
int ix;
VEC(memory_read_result_s) *result;
long offset = 0;
+ int byte_size = gdbarch_memory_byte_size (gdbarch);
int oind = 0;
char *oarg;
enum opt
@@ -1650,10 +1651,11 @@ mi_cmd_data_read_memory_bytes (char *command, char **argv, int argc)
- addr);
ui_out_field_core_addr (uiout, "end", gdbarch, read_result->end);
- data = xmalloc ((read_result->end - read_result->begin) * 2 + 1);
+ data = xmalloc (
+ (read_result->end - read_result->begin) * 2 * byte_size + 1);
for (i = 0, p = data;
- i < (read_result->end - read_result->begin);
+ i < ((read_result->end - read_result->begin) * byte_size);
++i, p += 2)
{
sprintf (p, "%02x", read_result->data[i]);
@@ -1762,29 +1764,35 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
char *cdata;
gdb_byte *data;
gdb_byte *databuf;
- size_t len, i, steps, remainder;
- long int count, j;
+ size_t len_chars, len_octets, len_bytes, i, steps, remainder_bytes;
+ long int count_bytes;
struct cleanup *back_to;
+ int byte_size;
if (argc != 2 && argc != 3)
error (_("Usage: ADDR DATA [COUNT]."));
addr = parse_and_eval_address (argv[0]);
cdata = argv[1];
- if (strlen (cdata) % 2)
- error (_("Hex-encoded '%s' must have an even number of characters."),
+ len_chars = strlen (cdata);
+ byte_size = gdbarch_memory_byte_size (get_current_arch ());
+
+ if (len_chars % (byte_size * 2) != 0)
+ error (_("Hex-encoded '%s' must represent an integral number of bytes."),
cdata);
- len = strlen (cdata)/2;
+ len_octets = len_chars / 2;
+ len_bytes = len_octets / byte_size;
+
if (argc == 3)
- count = strtoul (argv[2], NULL, 10);
+ count_bytes = strtoul (argv[2], NULL, 10);
else
- count = len;
+ count_bytes = len_bytes;
- databuf = xmalloc (len * sizeof (gdb_byte));
+ databuf = xmalloc (len_octets * sizeof (gdb_byte));
back_to = make_cleanup (xfree, databuf);
- for (i = 0; i < len; ++i)
+ for (i = 0; i < len_octets; ++i)
{
int x;
if (sscanf (cdata + i * 2, "%02x", &x) != 1)
@@ -1792,20 +1800,23 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
databuf[i] = (gdb_byte) x;
}
- if (len < count)
+ if (len_bytes < count_bytes)
{
/* Pattern is made of less bytes than count:
repeat pattern to fill memory. */
- data = xmalloc (count);
+ data = xmalloc (count_bytes * byte_size);
make_cleanup (xfree, data);
- steps = count / len;
- remainder = count % len;
- for (j = 0; j < steps; j++)
- memcpy (data + j * len, databuf, len);
+ /* Number of times the pattern is entirely repeated. */
+ steps = count_bytes / len_bytes;
+ /* Number of remaining target bytes. */
+ remainder_bytes = count_bytes % len_bytes;
+ for (i = 0; i < steps; i++)
+ memcpy (data + i * len_octets, databuf, len_octets);
- if (remainder > 0)
- memcpy (data + steps * len, databuf, remainder);
+ if (remainder_bytes > 0)
+ memcpy (data + steps * len_octets, databuf,
+ remainder_bytes * byte_size);
}
else
{
@@ -1814,7 +1825,7 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
data = databuf;
}
- write_memory_with_notification (addr, data, count);
+ write_memory_with_notification (addr, data, count_bytes);
do_cleanups (back_to);
}
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/7] gdbarch: add memory_byte_size method
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
` (3 preceding siblings ...)
2015-04-08 19:56 ` [PATCH 5/7] target: consider byte size when reading/writing memory Simon Marchi
@ 2015-04-08 19:56 ` Simon Marchi
2015-04-08 19:56 ` [PATCH 2/7] Cleanup some docs about memory write Simon Marchi
` (2 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-08 19:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Add a new gdbarch method to get the length of a memory byte for a given
architecture. The default implementation returns 1.
gdb/ChangeLog:
* arch-utils.h (default_memory_byte_size): New.
* arch-utils.c (default_memory_byte_size): New.
* gdbarch.sh (memory_byte_size): New.
* gdbarch.h: Re-generated.
* gdbarch.c: Re-generated.
---
gdb/arch-utils.c | 9 +++++++++
gdb/arch-utils.h | 1 +
gdb/gdbarch.c | 23 +++++++++++++++++++++++
gdb/gdbarch.h | 6 ++++++
gdb/gdbarch.sh | 4 ++++
5 files changed, 43 insertions(+)
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index e1c8ab0..4f3cb7b 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -882,6 +882,15 @@ default_gnu_triplet_regexp (struct gdbarch *gdbarch)
return gdbarch_bfd_arch_info (gdbarch)->arch_name;
}
+/* Default method for gdbarch_memory_byte_size. By default, a memory byte has
+ a size of 1 octet. */
+
+int
+default_memory_byte_size (struct gdbarch *gdbarch)
+{
+ return 1;
+}
+
/* -Wmissing-prototypes */
extern initialize_file_ftype _initialize_gdbarch_utils;
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index ed3bec9..ecf5e0d 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -202,5 +202,6 @@ extern void default_skip_permanent_breakpoint (struct regcache *regcache);
extern CORE_ADDR default_infcall_mmap (CORE_ADDR size, unsigned prot);
extern char *default_gcc_target_options (struct gdbarch *gdbarch);
extern const char *default_gnu_triplet_regexp (struct gdbarch *gdbarch);
+extern int default_memory_byte_size (struct gdbarch *gdbarch);
#endif
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 97874c9..58b9630 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -328,6 +328,7 @@ struct gdbarch
gdbarch_infcall_mmap_ftype *infcall_mmap;
gdbarch_gcc_target_options_ftype *gcc_target_options;
gdbarch_gnu_triplet_regexp_ftype *gnu_triplet_regexp;
+ gdbarch_memory_byte_size_ftype *memory_byte_size;
};
/* Create a new ``struct gdbarch'' based on information provided by
@@ -428,6 +429,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
gdbarch->infcall_mmap = default_infcall_mmap;
gdbarch->gcc_target_options = default_gcc_target_options;
gdbarch->gnu_triplet_regexp = default_gnu_triplet_regexp;
+ gdbarch->memory_byte_size = default_memory_byte_size;
/* gdbarch_alloc() */
return gdbarch;
@@ -660,6 +662,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
/* Skip verify of infcall_mmap, invalid_p == 0 */
/* Skip verify of gcc_target_options, invalid_p == 0 */
/* Skip verify of gnu_triplet_regexp, invalid_p == 0 */
+ /* Skip verify of memory_byte_size, invalid_p == 0 */
buf = ui_file_xstrdup (log, &length);
make_cleanup (xfree, buf);
if (length > 0)
@@ -1095,6 +1098,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
"gdbarch_dump: max_insn_length = %s\n",
plongest (gdbarch->max_insn_length));
fprintf_unfiltered (file,
+ "gdbarch_dump: memory_byte_size = <%s>\n",
+ host_address_to_string (gdbarch->memory_byte_size));
+ fprintf_unfiltered (file,
"gdbarch_dump: memory_insert_breakpoint = <%s>\n",
host_address_to_string (gdbarch->memory_insert_breakpoint));
fprintf_unfiltered (file,
@@ -4707,6 +4713,23 @@ set_gdbarch_gnu_triplet_regexp (struct gdbarch *gdbarch,
gdbarch->gnu_triplet_regexp = gnu_triplet_regexp;
}
+int
+gdbarch_memory_byte_size (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->memory_byte_size != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_memory_byte_size called\n");
+ return gdbarch->memory_byte_size (gdbarch);
+}
+
+void
+set_gdbarch_memory_byte_size (struct gdbarch *gdbarch,
+ gdbarch_memory_byte_size_ftype memory_byte_size)
+{
+ gdbarch->memory_byte_size = memory_byte_size;
+}
+
/* Keep a registry of per-architecture data-pointers required by GDB
modules. */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index c94c19c..169f752 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1459,6 +1459,12 @@ typedef const char * (gdbarch_gnu_triplet_regexp_ftype) (struct gdbarch *gdbarch
extern const char * gdbarch_gnu_triplet_regexp (struct gdbarch *gdbarch);
extern void set_gdbarch_gnu_triplet_regexp (struct gdbarch *gdbarch, gdbarch_gnu_triplet_regexp_ftype *gnu_triplet_regexp);
+/* Return the length in octets of a memory byte on this architecture. */
+
+typedef int (gdbarch_memory_byte_size_ftype) (struct gdbarch *gdbarch);
+extern int gdbarch_memory_byte_size (struct gdbarch *gdbarch);
+extern void set_gdbarch_memory_byte_size (struct gdbarch *gdbarch, gdbarch_memory_byte_size_ftype *memory_byte_size);
+
/* Definition for an unknown syscall, used basically in error-cases. */
#define UNKNOWN_SYSCALL (-1)
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 0f303a4..607caa1 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1109,6 +1109,10 @@ m:char *:gcc_target_options:void:::default_gcc_target_options::0
# returns the BFD architecture name, which is correct in nearly every
# case.
m:const char *:gnu_triplet_regexp:void:::default_gnu_triplet_regexp::0
+
+# Return the length in octets of a memory byte on this architecture.
+m:int:memory_byte_size:void:::default_memory_byte_size::0
+
EOF
}
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/7] Cleanup some docs about memory write
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
` (4 preceding siblings ...)
2015-04-08 19:56 ` [PATCH 4/7] gdbarch: add memory_byte_size method Simon Marchi
@ 2015-04-08 19:56 ` Simon Marchi
2015-04-08 19:56 ` [PATCH 6/7] remote: Consider byte size when reading/writing memory Simon Marchi
2015-04-09 8:20 ` [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Eli Zaretskii
7 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-08 19:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Some docs seemed outdated. In the case of target_write_memory, the docs
in target.c and target/target.h diverged a bit, so I tried to find a
reasonnable in-between version.
gdb/ChangeLog:
* corefile.c (write_memory): Update doc.
* gdbcore.h (write_memory): Same.
* target.c (target_write_memory): Same.
* target/target.h (target_write_memory): Same.
---
gdb/corefile.c | 4 ++--
gdb/gdbcore.h | 6 ++----
gdb/target.c | 6 +-----
gdb/target/target.h | 14 +++++++-------
4 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/gdb/corefile.c b/gdb/corefile.c
index a042e6d..83b0e80 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -383,8 +383,8 @@ read_memory_typed_address (CORE_ADDR addr, struct type *type)
return extract_typed_address (buf, type);
}
-/* Same as target_write_memory, but report an error if can't
- write. */
+/* See gdbcore.h. */
+
void
write_memory (CORE_ADDR memaddr,
const bfd_byte *myaddr, ssize_t len)
diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h
index 63a75f0..1106db8 100644
--- a/gdb/gdbcore.h
+++ b/gdb/gdbcore.h
@@ -101,10 +101,8 @@ extern void read_memory_string (CORE_ADDR, char *, int);
CORE_ADDR read_memory_typed_address (CORE_ADDR addr, struct type *type);
-/* This takes a char *, not void *. This is probably right, because
- passing in an int * or whatever is wrong with respect to
- byteswapping, alignment, different sizes for host vs. target types,
- etc. */
+/* Same as target_write_memory, but report an error if can't
+ write. */
extern void write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
ssize_t len);
diff --git a/gdb/target.c b/gdb/target.c
index fcf7090..bd9a0eb 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1464,11 +1464,7 @@ target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
return TARGET_XFER_E_IO;
}
-/* Write LEN bytes from MYADDR to target memory at address MEMADDR.
- Returns either 0 for success or TARGET_XFER_E_IO if any
- error occurs. If an error occurs, no guarantee is made about how
- much data got written. Callers that can deal with partial writes
- should call target_write. */
+/* See target/target.h. */
int
target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
diff --git a/gdb/target/target.h b/gdb/target/target.h
index 05ac758..355851e 100644
--- a/gdb/target/target.h
+++ b/gdb/target/target.h
@@ -48,13 +48,13 @@ extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
extern int target_read_uint32 (CORE_ADDR memaddr, uint32_t *result);
-/* Write LEN bytes from MYADDR to target memory at address MEMADDR.
- Return zero for success, nonzero if any error occurs. This
- function must be provided by the client. Implementations of this
- function may define and use their own error codes, but functions
- in the common, nat and target directories must treat the return
- code as opaque. No guarantee is made about the contents of the
- data at MEMADDR if any error occurs. */
+/* Write LEN target memory bytes from MYADDR to target memory at address
+ MEMADDR. Return zero for success or TARGET_XFER_E_IO if any error
+ occurs. This function must be provided by the client.
+ Implementations of this function may define and use their own error
+ codes, but functions in the common, nat and target directories must
+ treat the return code as opaque. No guarantee is made about how much
+ data got written if any error occurs. */
extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
ssize_t len);
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/7] Various cleanups in target read/write code
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
@ 2015-04-08 19:56 ` Simon Marchi
2015-04-08 19:56 ` [PATCH 7/7] MI: Consider byte size when reading/writing memory Simon Marchi
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-08 19:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
This contains various cleanups in the target memory read and write code.
They are not directly related to the non-8-bits changes, but they
clarify things a bit down the line.
gdb/ChangeLog:
* target.c (target_read): Rename variables and use
TARGET_XFER_E_IO.
(target_read_with_progress): Same.
(read_memory_robust): Constify parameters and rename
variables.
(read_whatever_is_readable): Constify parameters,
rename variables, adjust formatting.
* target.h (read_memory_robust): Constify parameters.
---
gdb/target.c | 90 +++++++++++++++++++++++++++++++-----------------------------
gdb/target.h | 6 ++--
2 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/gdb/target.c b/gdb/target.c
index 306c21d..fcf7090 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1592,28 +1592,28 @@ target_read (struct target_ops *ops,
const char *annex, gdb_byte *buf,
ULONGEST offset, LONGEST len)
{
- LONGEST xfered = 0;
+ LONGEST xfered_total = 0;
- while (xfered < len)
+ while (xfered_total < len)
{
- ULONGEST xfered_len;
+ ULONGEST xfered_partial;
enum target_xfer_status status;
status = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- offset + xfered, len - xfered,
- &xfered_len);
+ buf + xfered_total,
+ offset + xfered_total, len - xfered_total,
+ &xfered_partial);
/* Call an observer, notifying them of the xfer progress? */
if (status == TARGET_XFER_EOF)
- return xfered;
+ return xfered_total;
else if (status == TARGET_XFER_OK)
{
- xfered += xfered_len;
+ xfered_total += xfered_partial;
QUIT;
}
else
- return -1;
+ return TARGET_XFER_E_IO;
}
return len;
@@ -1642,7 +1642,7 @@ target_read (struct target_ops *ops,
static void
read_whatever_is_readable (struct target_ops *ops,
- ULONGEST begin, ULONGEST end,
+ const ULONGEST begin, const ULONGEST end,
VEC(memory_read_result_s) **result)
{
gdb_byte *buf = xmalloc (end - begin);
@@ -1669,7 +1669,7 @@ read_whatever_is_readable (struct target_ops *ops,
++current_begin;
}
else if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
- buf + (end-begin) - 1, end - 1, 1,
+ buf + (end - begin) - 1, end - 1, 1,
&xfered_len) == TARGET_XFER_OK)
{
forward = 0;
@@ -1691,7 +1691,7 @@ read_whatever_is_readable (struct target_ops *ops,
ULONGEST first_half_begin, first_half_end;
ULONGEST second_half_begin, second_half_end;
LONGEST xfer;
- ULONGEST middle = current_begin + (current_end - current_begin)/2;
+ ULONGEST middle = current_begin + (current_end - current_begin) / 2;
if (forward)
{
@@ -1723,7 +1723,7 @@ read_whatever_is_readable (struct target_ops *ops,
else
{
/* This half is not readable. Because we've tried one byte, we
- know some part of this half if actually redable. Go to the next
+ know some part of this half if actually readable. Go to the next
iteration to divide again and try to read.
We don't handle the other half, because this function only tries
@@ -1743,10 +1743,10 @@ read_whatever_is_readable (struct target_ops *ops,
else
{
/* The [current_end, end) range has been read. */
- LONGEST rlen = end - current_end;
+ LONGEST region_len = end - current_end;
- r.data = xmalloc (rlen);
- memcpy (r.data, buf + current_end - begin, rlen);
+ r.data = xmalloc (region_len);
+ memcpy (r.data, buf + current_end - begin, region_len);
r.begin = current_end;
r.end = end;
xfree (buf);
@@ -1769,57 +1769,59 @@ free_memory_read_result_vector (void *x)
}
VEC(memory_read_result_s) *
-read_memory_robust (struct target_ops *ops, ULONGEST offset, LONGEST len)
+read_memory_robust (struct target_ops *ops,
+ const ULONGEST offset, const LONGEST len)
{
VEC(memory_read_result_s) *result = 0;
- LONGEST xfered = 0;
- while (xfered < len)
+ LONGEST xfered_total = 0;
+ while (xfered_total < len)
{
- struct mem_region *region = lookup_mem_region (offset + xfered);
- LONGEST rlen;
+ struct mem_region *region = lookup_mem_region (offset + xfered_total);
+ LONGEST region_len;
/* If there is no explicit region, a fake one should be created. */
gdb_assert (region);
if (region->hi == 0)
- rlen = len - xfered;
+ region_len = len - xfered_total;
else
- rlen = region->hi - offset;
+ region_len = region->hi - offset;
if (region->attrib.mode == MEM_NONE || region->attrib.mode == MEM_WO)
{
/* Cannot read this region. Note that we can end up here only
if the region is explicitly marked inaccessible, or
'inaccessible-by-default' is in effect. */
- xfered += rlen;
+ xfered_total += region_len;
}
else
{
- LONGEST to_read = min (len - xfered, rlen);
+ LONGEST to_read = min (len - xfered_total, region_len);
gdb_byte *buffer = (gdb_byte *)xmalloc (to_read);
- LONGEST xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
- (gdb_byte *) buffer,
- offset + xfered, to_read);
+ LONGEST xfered_partial =
+ target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ (gdb_byte *) buffer,
+ offset + xfered_total, to_read);
/* Call an observer, notifying them of the xfer progress? */
- if (xfer <= 0)
+ if (xfered_partial <= 0)
{
/* Got an error reading full chunk. See if maybe we can read
some subrange. */
xfree (buffer);
- read_whatever_is_readable (ops, offset + xfered,
- offset + xfered + to_read, &result);
- xfered += to_read;
+ read_whatever_is_readable (ops, offset + xfered_total,
+ offset + xfered_total + to_read, &result);
+ xfered_total += to_read;
}
else
{
struct memory_read_result r;
r.data = buffer;
- r.begin = offset + xfered;
- r.end = r.begin + xfer;
+ r.begin = offset + xfered_total;
+ r.end = r.begin + xfered_partial;
VEC_safe_push (memory_read_result_s, result, &r);
- xfered += xfer;
+ xfered_total += xfered_partial;
}
QUIT;
}
@@ -1837,29 +1839,29 @@ target_write_with_progress (struct target_ops *ops,
ULONGEST offset, LONGEST len,
void (*progress) (ULONGEST, void *), void *baton)
{
- LONGEST xfered = 0;
+ LONGEST xfered_total = 0;
/* Give the progress callback a chance to set up. */
if (progress)
(*progress) (0, baton);
- while (xfered < len)
+ while (xfered_total < len)
{
- ULONGEST xfered_len;
+ ULONGEST xfered_partial;
enum target_xfer_status status;
status = target_write_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- offset + xfered, len - xfered,
- &xfered_len);
+ (gdb_byte *) buf + xfered_total,
+ offset + xfered_total, len - xfered_total,
+ &xfered_partial);
if (status != TARGET_XFER_OK)
- return status == TARGET_XFER_EOF ? xfered : -1;
+ return status == TARGET_XFER_EOF ? xfered_total : TARGET_XFER_E_IO;
if (progress)
- (*progress) (xfered_len, baton);
+ (*progress) (xfered_partial, baton);
- xfered += xfered_len;
+ xfered_total += xfered_partial;
QUIT;
}
return len;
diff --git a/gdb/target.h b/gdb/target.h
index f57e431..ad50f07 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -291,9 +291,9 @@ DEF_VEC_O(memory_read_result_s);
extern void free_memory_read_result_vector (void *);
extern VEC(memory_read_result_s)* read_memory_robust (struct target_ops *ops,
- ULONGEST offset,
- LONGEST len);
-
+ const ULONGEST offset,
+ const LONGEST len);
+
extern LONGEST target_write (struct target_ops *ops,
enum target_object object,
const char *annex, const gdb_byte *buf,
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
@ 2015-04-08 19:56 Simon Marchi
2015-04-08 19:56 ` [PATCH 1/7] Various cleanups in target read/write code Simon Marchi
` (7 more replies)
0 siblings, 8 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-08 19:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
At Ericsson, we work with an architecture which uses 16-bits bytes
instead of the common 8-bits bytes. There are a few examples in the DSP
world of processors that work with 16, 24 or 32-bits bytes. Over the
past year or so, we have adapted gdb to work with our architecture and
we now feel it's the right time to start contributing some of the
changes back. On gdb's side, it should be a step towards supporting a
new range of chips. On our side, it will help us stay closer to
mainline.
On such a system, memory is addressable in atomic chunks of 16-bits. On
a "normal" system, you have 8 bits of data associated with each memory
address:
Address Data
---------------
0x1000 0xaa
0x1001 0xbb
0x1002 0xcc
0x1003 0xdd
whereas on a system with 16-bits bytes, you have 16-bits of data per
address:
Address Data
---------------
0x1000 0xaaaa
0x1001 0xbbbb
0x1002 0xcccc
0x1003 0xdddd
To support these systems, GDB must be modified to consider the byte size
when reading/writing memory. This is what this first patch series is
about.
Also, on these systems, sizeof(char) == 1 == 16 bits. There is therefore
many places related to types and values handling that need to be
modified. This will be the subject of subsequent patch series.
I did a quick research of publicly available chips which use non-8-bits
bytes.
- TI C54x family (16-bits memory)
http://www.ti.com/lsds/ti/dsp/c5000_dsp/c54x/products.page
It seems like work had been done to port an ancient GDB for the
earlier C4x family a long time ago, but I can't find the source
anywhere. They must have done changes similar to ours in order to
support the 16-bits memory. If you know where to find that source,
I'd be really interested to see it!
http://www.elec.canterbury.ac.nz/c4x/
- Atmel ATMega family, the flash program space uses 16-bits words
http://www.atmel.com/products/microcontrollers/avr/megaavr.aspx
- Some DSPs targetting audio have 16-bits or 24-bits memory words.
http://www.analog.com/media/en/technical-documentation/data-sheets/ADSP-21990.pdf
- Another one by Freescale
http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=DSP56321
Terminology
-----------
Talking about this stuff becomes quickly confusing without a consistent
terminology. Here is what I try to stick to in my messages,
documentation and code:
- `octet` when referring to a chunk of 8 bits
- `byte`, `target byte` or `target memory byte`, to be even more
explicit, when referring to one chunk of the addressable size of the
memory
API proposition
---------------
The current APIs (MI and RSP) need to be clarified in order to be usable
with 16-bits (and other) systems. In the current state, it is not clear
if a length should be expressed in octets or target bytes. Here are the
changes we suggest. They are based on what we have been using for some
time, so we are confident it works quite well. This should not affect
any existing system using 8-bits memory (IOW, everything supported right
now).
For MI, the parameters representing numbers of bytes and offsets in
-data-read-memory-bytes
-data-write-memory-bytes
should be in target memory bytes. This makes the API easier to use,
because it's impossible to request an invalid amount of bytes. Also, it
feels more natural this way, because the client expresses the length as
the number of memory addresses it wants to read. Here is an example MI
session with an hypothetic system that uses 16-bits bytes.
-data-read-memory-bytes -o 4096 0 4
^done,memory=[{begin="0x1000",offset="0x0000",end="0x1004",contents="aaaabbbbccccdddd"}]
-data-write-memory-bytes 4096 eeeeffff 3
^done
-data-read-memory-bytes -o 4096 0 4
^done,memory=[{begin="0x1000",offset="0x0000",end="0x1004",contents="eeeeffffeeeedddd"}]
-data-write-memory-bytes 4096 ee
^error,msg="Some error about non integral number of bytes."
The error case in the last command is similar to the error thrown when
trying to write an odd number of hex digits. The number of given hex
digits must represent an integral number of bytes of the memory you are
writing to.
For RSP's m, M and X packets, the "length" parameters are used to
correctly encode or decode the packet at a low level, where we don't
want to have to deal with different target byte sizes. Therefore,
it would be easier if those lengths were always in octets. Here is an
example that corresponds to the previous MI example.
-> $m1000,8#??
<- aaaabbbbccccdddd
-> $M1000,6:eeeeffffeeee#??
<- OK
-> $m1000,8#??
<- eeeeffffeeeedddd
If there are any other RSP packets or MI commands that need such
clarification, it will be on a case-by-case basis, whatever makes more
sense for each particular one.
Simon Marchi (7):
Various cleanups in target read/write code
Cleanup some docs about memory write
Clarify doc about memory read/write and non-8-bits bytes
gdbarch: add memory_byte_size method
target: consider byte size when reading/writing memory
remote: Consider byte size when reading/writing memory
MI: Consider byte size when reading/writing memory
gdb/arch-utils.c | 9 +++
gdb/arch-utils.h | 1 +
gdb/common/rsp-low.c | 71 ++++++++++++++++-------
gdb/common/rsp-low.h | 16 +++---
gdb/corefile.c | 4 +-
gdb/doc/gdb.texinfo | 41 +++++++------
gdb/doc/python.texi | 5 +-
gdb/gdbarch.c | 23 ++++++++
gdb/gdbarch.h | 6 ++
gdb/gdbarch.sh | 4 ++
gdb/gdbcore.h | 6 +-
gdb/gdbserver/server.c | 4 +-
gdb/mi/mi-main.c | 51 ++++++++++-------
gdb/remote.c | 153 +++++++++++++++++++++++++++----------------------
gdb/target.c | 121 +++++++++++++++++++++-----------------
gdb/target.h | 32 ++++++++---
gdb/target/target.h | 14 ++---
17 files changed, 350 insertions(+), 211 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/7] Clarify doc about memory read/write and non-8-bits bytes
2015-04-08 19:56 ` [PATCH 3/7] Clarify doc about memory read/write and non-8-bits bytes Simon Marchi
@ 2015-04-09 7:02 ` Eli Zaretskii
0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2015-04-09 7:02 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Wed, 8 Apr 2015 15:56:15 -0400
>
> This patch modifies the manual to clarify the MI, RSP and Python APIs in
> regard to reading/writing memory on architectures with non-8-bits bytes.
>
> Care is taken to use the word byte when referring to one piece of the
> smallest addressable size on the current architecture and the word octet
> when referring to an 8-bits data piece. I try to avoid "word", because
> it can be ambiguous.
Thanks. However, I think we need to be more explicit about this
issue. Just using "target memory bytes" without ever explaining what
that means, or how it is different from the host bytes, makes these
changes more accurate, but not more clear to the reader.
So please add some short discussion of this issue (the node "Data"
sounds pertinent, or maybe "Memory"), and introduce there the "target
memory byte" term.
> +Optional argument indicating the number of target bytes to be written.
^^^^^^^^^^^^
Let's be consistent and use "target memory bytes" everywhere.
> -Read @var{length} bytes of memory starting at address @var{addr}.
> +Read @var{length} octets of memory starting at address @var{addr}.
"Octet" is not defined anywhere, so I think it should be part of the
above-mentioned introduction. Here, I would add a cross-reference to
that place.
> -Read @var{length} bytes of memory from the inferior, starting at
> +Read @var{length} target bytes of memory from the inferior, starting at
^^^^^^^^^^^^^^^^^^^^^^
"target memory bytes"
Thanks.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
` (6 preceding siblings ...)
2015-04-08 19:56 ` [PATCH 6/7] remote: Consider byte size when reading/writing memory Simon Marchi
@ 2015-04-09 8:20 ` Eli Zaretskii
2015-04-09 15:39 ` Simon Marchi
7 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2015-04-09 8:20 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Wed, 8 Apr 2015 15:56:12 -0400
>
> On such a system, memory is addressable in atomic chunks of 16-bits. On
> a "normal" system, you have 8 bits of data associated with each memory
> address:
>
> Address Data
> ---------------
> 0x1000 0xaa
> 0x1001 0xbb
> 0x1002 0xcc
> 0x1003 0xdd
>
> whereas on a system with 16-bits bytes, you have 16-bits of data per
> address:
>
> Address Data
> ---------------
> 0x1000 0xaaaa
> 0x1001 0xbbbb
> 0x1002 0xcccc
> 0x1003 0xdddd
>
> To support these systems, GDB must be modified to consider the byte size
> when reading/writing memory. This is what this first patch series is
> about.
>
> Also, on these systems, sizeof(char) == 1 == 16 bits. There is therefore
> many places related to types and values handling that need to be
> modified. This will be the subject of subsequent patch series.
I wonder: wouldn't it be possible to keep the current "byte == 8 bits"
notion, and instead to change the way addresses are interpreted by the
target back-end?
IOW, do we really need to expose this issue all the way to the higher
levels of GDB application code?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-09 8:20 ` [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Eli Zaretskii
@ 2015-04-09 15:39 ` Simon Marchi
2015-04-09 16:29 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2015-04-09 15:39 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 15-04-09 04:20 AM, Eli Zaretskii wrote:
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> CC: Simon Marchi <simon.marchi@ericsson.com>
>> Date: Wed, 8 Apr 2015 15:56:12 -0400
>>
>> On such a system, memory is addressable in atomic chunks of 16-bits. On
>> a "normal" system, you have 8 bits of data associated with each memory
>> address:
>>
>> Address Data
>> ---------------
>> 0x1000 0xaa
>> 0x1001 0xbb
>> 0x1002 0xcc
>> 0x1003 0xdd
>>
>> whereas on a system with 16-bits bytes, you have 16-bits of data per
>> address:
>>
>> Address Data
>> ---------------
>> 0x1000 0xaaaa
>> 0x1001 0xbbbb
>> 0x1002 0xcccc
>> 0x1003 0xdddd
>>
>> To support these systems, GDB must be modified to consider the byte size
>> when reading/writing memory. This is what this first patch series is
>> about.
>>
>> Also, on these systems, sizeof(char) == 1 == 16 bits. There is therefore
>> many places related to types and values handling that need to be
>> modified. This will be the subject of subsequent patch series.
>
> I wonder: wouldn't it be possible to keep the current "byte == 8 bits"
> notion, and instead to change the way addresses are interpreted by the
> target back-end?
>
> IOW, do we really need to expose this issue all the way to the higher
> levels of GDB application code?
Hi Eli,
I don't think there is an elegant way of making this work without gdb
knowing at least a bit about it. If you don't make some changes at one
level, you'll end up needing to make the equivalent changes at some other
level (still in gdb core). We tried a few different combinations, and
I don't know of any where we don't need to touch the core of gdb at some
point. I'd be happy to be proven wrong though!
From what I understand, your suggestion would be to treat addresses as
indexes of octets in memory. So, to read target bytes at addresses 3
and 4, I would have to ask gdb for 4 "gdb" bytes starting at address 6.
size == 2
v-------------------v
+---------+---------+---------+---------+---------+---------+
real idx | 0 | 1 | 2 | 3 | 4 | 5 |
+----+----+----+----+----+----+----+----+----+----+----+----+
octet idx | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 |
+----+----+----+----+----+----+----+----+----+----+----+----+
^-------------------^
size == 4
The backend would then divide everything by two and read 2 target bytes
starting at address 3.
If we require the user or the front-end to do that conversion, we just push
the responsibility over the fence to them. The interface will be quite poor
and unintuitive. For the developer working with that system 8 hours per day,
a size of 1 is one 16-bits byte. His debugger should understand that
language.
If I have a pointer p (char *p) and I want to examine memory starting at p,
I would do "x/10h p". That wouldn't give me what I want, as it would give me
memory at p/2.
Also, the gdb code in the context of these platforms becomes instantly more
hackish if you say that the address variable is not really the address we want
to read, but the double. There are enough things in as it is to scare new gdb
developers, we do not need any more :).
Another problem: the DWARF information describes the types using sizes in
target bytes (at least in our case, other implementations could do it
differently I suppose). The "char" type has a size of 1 (1 x 16-bits).
So, when you "print myvar", gdb would have to know that it needs to convert
the size to octets to request the right amount of memory.
I think the solution we propose is the one that models the best the debugged
system and therefore is the least hacky. From a gdb development point of view,
it makes things clear and explicit.
Does this answer you concerns?
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-09 15:39 ` Simon Marchi
@ 2015-04-09 16:29 ` Eli Zaretskii
2015-04-09 21:00 ` Simon Marchi
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2015-04-09 16:29 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> Date: Thu, 9 Apr 2015 11:39:06 -0400
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: <gdb-patches@sourceware.org>
>
> > I wonder: wouldn't it be possible to keep the current "byte == 8 bits"
> > notion, and instead to change the way addresses are interpreted by the
> > target back-end?
> >
> > IOW, do we really need to expose this issue all the way to the higher
> > levels of GDB application code?
>
> I don't think there is an elegant way of making this work without gdb
> knowing at least a bit about it. If you don't make some changes at one
> level, you'll end up needing to make the equivalent changes at some other
> level (still in gdb core).
I didn't mean to imply that this could work without changes on _some_
level. The question is what level, and whether or not we expose this
to the application level, where commands are interpreted.
> >From what I understand, your suggestion would be to treat addresses as
> indexes of octets in memory. So, to read target bytes at addresses 3
> and 4, I would have to ask gdb for 4 "gdb" bytes starting at address 6.
>
> size == 2
> v-------------------v
> +---------+---------+---------+---------+---------+---------+
> real idx | 0 | 1 | 2 | 3 | 4 | 5 |
> +----+----+----+----+----+----+----+----+----+----+----+----+
> octet idx | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 |
> +----+----+----+----+----+----+----+----+----+----+----+----+
> ^-------------------^
> size == 4
>
> The backend would then divide everything by two and read 2 target bytes
> starting at address 3.
Something like that, yes.
> If we require the user or the front-end to do that conversion, we just push
> the responsibility over the fence to them.
I don't follow: how does the above place any requirements on the user?
> For the developer working with that system 8 hours per day, a size
> of 1 is one 16-bits byte. His debugger should understand that
> language.
By "size" do you mean the result of "sizeof"? That could still
measure in target-side units, I see no contradiction there. I just
don't see why do we need to call that unit a "byte".
> If I have a pointer p (char *p) and I want to examine memory starting at p,
> I would do "x/10h p". That wouldn't give me what I want, as it would give me
> memory at p/2.
I don't see how it follows from my suggestion that 10 here must mean
80 bits. It could continue meaning 10 16-bit units.
> Also, the gdb code in the context of these platforms becomes instantly more
> hackish if you say that the address variable is not really the address we want
> to read, but the double.
I didn't say that, either.
> Another problem: the DWARF information describes the types using sizes in
> target bytes (at least in our case, other implementations could do it
> differently I suppose). The "char" type has a size of 1 (1 x 16-bits).
That's fine, just don't call that a "byte". Call it a "word".
> So, when you "print myvar", gdb would have to know that it needs to convert
> the size to octets to request the right amount of memory.
No, it won't. It sounds like my suggestion was totally misunderstood.
> I think the solution we propose is the one that models the best the debugged
> system and therefore is the least hacky.
My problem with your solution is that you require the user to change
her thinking about what a "byte" and "word" are. GDB is moving to
being able to debug several different targets at the same time, and I
worry about the user's sanity when one of those targets is of the kind
you are describing. E.g., suppose we will have a command to copy
memory from one target to another: how do we count the size of the
buffer then?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-09 16:29 ` Eli Zaretskii
@ 2015-04-09 21:00 ` Simon Marchi
2015-04-10 8:11 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2015-04-09 21:00 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 15-04-09 12:29 PM, Eli Zaretskii wrote:
>> Date: Thu, 9 Apr 2015 11:39:06 -0400
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> CC: <gdb-patches@sourceware.org>
>>
>>> I wonder: wouldn't it be possible to keep the current "byte == 8 bits"
>>> notion, and instead to change the way addresses are interpreted by the
>>> target back-end?
>>>
>>> IOW, do we really need to expose this issue all the way to the higher
>>> levels of GDB application code?
>>
>> I don't think there is an elegant way of making this work without gdb
>> knowing at least a bit about it. If you don't make some changes at one
>> level, you'll end up needing to make the equivalent changes at some other
>> level (still in gdb core).
>
> I didn't mean to imply that this could work without changes on _some_
> level. The question is what level, and whether or not we expose this
> to the application level, where commands are interpreted.
>
>> >From what I understand, your suggestion would be to treat addresses as
>> indexes of octets in memory. So, to read target bytes at addresses 3
>> and 4, I would have to ask gdb for 4 "gdb" bytes starting at address 6.
>>
>> size == 2
>> v-------------------v
>> +---------+---------+---------+---------+---------+---------+
>> real idx | 0 | 1 | 2 | 3 | 4 | 5 |
>> +----+----+----+----+----+----+----+----+----+----+----+----+
>> octet idx | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 |
>> +----+----+----+----+----+----+----+----+----+----+----+----+
>> ^-------------------^
>> size == 4
>>
>> The backend would then divide everything by two and read 2 target bytes
>> starting at address 3.
>
> Something like that, yes.
Ok.
>> If we require the user or the front-end to do that conversion, we just push
>> the responsibility over the fence to them.
>
> I don't follow: how does the above place any requirements on the user?
This was my train of thoughts:
- The gdb core (the target interface I suppose?) would use octet indexing
and octet size, which is compensated by the backend. I understand that
we are clear on that, given your "Something like that, yes".
- The functions handling the commands (the application level?) should be
agnostic about the byte size, meaning they won't do any adjustment for
that.
- Therefore, if we take mi_cmd_data_read_memory_bytes as an example, it
would mean that the front-end would have to pass the double of the
address and the size to get the desired result.
If you want the gdb core to keep using addressing and size in octets, the
conversion needs to be done somewhere, either in the head of the user, in
the front-end or in the command handling function, passing control to a
gdb core function.
>> For the developer working with that system 8 hours per day, a size
>> of 1 is one 16-bits byte. His debugger should understand that
>> language.
>
> By "size" do you mean the result of "sizeof"? That could still
> measure in target-side units, I see no contradiction there. I just
> don't see why do we need to call that unit a "byte".
>
>> If I have a pointer p (char *p) and I want to examine memory starting at p,
>> I would do "x/10h p". That wouldn't give me what I want, as it would give me
>> memory at p/2.
>
> I don't see how it follows from my suggestion that 10 here must mean
> 80 bits. It could continue meaning 10 16-bit units.
Sorry about that, I should have just used "x p". The /10h part was not part of
my point. Following my previous point where the user would have needed to specify
the double of the address, it would have meant that asking to read at address p
would have given memory starting at address p/2.
>> Also, the gdb code in the context of these platforms becomes instantly more
>> hackish if you say that the address variable is not really the address we want
>> to read, but the double.
>
> I didn't say that, either.
That's what I understood. If the backend needs to adjust the address by dividing it
by two, it means that the address parameter it received was the double of the actual
address...
>> Another problem: the DWARF information describes the types using sizes in
>> target bytes (at least in our case, other implementations could do it
>> differently I suppose). The "char" type has a size of 1 (1 x 16-bits).
>
> That's fine, just don't call that a "byte". Call it a "word".
I actually started by using word throughout the code, but then I found it even
more ambiguous than byte. In the context of the x command, a word is defined as
four bytes, so it still clashes.
>> So, when you "print myvar", gdb would have to know that it needs to convert
>> the size to octets to request the right amount of memory.
>
> No, it won't. It sounds like my suggestion was totally misunderstood.
Indeed, I think I missed your point. Re-reading the discussion doesn't help. Could
you clarify a bit how you envision things would work at various levels in gdb? If
we don't understand each other clearly, this discussion won't go anywhere useful.
>> I think the solution we propose is the one that models the best the debugged
>> system and therefore is the least hacky.
>
> My problem with your solution is that you require the user to change
> her thinking about what a "byte" and "word" are.
It doesn't change anything for all the existing users of GDB. A byte will continue
to be 8-bits for those platforms. So they don't need to change anything about how
they think.
I would assume that somebody developing for a system with 16-bits byte would be very
well aware of that fact. It is quite fundamental. They won't be shocked if the
debugger shows 16-bits when they asked to read 1 byte. Quite the opposite actually,
it will feel like a natural extension of the compiler.
> GDB is moving to
> being able to debug several different targets at the same time, and I
> worry about the user's sanity when one of those targets is of the kind
> you are describing. E.g., suppose we will have a command to copy
> memory from one target to another: how do we count the size of the
> buffer then?
About that particular example, I guess such a command would have no other choice but
to use the greatest common divisor of each memory unit, the octet. If there was
a command to copy data from one memory to another, this situation would arise with
the AVR ATMega chips, where SRAM data memory is 8-bits but the flash program memory is
16-bits.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-09 21:00 ` Simon Marchi
@ 2015-04-10 8:11 ` Eli Zaretskii
2015-04-10 16:01 ` Simon Marchi
0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2015-04-10 8:11 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> Date: Thu, 9 Apr 2015 17:00:45 -0400
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: <gdb-patches@sourceware.org>
>
> This was my train of thoughts:
>
> - The gdb core (the target interface I suppose?) would use octet indexing
> and octet size, which is compensated by the backend. I understand that
> we are clear on that, given your "Something like that, yes".
> - The functions handling the commands (the application level?) should be
> agnostic about the byte size, meaning they won't do any adjustment for
> that.
> - Therefore, if we take mi_cmd_data_read_memory_bytes as an example, it
> would mean that the front-end would have to pass the double of the
> address and the size to get the desired result.
>
> If you want the gdb core to keep using addressing and size in octets, the
> conversion needs to be done somewhere, either in the head of the user, in
> the front-end or in the command handling function, passing control to a
> gdb core function.
I want GDB to be agnostic, as far as possible, to the size of 1 unit
of memory. Ideally, one unit will start as one unit in user-level
commands, pass all the way down to the target level, which should know
what one unit means. In the cases where that ideal is unreachable,
there should be two conversions: once from user-level commands to
bytes, and then one other time from bytes back to target-level units.
> Sorry about that, I should have just used "x p". The /10h part was not part of
> my point. Following my previous point where the user would have needed to specify
> the double of the address, it would have meant that asking to read at address p
> would have given memory starting at address p/2.
No, the addresses don't need to change at all. Why would they?
> >> Also, the gdb code in the context of these platforms becomes instantly more
> >> hackish if you say that the address variable is not really the address we want
> >> to read, but the double.
> >
> > I didn't say that, either.
>
> That's what I understood. If the backend needs to adjust the address by dividing it
> by two, it means that the address parameter it received was the double of the actual
> address...
No, see above.
> >> Another problem: the DWARF information describes the types using sizes in
> >> target bytes (at least in our case, other implementations could do it
> >> differently I suppose). The "char" type has a size of 1 (1 x 16-bits).
> >
> > That's fine, just don't call that a "byte". Call it a "word".
>
> I actually started by using word throughout the code, but then I found it even
> more ambiguous than byte. In the context of the x command, a word is defined as
> four bytes, so it still clashes.
OK, "half-word", then. (And please note that AFAIR there are
architectures where a "byte" is 32 bit wide, so there "word" would be
accurate.)
> >> So, when you "print myvar", gdb would have to know that it needs to convert
> >> the size to octets to request the right amount of memory.
> >
> > No, it won't. It sounds like my suggestion was totally misunderstood.
>
> Indeed, I think I missed your point. Re-reading the discussion doesn't help. Could
> you clarify a bit how you envision things would work at various levels in gdb?
I tried to do that above, let me know if something is still unclear.
> > My problem with your solution is that you require the user to change
> > her thinking about what a "byte" and "word" are.
>
> It doesn't change anything for all the existing users of GDB. A byte will continue
> to be 8-bits for those platforms. So they don't need to change anything about how
> they think.
I would like to find a solution where a byte continues to be 8 bits on
_all_ platforms.
> I would assume that somebody developing for a system with 16-bits byte would be very
> well aware of that fact. It is quite fundamental. They won't be shocked if the
> debugger shows 16-bits when they asked to read 1 byte. Quite the opposite actually,
> it will feel like a natural extension of the compiler.
What I have before my eyes is a user who debugs several different
platforms, and therefore doesn't immerse themselves in this world of
different meanings for too long times.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-10 8:11 ` Eli Zaretskii
@ 2015-04-10 16:01 ` Simon Marchi
2015-04-10 16:35 ` Pedro Alves
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-10 16:01 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
> I want GDB to be agnostic, as far as possible, to the size of 1 unit
> of memory. Ideally, one unit will start as one unit in user-level
> commands, pass all the way down to the target level, which should know
> what one unit means.
I totally agree with you, and I believe that's the idea you'll find implemented
in the patches. The length is always passed in "units of memory" of whatever you
are trying to read or write. The only thing is that I called a "unit of memory"
a "byte", which seems the friction point. If it's just a wording issue, it can
be changed easily. I just don't know what succinct term to use.
> In the cases where that ideal is unreachable,
> there should be two conversions: once from user-level commands to
> bytes, and then one other time from bytes back to target-level units.
In the memory read/write call chains, the only times where we need to know the
size in octets of the memory unit is when gdb is actually handling the data read
or to be written. We need to know how many bytes (as in octets) to malloc, memcpy
or hex-encode/decode.
>> Sorry about that, I should have just used "x p". The /10h part was not part of
>> my point. Following my previous point where the user would have needed to specify
>> the double of the address, it would have meant that asking to read at address p
>> would have given memory starting at address p/2.
>
> No, the addresses don't need to change at all. Why would they?
That's what I understood from you "and instead to change the way addresses are
interpreted by the target back-end", and it was further confirmed when you replied
"Something like that, yes" to the example I gave, in which the address is doubled.
>>>> Also, the gdb code in the context of these platforms becomes instantly more
>>>> hackish if you say that the address variable is not really the address we want
>>>> to read, but the double.
>>>
>>> I didn't say that, either.
>>
>> That's what I understood. If the backend needs to adjust the address by dividing it
>> by two, it means that the address parameter it received was the double of the actual
>> address...
>
> No, see above.
Idem.
>>>> Another problem: the DWARF information describes the types using sizes in
>>>> target bytes (at least in our case, other implementations could do it
>>>> differently I suppose). The "char" type has a size of 1 (1 x 16-bits).
>>>
>>> That's fine, just don't call that a "byte". Call it a "word".
>>
>> I actually started by using word throughout the code, but then I found it even
>> more ambiguous than byte. In the context of the x command, a word is defined as
>> four bytes, so it still clashes.
>
> OK, "half-word", then. (And please note that AFAIR there are
> architectures where a "byte" is 32 bit wide, so there "word" would be
> accurate.)
The term we are looking for is one for a single memory unit, regardless of its size.
>>>> So, when you "print myvar", gdb would have to know that it needs to convert
>>>> the size to octets to request the right amount of memory.
>>>
>>> No, it won't. It sounds like my suggestion was totally misunderstood.
>>
>> Indeed, I think I missed your point. Re-reading the discussion doesn't help. Could
>> you clarify a bit how you envision things would work at various levels in gdb?
>
> I tried to do that above, let me know if something is still unclear.
>
>>> My problem with your solution is that you require the user to change
>>> her thinking about what a "byte" and "word" are.
>>
>> It doesn't change anything for all the existing users of GDB. A byte will continue
>> to be 8-bits for those platforms. So they don't need to change anything about how
>> they think.
>
> I would like to find a solution where a byte continues to be 8 bits on
> _all_ platforms.
Ok, so if I understand correctly, you would be fine if the -data-read-memory-bytes
command accepted a length in number of memory units, as long as this unit is not
called a byte. Is that right? If so, it would confirm that it's a wording issue
more than a technical one.
>> I would assume that somebody developing for a system with 16-bits byte would be very
>> well aware of that fact. It is quite fundamental. They won't be shocked if the
>> debugger shows 16-bits when they asked to read 1 byte. Quite the opposite actually,
>> it will feel like a natural extension of the compiler.
>
> What I have before my eyes is a user who debugs several different
> platforms, and therefore doesn't immerse themselves in this world of
> different meanings for too long times.
I understand your concern. The term "byte" is probably set in stone as 8-bits for pretty
much everybody, so trying to redefine it as variable length would probably cause more harm
than good.
Thanks a lot for your patience.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-10 16:01 ` Simon Marchi
@ 2015-04-10 16:35 ` Pedro Alves
2015-04-10 16:39 ` Paul_Koning
2015-04-10 17:42 ` Eli Zaretskii
2 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2015-04-10 16:35 UTC (permalink / raw)
To: Simon Marchi, Eli Zaretskii; +Cc: gdb-patches
On 04/10/2015 05:01 PM, Simon Marchi wrote:
> The term we are looking for is one for a single memory unit, regardless of its size.
That suggests "addressable memory unit".
Looks like http://en.wikipedia.org/wiki/Word_%28computer_architecture%29 calls
it "Unit of address resolution".
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-10 16:01 ` Simon Marchi
2015-04-10 16:35 ` Pedro Alves
@ 2015-04-10 16:39 ` Paul_Koning
2015-04-10 17:34 ` Simon Marchi
2015-04-10 17:42 ` Eli Zaretskii
2 siblings, 1 reply; 21+ messages in thread
From: Paul_Koning @ 2015-04-10 16:39 UTC (permalink / raw)
To: simon.marchi; +Cc: eliz, gdb-patches
> ...
>>> I would assume that somebody developing for a system with 16-bits byte would be very
>>> well aware of that fact. It is quite fundamental. They won't be shocked if the
>>> debugger shows 16-bits when they asked to read 1 byte. Quite the opposite actually,
>>> it will feel like a natural extension of the compiler.
>>
>> What I have before my eyes is a user who debugs several different
>> platforms, and therefore doesn't immerse themselves in this world of
>> different meanings for too long times.
>
> I understand your concern. The term "byte" is probably set in stone as 8-bits for pretty
> much everybody, so trying to redefine it as variable length would probably cause more harm
> than good.
But if you work on several platforms, you would reasonably be expected to understand each of them. Debugging on A, B, and C with a working knowledge only of A isn’t very realistic.
8 bits == byte is certainly the predominant modern usage, but it hasn’t always been that way and even today it apparently isn’t quite universal. GCC used to support architectures that did not do 8-bit byte memory addressing, though that capability seems to have faded away in the past couple of years.
paul
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-10 16:39 ` Paul_Koning
@ 2015-04-10 17:34 ` Simon Marchi
0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2015-04-10 17:34 UTC (permalink / raw)
To: Paul_Koning; +Cc: eliz, gdb-patches
>> I understand your concern. The term "byte" is probably set in stone as 8-bits for pretty
>> much everybody, so trying to redefine it as variable length would probably cause more harm
>> than good.
>
> But if you work on several platforms, you would reasonably be expected to understand each of them. Debugging on A, B, and C with a working knowledge only of A isn’t very realistic.
>
> 8 bits == byte is certainly the predominant modern usage, but it hasn’t always been that way and even today it apparently isn’t quite universal. GCC used to support architectures that did not do 8-bit byte memory addressing, though that capability seems to have faded away in the past couple of years.
I guess you are referring to older architectures with 6, 7, 9, 12, 36-bits or other
byte size. I'd like to make it clear that our goal is not to support that kind of
bytes that is not a multiple of 8 bits. That would be incredibly more complicated.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-10 16:01 ` Simon Marchi
2015-04-10 16:35 ` Pedro Alves
2015-04-10 16:39 ` Paul_Koning
@ 2015-04-10 17:42 ` Eli Zaretskii
2015-04-10 18:01 ` Simon Marchi
2 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2015-04-10 17:42 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> Date: Fri, 10 Apr 2015 12:01:00 -0400
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: <gdb-patches@sourceware.org>
>
> > I want GDB to be agnostic, as far as possible, to the size of 1 unit
> > of memory. Ideally, one unit will start as one unit in user-level
> > commands, pass all the way down to the target level, which should know
> > what one unit means.
>
> I totally agree with you, and I believe that's the idea you'll find implemented
> in the patches. The length is always passed in "units of memory" of whatever you
> are trying to read or write. The only thing is that I called a "unit of memory"
> a "byte", which seems the friction point. If it's just a wording issue, it can
> be changed easily. I just don't know what succinct term to use.
W could use the terminology that is already in use: "half-words" for
16 bits and "words" for 32 bits. Would that be OK?
> Ok, so if I understand correctly, you would be fine if the -data-read-memory-bytes
> command accepted a length in number of memory units, as long as this unit is not
> called a byte.
Yes. Though it's unfortunate that the name of the command explicitly
mentions "bytes".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-10 17:42 ` Eli Zaretskii
@ 2015-04-10 18:01 ` Simon Marchi
2015-04-10 18:16 ` Eli Zaretskii
0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2015-04-10 18:01 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 15-04-10 01:42 PM, Eli Zaretskii wrote:
>> Date: Fri, 10 Apr 2015 12:01:00 -0400
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> CC: <gdb-patches@sourceware.org>
>>
>>> I want GDB to be agnostic, as far as possible, to the size of 1 unit
>>> of memory. Ideally, one unit will start as one unit in user-level
>>> commands, pass all the way down to the target level, which should know
>>> what one unit means.
>>
>> I totally agree with you, and I believe that's the idea you'll find implemented
>> in the patches. The length is always passed in "units of memory" of whatever you
>> are trying to read or write. The only thing is that I called a "unit of memory"
>> a "byte", which seems the friction point. If it's just a wording issue, it can
>> be changed easily. I just don't know what succinct term to use.
>
> W could use the terminology that is already in use: "half-words" for
> 16 bits and "words" for 32 bits. Would that be OK?
When you know what is the size of the data unit you are referring to that's fine. But
we need another word or expression for when we don't know it. For example, in the
-data-read-memory-bytes documentation, we would need to change:
‘count’
The number of bytes to read. This should be an integer literal.
to something like
‘count’
The number of addressable memory units to read. This should be an integer literal.
We can't use "byte", "half-words" or "words" here, since the doc should work for all the
platforms. We could also use "addressable memory unit" or "unit of address resolution",
it just seems a bit verbose to me.
>> Ok, so if I understand correctly, you would be fine if the -data-read-memory-bytes
>> command accepted a length in number of memory units, as long as this unit is not
>> called a byte.
>
> Yes. Though it's unfortunate that the name of the command explicitly
> mentions "bytes".
There is also the parameter named byte-offset. Although I guess we could change it since
it's not really referred by name in the code, just the position.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes
2015-04-10 18:01 ` Simon Marchi
@ 2015-04-10 18:16 ` Eli Zaretskii
0 siblings, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2015-04-10 18:16 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> Date: Fri, 10 Apr 2015 14:01:40 -0400
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: <gdb-patches@sourceware.org>
>
> > We could use the terminology that is already in use: "half-words" for
> > 16 bits and "words" for 32 bits. Would that be OK?
>
> When you know what is the size of the data unit you are referring to that's fine. But
> we need another word or expression for when we don't know it.
"Addressable memory unit" sounds good to me, provided that we explain
somewhere what that means.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2015-04-10 18:16 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-08 19:56 [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Simon Marchi
2015-04-08 19:56 ` [PATCH 1/7] Various cleanups in target read/write code Simon Marchi
2015-04-08 19:56 ` [PATCH 7/7] MI: Consider byte size when reading/writing memory Simon Marchi
2015-04-08 19:56 ` [PATCH 3/7] Clarify doc about memory read/write and non-8-bits bytes Simon Marchi
2015-04-09 7:02 ` Eli Zaretskii
2015-04-08 19:56 ` [PATCH 5/7] target: consider byte size when reading/writing memory Simon Marchi
2015-04-08 19:56 ` [PATCH 4/7] gdbarch: add memory_byte_size method Simon Marchi
2015-04-08 19:56 ` [PATCH 2/7] Cleanup some docs about memory write Simon Marchi
2015-04-08 19:56 ` [PATCH 6/7] remote: Consider byte size when reading/writing memory Simon Marchi
2015-04-09 8:20 ` [PATCH 0/7] Support reading/writing memory on architectures with non 8-bits bytes Eli Zaretskii
2015-04-09 15:39 ` Simon Marchi
2015-04-09 16:29 ` Eli Zaretskii
2015-04-09 21:00 ` Simon Marchi
2015-04-10 8:11 ` Eli Zaretskii
2015-04-10 16:01 ` Simon Marchi
2015-04-10 16:35 ` Pedro Alves
2015-04-10 16:39 ` Paul_Koning
2015-04-10 17:34 ` Simon Marchi
2015-04-10 17:42 ` Eli Zaretskii
2015-04-10 18:01 ` Simon Marchi
2015-04-10 18:16 ` Eli Zaretskii
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox