* Better MI memory commands
@ 2010-06-25 8:33 Vladimir Prus
2010-06-25 16:05 ` Eli Zaretskii
2010-07-07 16:30 ` Daniel Jacobowitz
0 siblings, 2 replies; 12+ messages in thread
From: Vladimir Prus @ 2010-06-25 8:33 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: Text/Plain, Size: 1311 bytes --]
The attached patch makes a few changes in MI memory commands, with the
goal of making it easy for frontend to just display memory view, even
around the boundaries of accessible memory. The changes are as follows:
- The code that reads memory is modified read around inaccessible memory
regions as defined by memory map, as opposed to giving error if the first
address to read happens to be inaccessible.
- If we think the memory should be readable, but get an error, we now use
binary search to find subrange at the beginning that is still readable.
With this patch, we do the same for the end. So, if the start address is not
readable but end address is, the readable tail will be returned.
- The current output of -data-read-memory is totally insane. This patch
introduces -data-read-memory-raw that clearly reports what regions were
read, and shows the data as hex string. I've also added -data-write-memory-raw
which, contrary to -data-write-memory can actually write *memory*, as opposed to
a single byte.
I've tested this against a certain bare-metal board, first with memory map and
second with manually cleared memory map in GDB. In both cases, reads before
and after accessible memory worked reasonably.
OK?
Thanks,
--
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722
[-- Attachment #2: mi-memory.diff --]
[-- Type: text/x-patch, Size: 18873 bytes --]
commit 411034a96240a40e3293f05c5e223944177ac7b3
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu Jun 17 20:23:17 2010 +0400
Easier and more stubborn MI memory read commands.
gdb/
* mi/mi-cmds.c (mi_cmds): Register data-read-memory-raw
and data-write-memory-raw.
* mi/mi-cmds.h (mi_cmd_data_read_memory_raw)
(mi_cmd_data_write_memory_raw): New.
* mi/mi-main.c (mi_cmd_data_read_memory): Use regular target_read.
(mi_cmd_data_read_memory_raw, mi_cmd_data_write_memory_raw): New.
* target.c (target_read_until_error): Remove.
(read_whatever_is_readable, free_memory_read_result_vector)
(read_memory_robust): New.
* target.h (target_read_until_error): Remove.
(struct memory_read_result, free_memory_read_result_vector)
(read_memory_robust): New.
gdb/doc
* gdb.texinfo (GDB/MI Data Manipulation): Document
-data-read-memory-raw and -data-write-memory-raw.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index cbd636f..e1da81d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26895,6 +26895,8 @@ don't appear in the actual output):
@subheading The @code{-data-read-memory} Command
@findex -data-read-memory
+This command is deprecated, use @code{-data-read-memory-raw} instead.
+
@subsubheading Synopsis
@smallexample
@@ -27006,6 +27008,120 @@ next-page="0x000013c0",prev-page="0x00001380",memory=[
(gdb)
@end smallexample
+@subheading The @code{-data-read-memory-raw} Command
+@findex -data-read-memory-raw
+
+@subsubheading Synopsis
+
+@smallexample
+ -data-read-memory-raw [ -o @var{byte-offset} ]
+ @var{address} @var{count}
+@end smallexample
+
+@noindent
+where:
+
+@table @samp
+@item @var{address}
+An expression specifying the address of the first memory word 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.
+
+@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.
+
+@end table
+
+The commands attempts to read memory at the specified address. When a
+memory map is available, regions marked as unreadable in the memory
+map will be skipped. In addition, when @value{GDBN} encouters an error
+reading a memory range, it will attempt to find readable subrange at
+the beginning and range. Essentially, this command will make
+reasonable effort to return all readable memory content within the
+requested range.
+
+The output of the command has a result record named @samp{memory},
+whose content is a list of tuples. Each tuple represent a
+successfully read memory block and has the following
+fields:
+
+@table @code
+@item begin
+The start address of the memory block, as hexadecimal literal.
+
+@item end
+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-raw}.
+
+@item contents
+The contents of the memory block, as hex-encoded string of bytes.
+
+@end table
+
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{x}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-data-read-memory-raw &a 10
+^done,memory=[@{begin="0xbffff154",offset="0x00000000",
+ end="0xbffff15e",
+ contents="01000000020000000300"@}]
+(gdb)
+@end smallexample
+
+
+@subheading The @code{-data-write-memory-raw} Command
+@findex -data-write-memory-raw
+
+@subsubheading Synopsis
+
+@smallexample
+ -data-write-memory-raw @var{address} @var{contents}
+@end smallexample
+
+@noindent
+where:
+
+@table @samp
+@item @var{address}
+An expression specifying the address of the first memory word to be
+read. Complex expressions containing embedded white space should be
+quoted using the C convention.
+
+@item @var{contents}
+The hex-encoded bytes to write. The size of this parameter determines
+how many bytes should be written.
+
+@end table
+
+@subsubheading @value{GDBN} Command
+
+There's no corresponding @value{GDBN} command.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-data-write-memory-raw &a "aabbccdd"
+^done
+(gdb)
+@end smallexample
+
+
@c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@node GDB/MI Tracepoint Commands
@section @sc{gdb/mi} Tracepoint Commands
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 8441e17..65fe0ae 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -51,7 +51,9 @@ struct mi_cmd mi_cmds[] =
{ "data-list-register-names", { NULL, 0 }, mi_cmd_data_list_register_names},
{ "data-list-register-values", { NULL, 0 }, mi_cmd_data_list_register_values},
{ "data-read-memory", { NULL, 0 }, mi_cmd_data_read_memory},
+ { "data-read-memory-raw", { NULL, 0 }, mi_cmd_data_read_memory_raw},
{ "data-write-memory", { NULL, 0 }, mi_cmd_data_write_memory},
+ { "data-write-memory-raw", {NULL, 0}, mi_cmd_data_write_memory_raw},
{ "data-write-register-values", { NULL, 0 }, mi_cmd_data_write_register_values},
{ "enable-timings", { NULL, 0 }, mi_cmd_enable_timings},
{ "enable-pretty-printing", { NULL, 0 }, mi_cmd_enable_pretty_printing},
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 5954aef..90a59c7 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -47,7 +47,9 @@ extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;
extern mi_cmd_argv_ftype mi_cmd_data_list_register_values;
extern mi_cmd_argv_ftype mi_cmd_data_list_changed_registers;
extern mi_cmd_argv_ftype mi_cmd_data_read_memory;
+extern mi_cmd_argv_ftype mi_cmd_data_read_memory_raw;
extern mi_cmd_argv_ftype mi_cmd_data_write_memory;
+extern mi_cmd_argv_ftype mi_cmd_data_write_memory_raw;
extern mi_cmd_argv_ftype mi_cmd_data_write_register_values;
extern mi_cmd_argv_ftype mi_cmd_enable_timings;
extern mi_cmd_argv_ftype mi_cmd_env_cd;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 85a3f99..a99a91e 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1352,9 +1352,9 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
/* Dispatch memory reads to the topmost target, not the flattened
current_target. */
- nr_bytes = target_read_until_error (current_target.beneath,
- TARGET_OBJECT_MEMORY, NULL, mbuf,
- addr, total_bytes);
+ nr_bytes = target_read (current_target.beneath,
+ TARGET_OBJECT_MEMORY, NULL, mbuf,
+ addr, total_bytes);
if (nr_bytes <= 0)
error ("Unable to read memory.");
@@ -1437,6 +1437,88 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
do_cleanups (cleanups);
}
+void
+mi_cmd_data_read_memory_raw (char *command, char **argv, int argc)
+{
+ struct gdbarch *gdbarch = get_current_arch ();
+ struct cleanup *cleanups;
+ CORE_ADDR addr;
+ LONGEST length;
+ memory_read_result_s *read_result;
+ int ix;
+ VEC(memory_read_result_s) *result;
+ long offset = 0;
+ int optind = 0;
+ char *optarg;
+ enum opt
+ {
+ OFFSET_OPT
+ };
+ static struct mi_opt opts[] =
+ {
+ {"o", OFFSET_OPT, 1},
+ { 0, 0, 0 }
+ };
+
+ while (1)
+ {
+ int opt = mi_getopt ("mi_cmd_data_read_memory_raw", argc, argv, opts,
+ &optind, &optarg);
+ if (opt < 0)
+ break;
+ switch ((enum opt) opt)
+ {
+ case OFFSET_OPT:
+ offset = atol (optarg);
+ break;
+ }
+ }
+ argv += optind;
+ argc -= optind;
+
+ if (argc != 2)
+ error ("Usage: ADDR LENGTH.");
+
+ addr = parse_and_eval_address (argv[0]) + offset;
+ length = atol (argv[1]);
+
+ result = read_memory_robust (current_target.beneath, addr, length);
+
+ cleanups = make_cleanup (free_memory_read_result_vector, result);
+
+ if (VEC_length (memory_read_result_s, result) == 0)
+ error ("Unable to read memory.");
+
+ make_cleanup_ui_out_list_begin_end (uiout, "memory");
+ for (ix = 0;
+ VEC_iterate (memory_read_result_s, result, ix, read_result);
+ ++ix)
+ {
+ struct cleanup *t = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+ char *data, *p;
+ int i;
+
+ ui_out_field_core_addr (uiout, "begin", gdbarch, read_result->begin);
+ ui_out_field_core_addr (uiout, "offset", gdbarch, read_result->begin
+ - addr);
+ ui_out_field_core_addr (uiout, "end", gdbarch, read_result->end);
+
+ data = xmalloc ((read_result->end - read_result->begin) * 2 + 1);
+
+ for (i = 0, p = data;
+ i < (read_result->end - read_result->begin);
+ ++i, p += 2)
+ {
+ sprintf (p, "%02x", read_result->data[i]);
+ }
+ ui_out_field_string (uiout, "contents", data);
+ xfree (data);
+ do_cleanups (t);
+ }
+ do_cleanups (cleanups);
+}
+
+
/* DATA-MEMORY-WRITE:
COLUMN_OFFSET: optional argument. Must be preceeded by '-o'. The
@@ -1523,6 +1605,44 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
do_cleanups (old_chain);
}
+/* DATA-MEMORY-WRITE-RAW:
+
+ ADDR: start address
+ DATA: string of bytes to write at that address. */
+void
+mi_cmd_data_write_memory_raw (char *command, char **argv, int argc)
+{
+ CORE_ADDR addr;
+ char *cdata;
+ gdb_byte *data;
+ int len, r, i;
+ struct cleanup *back_to;
+
+ if (argc != 2)
+ error ("Usage: ADDR DATA.");
+
+ addr = parse_and_eval_address (argv[0]);
+ cdata = argv[1];
+ len = strlen (cdata)/2;
+
+ data = xmalloc (len);
+ back_to = make_cleanup (xfree, data);
+
+ for (i = 0; i < len; ++i)
+ {
+ int x;
+ sscanf (cdata + i * 2, "%02x", &x);
+ data[i] = (gdb_byte)x;
+ }
+
+ r = target_write_memory (addr, data, len);
+ if (r != 0)
+ error (_("Could not write memory"));
+
+ do_cleanups (back_to);
+}
+
+
void
mi_cmd_enable_timings (char *command, char **argv, int argc)
{
diff --git a/gdb/target.c b/gdb/target.c
index 7aa77e6..f48ecdc 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1751,73 +1751,203 @@ target_read (struct target_ops *ops,
return len;
}
-LONGEST
-target_read_until_error (struct target_ops *ops,
- enum target_object object,
- const char *annex, gdb_byte *buf,
- ULONGEST offset, LONGEST len)
+/** Assuming that the entire [begin, end) range of memory cannot be read,
+ try to read whatever subrange is possible to read.
+
+ The function results, in RESULT, either zero or one memory block.
+ If there's a readable subrange at the beginning, it is completely
+ read and returned. Any further readable subrange will not be read.
+ Otherwise, if there's a readable subrange at the end, it will be
+ completely and returned. Any readable subranges before it (obviously,
+ not starting at the beginning), will be ignored. In other cases --
+ either no readable subrange, or readable subrange (s) that is neither
+ at the beginning, or end, nothing is returned.
+
+ The purpose of this function is to handle a read across a boundary of
+ accessible memory in a case when memory map is not available. The above
+ restrictions are fine for this case, but will give incorrect results if
+ the memory is 'patchy'. However, supporting 'patchy' memory would require
+ trying to read every single byte, and it seems unacceptable solution.
+ Explicit memory map is recommended for this case -- and
+ target_read_memory_robust will take care of reading multiple ranges then. */
+
+static void
+read_whatever_is_readable (struct target_ops *ops, ULONGEST begin, ULONGEST end,
+ VEC(memory_read_result_s) **result)
{
- LONGEST xfered = 0;
+ gdb_byte *buf = xmalloc (end-begin);
+ ULONGEST current_begin = begin;
+ ULONGEST current_end = end;
+ int forward;
+ memory_read_result_s r;
+
+ /* If we previously failed to read 1 byte, nothing can be done here. */
+ if (end - begin <= 1)
+ return;
+ /* Check that either first or the last byte is readable, and give up
+ if not. This heuristic is meant to permit reading accessible memory
+ at the boundary of accessible region. */
+ if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf, begin, 1) == 1)
+ {
+ forward = 1;
+ ++current_begin;
+ }
+ else if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf + (end-begin) - 1, end - 1, 1) == 1)
+ {
+ forward = 0;
+ --current_end;
+ }
+ else
+ {
+ return;
+ }
+
+ /* Loop invariant is that the [current_begin, current_end) was previously
+ found to be not readable as a whole.
+
+ Note loop condition -- if the range has 1 byte, we can't divide the range
+ so there's no point trying further. */
+ while (current_end - current_begin > 1)
+ {
+ 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;
+ if (forward)
+ {
+ first_half_begin = current_begin;
+ first_half_end = middle;
+ second_half_begin = middle;
+ second_half_end = current_end;
+ }
+ else
+ {
+ first_half_begin = middle;
+ first_half_end = current_end;
+ second_half_begin = current_begin;
+ second_half_end = middle;
+ }
+
+ xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf + (first_half_begin - begin),
+ first_half_begin,
+ first_half_end - first_half_begin);
+
+ if (xfer == first_half_end - first_half_begin)
+ {
+ /* This half reads up fine. So, the error must be in the other half. */
+ current_begin = second_half_begin;
+ current_end = second_half_end;
+ }
+ else
+ {
+ current_begin = first_half_begin;
+ current_end = first_half_end;
+ }
+ }
+
+ if (forward)
+ {
+ /* The [begin, current_begin) range has been read. */
+ r.begin = begin;
+ r.end = current_begin;
+ r.data = buf;
+ }
+ else
+ {
+ /* The [current_end, end) range has been read. */
+ LONGEST rlen = end - current_end;
+ r.data = xmalloc (rlen);
+ memcpy (r.data, buf + current_end - begin, rlen);
+ r.begin = current_end;
+ r.end = end;
+ xfree (buf);
+ }
+ VEC_safe_push(memory_read_result_s, (*result), &r);
+}
+
+void
+free_memory_read_result_vector (void *x)
+{
+ VEC(memory_read_result_s) *v = x;
+ memory_read_result_s *current;
+ int ix;
+
+ for (ix = 0; VEC_iterate (memory_read_result_s, v, ix, current); ++ix)
+ {
+ xfree (current->data);
+ }
+ VEC_free (memory_read_result_s, v);
+}
+
+VEC(memory_read_result_s) *
+read_memory_robust (struct target_ops *ops, ULONGEST offset, LONGEST len)
+{
+ VEC(memory_read_result_s) *result = 0;
+
+ LONGEST xfered = 0;
while (xfered < len)
{
- LONGEST xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- offset + xfered, len - xfered);
+ struct mem_region *region = lookup_mem_region (offset + xfered);
+ LONGEST rlen;
- /* Call an observer, notifying them of the xfer progress? */
- if (xfer == 0)
- return xfered;
- if (xfer < 0)
+ /* If there is no explicit region, a fake one should be created. */
+ gdb_assert (region);
+
+ if (region->hi == 0)
+ rlen = len - xfered;
+ else
+ rlen = region->hi - offset;
+
+ if (region->attrib.mode == MEM_NONE || region->attrib.mode == MEM_WO)
{
- /* We've got an error. Try to read in smaller blocks. */
- ULONGEST start = offset + xfered;
- ULONGEST remaining = len - xfered;
- ULONGEST half;
-
- /* If an attempt was made to read a random memory address,
- it's likely that the very first byte is not accessible.
- Try reading the first byte, to avoid doing log N tries
- below. */
- xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered, start, 1);
- if (xfer <= 0)
- return xfered;
- start += 1;
- remaining -= 1;
- half = remaining/2;
-
- while (half > 0)
+ /* 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;
+ }
+ else
+ {
+ LONGEST to_read = min (len - xfered, rlen);
+ gdb_byte *buffer = (gdb_byte *)xmalloc (to_read);
+
+ LONGEST xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ (gdb_byte *) buffer,
+ offset + xfered, to_read);
+ /* Call an observer, notifying them of the xfer progress? */
+ if (xfer == 0)
{
- xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- start, half);
- if (xfer == 0)
- return xfered;
- if (xfer < 0)
- {
- remaining = half;
- }
- else
- {
- /* We have successfully read the first half. So, the
- error must be in the second half. Adjust start and
- remaining to point at the second half. */
- xfered += xfer;
- start += xfer;
- remaining -= xfer;
- }
- half = remaining/2;
+ xfree (buffer);
+ xfered += to_read;
}
-
- return xfered;
+ else if (xfer < 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;
+ }
+ else
+ {
+ struct memory_read_result r;
+ r.data = buffer;
+ r.begin = offset + xfered;
+ r.end = r.begin + xfer;
+ VEC_safe_push (memory_read_result_s, result, &r);
+ xfered += xfer;
+ }
+ QUIT;
}
- xfered += xfer;
- QUIT;
}
- return len;
+ return result;
}
+
/* An alternative to target_write with progress callbacks. */
LONGEST
diff --git a/gdb/target.h b/gdb/target.h
index 870a1eb..66e1166 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -292,10 +292,22 @@ extern LONGEST target_read (struct target_ops *ops,
const char *annex, gdb_byte *buf,
ULONGEST offset, LONGEST len);
-extern LONGEST target_read_until_error (struct target_ops *ops,
- enum target_object object,
- const char *annex, gdb_byte *buf,
- ULONGEST offset, LONGEST len);
+struct memory_read_result
+ {
+ /* First address that was read. */
+ ULONGEST begin;
+ /* Past-the-end address. */
+ ULONGEST end;
+ /* The data. */
+ gdb_byte *data;
+};
+typedef struct memory_read_result memory_read_result_s;
+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);
extern LONGEST target_write (struct target_ops *ops,
enum target_object object,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-06-25 8:33 Better MI memory commands Vladimir Prus
@ 2010-06-25 16:05 ` Eli Zaretskii
2010-08-11 12:38 ` Vladimir Prus
2010-07-07 16:30 ` Daniel Jacobowitz
1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2010-06-25 16:05 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Fri, 25 Jun 2010 12:32:55 +0400
>
> The attached patch makes a few changes in MI memory commands, with the
> goal of making it easy for frontend to just display memory view, even
> around the boundaries of accessible memory.
Thanks. I have a few comments about the documentation part of the
patch.
> +@item @var{count}
> +The number of bytes to read. This should be an integer literal.
^^
Two spaces here, please.
> +The commands attempts to read memory at the specified address.
^^^^^^^^^^^^
"This command"
> + When a
> +memory map is available
A cross-reference here to where memory maps are described would be
useful.
> + regions marked as unreadable in the memory
> +map will be skipped. In addition, when @value{GDBN} encouters an error
^^
Two spaces.
> +reading a memory range, it will attempt to find readable subrange at
^^^^^^^^^^^^^^^^^^
"a readable subrange"
> +the beginning and range. Essentially, this command will make
^^ ^^
Something ("end of the"?) is missing here. And two spaces between
sentences.
I'm not sure I completely understand this part:
In addition, when @value{GDBN} encouters an error reading a memory
range, it will attempt to find readable subrange [...]
What if there are two or more non-contiguous sub-ranges at the
beginning -- will GDB find and read both of them? More generally,
what happens if the specified range has several disjoint portions that
are not readable?
I'm asking because your description seems to imply that we only look
for the first readable address after start and the last readable
address before the end of the specified region. IOW, it sounds like
unreadable sub-regions in the middle of the region are not supported.
If my understanding is correct, then the following sentence
> + Essentially, this command will make
> +reasonable effort to return all readable memory content within the
> +requested range.
is at least inaccurate, if not misleading ("all readable memory within
the range").
> +The output of the command has a result record named @samp{memory},
^^^^^^^^^^^^^^^^^^^
Perhaps "is a result record"? And what is the importance of naming
this record `memory'? why is the name important here?
> +@item contents
> +The contents of the memory block, as hex-encoded string of bytes.
But your example shows this:
> + contents="01000000020000000300"@}]
This doesn't look like a ``string of bytes'' to me. Or maybe I don't
understand what you meant by that?
> +@item @var{contents}
> +The hex-encoded bytes to write. The size of this parameter determines
> +how many bytes should be written.^^^^^^^^
You probably meant "the value", not "the size".
> + Otherwise, if there's a readable subrange at the end, it will be
> + completely and returned. Any readable subranges before it (obviously,
^^
"read" is missing here.
> + The purpose of this function is to handle a read across a boundary of
> + accessible memory in a case when memory map is not available. The above
> + restrictions are fine for this case, but will give incorrect results if
> + the memory is 'patchy'. However, supporting 'patchy' memory would require
> + trying to read every single byte, and it seems unacceptable solution.
> + Explicit memory map is recommended for this case -- and
> + target_read_memory_robust will take care of reading multiple ranges then.
At least some of this comment should IMO be in the manual.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-06-25 8:33 Better MI memory commands Vladimir Prus
2010-06-25 16:05 ` Eli Zaretskii
@ 2010-07-07 16:30 ` Daniel Jacobowitz
2010-07-09 18:54 ` Vladimir Prus
1 sibling, 1 reply; 12+ messages in thread
From: Daniel Jacobowitz @ 2010-07-07 16:30 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
I have no further comments on the documentation, but Eli had several.
On Fri, Jun 25, 2010 at 12:32:55PM +0400, Vladimir Prus wrote:
> - The current output of -data-read-memory is totally insane. This patch
> introduces -data-read-memory-raw that clearly reports what regions were
> read, and shows the data as hex string. I've also added -data-write-memory-raw
> which, contrary to -data-write-memory can actually write *memory*, as opposed to
> a single byte.
I think that "raw" is not very expressive - it is too much like "new"
in that you don't know later what is newer than what. The main change
to -data-read-memory is to return a string of byte data, instead of
the word-oriented table. So how about -data-read-bytes?
> + if (argc != 2)
> + error ("Usage: ADDR LENGTH.");
Missing [-o BYTE-OFFSET] in the message.
> + /* Loop invariant is that the [current_begin, current_end) was previously
> + found to be not readable as a whole.
> +
> + Note loop condition -- if the range has 1 byte, we can't divide the range
> + so there's no point trying further. */
> + while (current_end - current_begin > 1)
> + {
> + 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;
> + if (forward)
> + {
> + first_half_begin = current_begin;
> + first_half_end = middle;
> + second_half_begin = middle;
> + second_half_end = current_end;
> + }
> + else
> + {
> + first_half_begin = middle;
> + first_half_end = current_end;
> + second_half_begin = current_begin;
> + second_half_end = middle;
> + }
> +
> + xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
> + buf + (first_half_begin - begin),
> + first_half_begin,
> + first_half_end - first_half_begin);
> +
> + if (xfer == first_half_end - first_half_begin)
> + {
> + /* This half reads up fine. So, the error must be in the other half. */
> + current_begin = second_half_begin;
> + current_end = second_half_end;
> + }
> + else
> + {
> + current_begin = first_half_begin;
> + current_end = first_half_end;
> + }
> + }
If we take the first branch of this if statement, then we've read the
"fine" half of the search space into BUF. We search the "bad" half
for the error. If we take the second branch of the if statement, then
we narrow the search space to the first half. But we didn't read the
second half. Won't we return uninitialized data in the buffer?
I think I see now. The assumption is a single valid/invalid
transition in the entire range. So we assume that in the else block,
the whole second half won't be readable. Right? If so, this is OK,
but I'd appreciate a comment to that effect.
> - if (xfer == 0)
> - return xfered;
> - if (xfer < 0)
> - {
> - remaining = half;
> - }
> - else
> - {
> - /* We have successfully read the first half. So, the
> - error must be in the second half. Adjust start and
> - remaining to point at the second half. */
> - xfered += xfer;
> - start += xfer;
> - remaining -= xfer;
> - }
> - half = remaining/2;
> + xfree (buffer);
> + xfered += to_read;
> }
Why do we skip to_read bytes if we succeed at reading zero bytes?
For that matter, what does a return value of zero mean? It seems like
this would mean the same as -1.
> -
> - return xfered;
> + else if (xfer < 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;
> + }
> + else
> + {
> + struct memory_read_result r;
> + r.data = buffer;
> + r.begin = offset + xfered;
> + r.end = r.begin + xfer;
> + VEC_safe_push (memory_read_result_s, result, &r);
> + xfered += xfer;
> + }
A single consecutive block of memory may appear as more than one
memory_read_result (and thus more than one MI record). Is this what
we want? If so, please mention it in the documentation.
> diff --git a/gdb/target.h b/gdb/target.h
> index 870a1eb..66e1166 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> +extern VEC(memory_read_result_s)*
> +read_memory_robust (struct target_ops *ops, ULONGEST offset, LONGEST len);
Formatting: function name doesn't go in column 1 for the prototype.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-07-07 16:30 ` Daniel Jacobowitz
@ 2010-07-09 18:54 ` Vladimir Prus
2010-07-09 18:59 ` Daniel Jacobowitz
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Prus @ 2010-07-09 18:54 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
On Wednesday 07 July 2010 20:29:55 Daniel Jacobowitz wrote:
> > - if (xfer == 0)
> > - return xfered;
> > - if (xfer < 0)
> > - {
> > - remaining = half;
> > - }
> > - else
> > - {
> > - /* We have successfully read the first half. So, the
> > - error must be in the second half. Adjust start and
> > - remaining to point at the second half. */
> > - xfered += xfer;
> > - start += xfer;
> > - remaining -= xfer;
> > - }
> > - half = remaining/2;
> > + xfree (buffer);
> > + xfered += to_read;
> > }
>
> Why do we skip to_read bytes if we succeed at reading zero bytes?
> For that matter, what does a return value of zero mean? It seems like
> this would mean the same as -1.
I am not really sure. Per documentation of target_read:
Return the number of bytes actually transfered, or -1 if the
transfer is not supported or otherwise fails. Return of a positive
value less than LEN indicates that no further transfer is possible.
So, value of 0 seems to mean 'there are no more bytes that that, honest',
and that we probably don't need to try further. Documentation for
to_xfer_partial seem to give such meaning to return of 0. On the
other hand, it's not clear what return value of <LEN might mean,
and whether we should try to read remaining chunk. What would you suggest?
Thanks,
--
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-07-09 18:54 ` Vladimir Prus
@ 2010-07-09 18:59 ` Daniel Jacobowitz
2010-08-11 12:38 ` Vladimir Prus
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Jacobowitz @ 2010-07-09 18:59 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
On Fri, Jul 09, 2010 at 10:54:07PM +0400, Vladimir Prus wrote:
> On Wednesday 07 July 2010 20:29:55 Daniel Jacobowitz wrote:
>
> > > - if (xfer == 0)
> > > - return xfered;
> > > - if (xfer < 0)
> > > - {
> > > - remaining = half;
> > > - }
> > > - else
> > > - {
> > > - /* We have successfully read the first half. So, the
> > > - error must be in the second half. Adjust start and
> > > - remaining to point at the second half. */
> > > - xfered += xfer;
> > > - start += xfer;
> > > - remaining -= xfer;
> > > - }
> > > - half = remaining/2;
> > > + xfree (buffer);
> > > + xfered += to_read;
> > > }
> >
> > Why do we skip to_read bytes if we succeed at reading zero bytes?
> > For that matter, what does a return value of zero mean? It seems like
> > this would mean the same as -1.
>
> I am not really sure. Per documentation of target_read:
>
> Return the number of bytes actually transfered, or -1 if the
> transfer is not supported or otherwise fails. Return of a positive
> value less than LEN indicates that no further transfer is possible.
>
> So, value of 0 seems to mean 'there are no more bytes that that, honest',
> and that we probably don't need to try further. Documentation for
> to_xfer_partial seem to give such meaning to return of 0.
I'd suggest treating 0 and -1 the same, for memory.
> On the
> other hand, it's not clear what return value of <LEN might mean,
> and whether we should try to read remaining chunk. What would you suggest?
A return of less than LEN from xfer_partial doesn't mean anything; you
just retry. A return of less than LEN from target_read, though, is
supposed to mean that there is no point in retrying; the next byte is
inaccessible or does not exist.
It doesn't look like memory reads (unlike other partial transfers)
implement that; usually they just fail. But we can treat it that way
anyway. So <LEN means we got some number of bytes, and then we should
see what happens after those successful bytes.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-06-25 16:05 ` Eli Zaretskii
@ 2010-08-11 12:38 ` Vladimir Prus
2010-08-11 18:00 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Prus @ 2010-08-11 12:38 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
[-- Attachment #1: Type: Text/Plain, Size: 3792 bytes --]
On Friday 25 June 2010 20:05:21 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Fri, 25 Jun 2010 12:32:55 +0400
> >
> > The attached patch makes a few changes in MI memory commands, with the
> > goal of making it easy for frontend to just display memory view, even
> > around the boundaries of accessible memory.
>
> Thanks. I have a few comments about the documentation part of the
> patch.
>
> > +@item @var{count}
> > +The number of bytes to read. This should be an integer literal.
> ^^
> Two spaces here, please.
>
> > +The commands attempts to read memory at the specified address.
> ^^^^^^^^^^^^
> "This command"
>
> > + When a
> > +memory map is available
>
> A cross-reference here to where memory maps are described would be
> useful.
>
> > + regions marked as unreadable in the memory
> > +map will be skipped. In addition, when @value{GDBN} encouters an error
> ^^
> Two spaces.
>
> > +reading a memory range, it will attempt to find readable subrange at
> ^^^^^^^^^^^^^^^^^^
> "a readable subrange"
>
> > +the beginning and range. Essentially, this command will make
> ^^ ^^
> Something ("end of the"?) is missing here. And two spaces between
> sentences.
>
> I'm not sure I completely understand this part:
>
> In addition, when @value{GDBN} encouters an error reading a memory
> range, it will attempt to find readable subrange [...]
>
> What if there are two or more non-contiguous sub-ranges at the
> beginning -- will GDB find and read both of them? More generally,
> what happens if the specified range has several disjoint portions that
> are not readable?
>
> I'm asking because your description seems to imply that we only look
> for the first readable address after start and the last readable
> address before the end of the specified region.
Yes, that's correct. I've adjusted the wording.
>
> If my understanding is correct, then the following sentence
>
> > + Essentially, this command will make
> > +reasonable effort to return all readable memory content within the
> > +requested range.
>
> is at least inaccurate, if not misleading ("all readable memory within
> the range").
>
> > +The output of the command has a result record named @samp{memory},
> ^^^^^^^^^^^^^^^^^^^
> Perhaps "is a result record"?
Nope. "result record" is actually a nonterminal in MI grammar, and output
of a command may have result records but is never a result record itself.
> And what is the importance of naming
> this record `memory'? why is the name important here?
Because for frontend to access a result record in a command output, it
has to know its name.
> > +@item contents
> > +The contents of the memory block, as hex-encoded string of bytes.
>
> But your example shows this:
>
> > + contents="01000000020000000300"@}]
>
> This doesn't look like a ``string of bytes'' to me. Or maybe I don't
> understand what you meant by that?
It seems very much like a hex-encoded string of bytes. Maybe, "hex-encoded
sequence of bytes" will work better?
>
> > +@item @var{contents}
> > +The hex-encoded bytes to write. The size of this parameter determines
> > +how many bytes should be written.^^^^^^^^
>
> You probably meant "the value", not "the size".
Actually, "the size". A parameter is a string, a string has a size, and the size
of that string determines how many bytes we are writing.
What about the attached revision?
Thanks,
--
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722
[-- Attachment #2: data-memory-bytes.diff --]
[-- Type: text/x-patch, Size: 19648 bytes --]
commit 97fb083bbb395f6bb9e79f5f7dba6d1caeb9e8b2
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu Jun 17 20:23:17 2010 +0400
Easier and more stubborn MI memory read commands.
gdb/
* mi/mi-cmds.c (mi_cmds): Register data-read-memory-bytes
and data-write-memory-bytes.
* mi/mi-cmds.h (mi_cmd_data_read_memory_bytes)
(mi_cmd_data_write_memory_bytes): New.
* mi/mi-main.c (mi_cmd_data_read_memory): Use regular target_read.
(mi_cmd_data_read_memory_bytes, mi_cmd_data_write_memory_bytes): New.
* target.c (target_read_until_error): Remove.
(read_whatever_is_readable, free_memory_read_result_vector)
(read_memory_robust): New.
* target.h (target_read_until_error): Remove.
(struct memory_read_result, free_memory_read_result_vector)
(read_memory_robust): New.
gdb/doc
* gdb.texinfo (GDB/MI Data Manipulation): Document
-data-read-memory-raw and -data-write-memory-raw.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index cbd636f..2765a71 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26895,6 +26895,8 @@ don't appear in the actual output):
@subheading The @code{-data-read-memory} Command
@findex -data-read-memory
+This command is deprecated, use @code{-data-read-memory-bytes} instead.
+
@subsubheading Synopsis
@smallexample
@@ -27006,6 +27008,130 @@ next-page="0x000013c0",prev-page="0x00001380",memory=[
(gdb)
@end smallexample
+@subheading The @code{-data-read-memory-bytes} Command
+@findex -data-read-memory-bytes
+
+@subsubheading Synopsis
+
+@smallexample
+ -data-read-memory-bytes [ -o @var{byte-offset} ]
+ @var{address} @var{count}
+@end smallexample
+
+@noindent
+where:
+
+@table @samp
+@item @var{address}
+An expression specifying the address of the first memory word 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.
+
+@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.
+
+@end table
+
+This command attempts to read all accessible memory regions in the
+specified range. First, all regions marked as unreadable in the memory
+map (if one is defined) will be skipped. Second, @value{GDBN} will
+attempt to read the remaining regions. For each one, if reading full
+region results in an errors, @value{GDBN} will try to read a subset
+of the region.
+
+In general, every single byte in the region may be readable or not,
+and the only way to read every readable byte is to try a read at
+every address, which is not practical. Therefore, @value{GDBN} will
+attempt to read all accessible bytes at either beginning or the end
+of the region, using a binary division scheme. This heuristic works
+well for reading accross a memory map boundary. Note that if a region
+has a readable range that is neither at the beginning or the end,
+@value{GDBN} will not read it.
+
+The output of the command has a result record named @samp{memory},
+whose content is a list of tuples. Each tuple represent a
+successfully read memory block and has the following
+fields:
+
+@table @code
+@item begin
+The start address of the memory block, as hexadecimal literal.
+
+@item end
+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}.
+
+@item contents
+The contents of the memory block, as hex-encoded string of bytes.
+
+@end table
+
+At present, if multiple
+
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{x}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-data-read-memory-bytes &a 10
+^done,memory=[@{begin="0xbffff154",offset="0x00000000",
+ end="0xbffff15e",
+ contents="01000000020000000300"@}]
+(gdb)
+@end smallexample
+
+
+@subheading The @code{-data-write-memory-bytes} Command
+@findex -data-write-memory-bytes
+
+@subsubheading Synopsis
+
+@smallexample
+ -data-write-memory-bytes @var{address} @var{contents}
+@end smallexample
+
+@noindent
+where:
+
+@table @samp
+@item @var{address}
+An expression specifying the address of the first memory word to be
+read. Complex expressions containing embedded white space should be
+quoted using the C convention.
+
+@item @var{contents}
+The hex-encoded bytes to write. The size of this parameter determines
+how many bytes should be written.
+
+@end table
+
+@subsubheading @value{GDBN} Command
+
+There's no corresponding @value{GDBN} command.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd"
+^done
+(gdb)
+@end smallexample
+
+
@c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@node GDB/MI Tracepoint Commands
@section @sc{gdb/mi} Tracepoint Commands
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 8441e17..a6a884f 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -51,7 +51,9 @@ struct mi_cmd mi_cmds[] =
{ "data-list-register-names", { NULL, 0 }, mi_cmd_data_list_register_names},
{ "data-list-register-values", { NULL, 0 }, mi_cmd_data_list_register_values},
{ "data-read-memory", { NULL, 0 }, mi_cmd_data_read_memory},
+ { "data-read-memory-bytes", { NULL, 0 }, mi_cmd_data_read_memory_bytes},
{ "data-write-memory", { NULL, 0 }, mi_cmd_data_write_memory},
+ { "data-write-memory-bytes", {NULL, 0}, mi_cmd_data_write_memory_bytes},
{ "data-write-register-values", { NULL, 0 }, mi_cmd_data_write_register_values},
{ "enable-timings", { NULL, 0 }, mi_cmd_enable_timings},
{ "enable-pretty-printing", { NULL, 0 }, mi_cmd_enable_pretty_printing},
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 5954aef..b357de6 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -47,7 +47,9 @@ extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;
extern mi_cmd_argv_ftype mi_cmd_data_list_register_values;
extern mi_cmd_argv_ftype mi_cmd_data_list_changed_registers;
extern mi_cmd_argv_ftype mi_cmd_data_read_memory;
+extern mi_cmd_argv_ftype mi_cmd_data_read_memory_bytes;
extern mi_cmd_argv_ftype mi_cmd_data_write_memory;
+extern mi_cmd_argv_ftype mi_cmd_data_write_memory_bytes;
extern mi_cmd_argv_ftype mi_cmd_data_write_register_values;
extern mi_cmd_argv_ftype mi_cmd_enable_timings;
extern mi_cmd_argv_ftype mi_cmd_env_cd;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 85a3f99..f7f28fa 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1352,9 +1352,9 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
/* Dispatch memory reads to the topmost target, not the flattened
current_target. */
- nr_bytes = target_read_until_error (current_target.beneath,
- TARGET_OBJECT_MEMORY, NULL, mbuf,
- addr, total_bytes);
+ nr_bytes = target_read (current_target.beneath,
+ TARGET_OBJECT_MEMORY, NULL, mbuf,
+ addr, total_bytes);
if (nr_bytes <= 0)
error ("Unable to read memory.");
@@ -1437,6 +1437,88 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
do_cleanups (cleanups);
}
+void
+mi_cmd_data_read_memory_bytes (char *command, char **argv, int argc)
+{
+ struct gdbarch *gdbarch = get_current_arch ();
+ struct cleanup *cleanups;
+ CORE_ADDR addr;
+ LONGEST length;
+ memory_read_result_s *read_result;
+ int ix;
+ VEC(memory_read_result_s) *result;
+ long offset = 0;
+ int optind = 0;
+ char *optarg;
+ enum opt
+ {
+ OFFSET_OPT
+ };
+ static struct mi_opt opts[] =
+ {
+ {"o", OFFSET_OPT, 1},
+ { 0, 0, 0 }
+ };
+
+ while (1)
+ {
+ int opt = mi_getopt ("mi_cmd_data_read_memory_bytes", argc, argv, opts,
+ &optind, &optarg);
+ if (opt < 0)
+ break;
+ switch ((enum opt) opt)
+ {
+ case OFFSET_OPT:
+ offset = atol (optarg);
+ break;
+ }
+ }
+ argv += optind;
+ argc -= optind;
+
+ if (argc != 2)
+ error ("Usage: [ -o OFFSET ] ADDR LENGTH.");
+
+ addr = parse_and_eval_address (argv[0]) + offset;
+ length = atol (argv[1]);
+
+ result = read_memory_robust (current_target.beneath, addr, length);
+
+ cleanups = make_cleanup (free_memory_read_result_vector, result);
+
+ if (VEC_length (memory_read_result_s, result) == 0)
+ error ("Unable to read memory.");
+
+ make_cleanup_ui_out_list_begin_end (uiout, "memory");
+ for (ix = 0;
+ VEC_iterate (memory_read_result_s, result, ix, read_result);
+ ++ix)
+ {
+ struct cleanup *t = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+ char *data, *p;
+ int i;
+
+ ui_out_field_core_addr (uiout, "begin", gdbarch, read_result->begin);
+ ui_out_field_core_addr (uiout, "offset", gdbarch, read_result->begin
+ - addr);
+ ui_out_field_core_addr (uiout, "end", gdbarch, read_result->end);
+
+ data = xmalloc ((read_result->end - read_result->begin) * 2 + 1);
+
+ for (i = 0, p = data;
+ i < (read_result->end - read_result->begin);
+ ++i, p += 2)
+ {
+ sprintf (p, "%02x", read_result->data[i]);
+ }
+ ui_out_field_string (uiout, "contents", data);
+ xfree (data);
+ do_cleanups (t);
+ }
+ do_cleanups (cleanups);
+}
+
+
/* DATA-MEMORY-WRITE:
COLUMN_OFFSET: optional argument. Must be preceeded by '-o'. The
@@ -1523,6 +1605,44 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
do_cleanups (old_chain);
}
+/* DATA-MEMORY-WRITE-RAW:
+
+ ADDR: start address
+ DATA: string of bytes to write at that address. */
+void
+mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
+{
+ CORE_ADDR addr;
+ char *cdata;
+ gdb_byte *data;
+ int len, r, i;
+ struct cleanup *back_to;
+
+ if (argc != 2)
+ error ("Usage: ADDR DATA.");
+
+ addr = parse_and_eval_address (argv[0]);
+ cdata = argv[1];
+ len = strlen (cdata)/2;
+
+ data = xmalloc (len);
+ back_to = make_cleanup (xfree, data);
+
+ for (i = 0; i < len; ++i)
+ {
+ int x;
+ sscanf (cdata + i * 2, "%02x", &x);
+ data[i] = (gdb_byte)x;
+ }
+
+ r = target_write_memory (addr, data, len);
+ if (r != 0)
+ error (_("Could not write memory"));
+
+ do_cleanups (back_to);
+}
+
+
void
mi_cmd_enable_timings (char *command, char **argv, int argc)
{
diff --git a/gdb/target.c b/gdb/target.c
index 7aa77e6..cf01b6c 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1751,73 +1751,204 @@ target_read (struct target_ops *ops,
return len;
}
-LONGEST
-target_read_until_error (struct target_ops *ops,
- enum target_object object,
- const char *annex, gdb_byte *buf,
- ULONGEST offset, LONGEST len)
+/** Assuming that the entire [begin, end) range of memory cannot be read,
+ try to read whatever subrange is possible to read.
+
+ The function results, in RESULT, either zero or one memory block.
+ If there's a readable subrange at the beginning, it is completely
+ read and returned. Any further readable subrange will not be read.
+ Otherwise, if there's a readable subrange at the end, it will be
+ completely read and returned. Any readable subranges before it (obviously,
+ not starting at the beginning), will be ignored. In other cases --
+ either no readable subrange, or readable subrange (s) that is neither
+ at the beginning, or end, nothing is returned.
+
+ The purpose of this function is to handle a read across a boundary of
+ accessible memory in a case when memory map is not available. The above
+ restrictions are fine for this case, but will give incorrect results if
+ the memory is 'patchy'. However, supporting 'patchy' memory would require
+ trying to read every single byte, and it seems unacceptable solution.
+ Explicit memory map is recommended for this case -- and
+ target_read_memory_robust will take care of reading multiple ranges then. */
+
+static void
+read_whatever_is_readable (struct target_ops *ops, ULONGEST begin, ULONGEST end,
+ VEC(memory_read_result_s) **result)
{
- LONGEST xfered = 0;
+ gdb_byte *buf = xmalloc (end-begin);
+ ULONGEST current_begin = begin;
+ ULONGEST current_end = end;
+ int forward;
+ memory_read_result_s r;
+
+ /* If we previously failed to read 1 byte, nothing can be done here. */
+ if (end - begin <= 1)
+ return;
+
+ /* Check that either first or the last byte is readable, and give up
+ if not. This heuristic is meant to permit reading accessible memory
+ at the boundary of accessible region. */
+ if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf, begin, 1) == 1)
+ {
+ forward = 1;
+ ++current_begin;
+ }
+ else if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf + (end-begin) - 1, end - 1, 1) == 1)
+ {
+ forward = 0;
+ --current_end;
+ }
+ else
+ {
+ return;
+ }
+
+ /* Loop invariant is that the [current_begin, current_end) was previously
+ found to be not readable as a whole.
+
+ Note loop condition -- if the range has 1 byte, we can't divide the range
+ so there's no point trying further. */
+ while (current_end - current_begin > 1)
+ {
+ 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;
+ if (forward)
+ {
+ first_half_begin = current_begin;
+ first_half_end = middle;
+ second_half_begin = middle;
+ second_half_end = current_end;
+ }
+ else
+ {
+ first_half_begin = middle;
+ first_half_end = current_end;
+ second_half_begin = current_begin;
+ second_half_end = middle;
+ }
+
+ xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf + (first_half_begin - begin),
+ first_half_begin,
+ first_half_end - first_half_begin);
+
+ if (xfer == first_half_end - first_half_begin)
+ {
+ /* This half reads up fine. So, the error must be in the other half. */
+ current_begin = second_half_begin;
+ current_end = second_half_end;
+ }
+ 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
+ iteration to divide again and try to read.
+
+ We don't handle the other half, because this function only tries
+ to read a single readable subrange. */
+ current_begin = first_half_begin;
+ current_end = first_half_end;
+ }
+ }
+
+ if (forward)
+ {
+ /* The [begin, current_begin) range has been read. */
+ r.begin = begin;
+ r.end = current_begin;
+ r.data = buf;
+ }
+ else
+ {
+ /* The [current_end, end) range has been read. */
+ LONGEST rlen = end - current_end;
+ r.data = xmalloc (rlen);
+ memcpy (r.data, buf + current_end - begin, rlen);
+ r.begin = current_end;
+ r.end = end;
+ xfree (buf);
+ }
+ VEC_safe_push(memory_read_result_s, (*result), &r);
+}
+
+void
+free_memory_read_result_vector (void *x)
+{
+ VEC(memory_read_result_s) *v = x;
+ memory_read_result_s *current;
+ int ix;
+
+ for (ix = 0; VEC_iterate (memory_read_result_s, v, ix, current); ++ix)
+ {
+ xfree (current->data);
+ }
+ VEC_free (memory_read_result_s, v);
+}
+VEC(memory_read_result_s) *
+read_memory_robust (struct target_ops *ops, ULONGEST offset, LONGEST len)
+{
+ VEC(memory_read_result_s) *result = 0;
+
+ LONGEST xfered = 0;
while (xfered < len)
{
- LONGEST xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- offset + xfered, len - xfered);
+ struct mem_region *region = lookup_mem_region (offset + xfered);
+ LONGEST rlen;
- /* Call an observer, notifying them of the xfer progress? */
- if (xfer == 0)
- return xfered;
- if (xfer < 0)
+ /* If there is no explicit region, a fake one should be created. */
+ gdb_assert (region);
+
+ if (region->hi == 0)
+ rlen = len - xfered;
+ else
+ rlen = region->hi - offset;
+
+ if (region->attrib.mode == MEM_NONE || region->attrib.mode == MEM_WO)
{
- /* We've got an error. Try to read in smaller blocks. */
- ULONGEST start = offset + xfered;
- ULONGEST remaining = len - xfered;
- ULONGEST half;
-
- /* If an attempt was made to read a random memory address,
- it's likely that the very first byte is not accessible.
- Try reading the first byte, to avoid doing log N tries
- below. */
- xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered, start, 1);
+ /* 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;
+ }
+ else
+ {
+ LONGEST to_read = min (len - xfered, rlen);
+ gdb_byte *buffer = (gdb_byte *)xmalloc (to_read);
+
+ LONGEST xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ (gdb_byte *) buffer,
+ offset + xfered, to_read);
+ /* Call an observer, notifying them of the xfer progress? */
if (xfer <= 0)
- return xfered;
- start += 1;
- remaining -= 1;
- half = remaining/2;
-
- while (half > 0)
{
- xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- start, half);
- if (xfer == 0)
- return xfered;
- if (xfer < 0)
- {
- remaining = half;
- }
- else
- {
- /* We have successfully read the first half. So, the
- error must be in the second half. Adjust start and
- remaining to point at the second half. */
- xfered += xfer;
- start += xfer;
- remaining -= xfer;
- }
- half = remaining/2;
+ /* 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;
}
-
- return xfered;
+ else
+ {
+ struct memory_read_result r;
+ r.data = buffer;
+ r.begin = offset + xfered;
+ r.end = r.begin + xfer;
+ VEC_safe_push (memory_read_result_s, result, &r);
+ xfered += xfer;
+ }
+ QUIT;
}
- xfered += xfer;
- QUIT;
}
- return len;
+ return result;
}
+
/* An alternative to target_write with progress callbacks. */
LONGEST
diff --git a/gdb/target.h b/gdb/target.h
index 870a1eb..e0a63ca 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -292,10 +292,23 @@ extern LONGEST target_read (struct target_ops *ops,
const char *annex, gdb_byte *buf,
ULONGEST offset, LONGEST len);
-extern LONGEST target_read_until_error (struct target_ops *ops,
- enum target_object object,
- const char *annex, gdb_byte *buf,
- ULONGEST offset, LONGEST len);
+struct memory_read_result
+ {
+ /* First address that was read. */
+ ULONGEST begin;
+ /* Past-the-end address. */
+ ULONGEST end;
+ /* The data. */
+ gdb_byte *data;
+};
+typedef struct memory_read_result memory_read_result_s;
+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);
extern LONGEST target_write (struct target_ops *ops,
enum target_object object,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-07-09 18:59 ` Daniel Jacobowitz
@ 2010-08-11 12:38 ` Vladimir Prus
2010-08-11 21:10 ` Daniel Jacobowitz
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Prus @ 2010-08-11 12:38 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
[-- Attachment #1: Type: Text/Plain, Size: 2510 bytes --]
On Friday 09 July 2010 22:59:06 Daniel Jacobowitz wrote:
> On Fri, Jul 09, 2010 at 10:54:07PM +0400, Vladimir Prus wrote:
> > On Wednesday 07 July 2010 20:29:55 Daniel Jacobowitz wrote:
> >
> > > > - if (xfer == 0)
> > > > - return xfered;
> > > > - if (xfer < 0)
> > > > - {
> > > > - remaining = half;
> > > > - }
> > > > - else
> > > > - {
> > > > - /* We have successfully read the first half. So, the
> > > > - error must be in the second half. Adjust start and
> > > > - remaining to point at the second half. */
> > > > - xfered += xfer;
> > > > - start += xfer;
> > > > - remaining -= xfer;
> > > > - }
> > > > - half = remaining/2;
> > > > + xfree (buffer);
> > > > + xfered += to_read;
> > > > }
> > >
> > > Why do we skip to_read bytes if we succeed at reading zero bytes?
> > > For that matter, what does a return value of zero mean? It seems like
> > > this would mean the same as -1.
> >
> > I am not really sure. Per documentation of target_read:
> >
> > Return the number of bytes actually transfered, or -1 if the
> > transfer is not supported or otherwise fails. Return of a positive
> > value less than LEN indicates that no further transfer is possible.
> >
> > So, value of 0 seems to mean 'there are no more bytes that that, honest',
> > and that we probably don't need to try further. Documentation for
> > to_xfer_partial seem to give such meaning to return of 0.
>
> I'd suggest treating 0 and -1 the same, for memory.
>
> > On the
> > other hand, it's not clear what return value of <LEN might mean,
> > and whether we should try to read remaining chunk. What would you suggest?
>
> A return of less than LEN from xfer_partial doesn't mean anything; you
> just retry. A return of less than LEN from target_read, though, is
> supposed to mean that there is no point in retrying; the next byte is
> inaccessible or does not exist.
>
> It doesn't look like memory reads (unlike other partial transfers)
> implement that; usually they just fail. But we can treat it that way
> anyway. So <LEN means we got some number of bytes, and then we should
> see what happens after those successful bytes.
Here's a revised version.
Thanks,
--
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722
[-- Attachment #2: data-memory-bytes.diff --]
[-- Type: text/x-patch, Size: 19648 bytes --]
commit 97fb083bbb395f6bb9e79f5f7dba6d1caeb9e8b2
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu Jun 17 20:23:17 2010 +0400
Easier and more stubborn MI memory read commands.
gdb/
* mi/mi-cmds.c (mi_cmds): Register data-read-memory-bytes
and data-write-memory-bytes.
* mi/mi-cmds.h (mi_cmd_data_read_memory_bytes)
(mi_cmd_data_write_memory_bytes): New.
* mi/mi-main.c (mi_cmd_data_read_memory): Use regular target_read.
(mi_cmd_data_read_memory_bytes, mi_cmd_data_write_memory_bytes): New.
* target.c (target_read_until_error): Remove.
(read_whatever_is_readable, free_memory_read_result_vector)
(read_memory_robust): New.
* target.h (target_read_until_error): Remove.
(struct memory_read_result, free_memory_read_result_vector)
(read_memory_robust): New.
gdb/doc
* gdb.texinfo (GDB/MI Data Manipulation): Document
-data-read-memory-raw and -data-write-memory-raw.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index cbd636f..2765a71 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26895,6 +26895,8 @@ don't appear in the actual output):
@subheading The @code{-data-read-memory} Command
@findex -data-read-memory
+This command is deprecated, use @code{-data-read-memory-bytes} instead.
+
@subsubheading Synopsis
@smallexample
@@ -27006,6 +27008,130 @@ next-page="0x000013c0",prev-page="0x00001380",memory=[
(gdb)
@end smallexample
+@subheading The @code{-data-read-memory-bytes} Command
+@findex -data-read-memory-bytes
+
+@subsubheading Synopsis
+
+@smallexample
+ -data-read-memory-bytes [ -o @var{byte-offset} ]
+ @var{address} @var{count}
+@end smallexample
+
+@noindent
+where:
+
+@table @samp
+@item @var{address}
+An expression specifying the address of the first memory word 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.
+
+@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.
+
+@end table
+
+This command attempts to read all accessible memory regions in the
+specified range. First, all regions marked as unreadable in the memory
+map (if one is defined) will be skipped. Second, @value{GDBN} will
+attempt to read the remaining regions. For each one, if reading full
+region results in an errors, @value{GDBN} will try to read a subset
+of the region.
+
+In general, every single byte in the region may be readable or not,
+and the only way to read every readable byte is to try a read at
+every address, which is not practical. Therefore, @value{GDBN} will
+attempt to read all accessible bytes at either beginning or the end
+of the region, using a binary division scheme. This heuristic works
+well for reading accross a memory map boundary. Note that if a region
+has a readable range that is neither at the beginning or the end,
+@value{GDBN} will not read it.
+
+The output of the command has a result record named @samp{memory},
+whose content is a list of tuples. Each tuple represent a
+successfully read memory block and has the following
+fields:
+
+@table @code
+@item begin
+The start address of the memory block, as hexadecimal literal.
+
+@item end
+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}.
+
+@item contents
+The contents of the memory block, as hex-encoded string of bytes.
+
+@end table
+
+At present, if multiple
+
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{x}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-data-read-memory-bytes &a 10
+^done,memory=[@{begin="0xbffff154",offset="0x00000000",
+ end="0xbffff15e",
+ contents="01000000020000000300"@}]
+(gdb)
+@end smallexample
+
+
+@subheading The @code{-data-write-memory-bytes} Command
+@findex -data-write-memory-bytes
+
+@subsubheading Synopsis
+
+@smallexample
+ -data-write-memory-bytes @var{address} @var{contents}
+@end smallexample
+
+@noindent
+where:
+
+@table @samp
+@item @var{address}
+An expression specifying the address of the first memory word to be
+read. Complex expressions containing embedded white space should be
+quoted using the C convention.
+
+@item @var{contents}
+The hex-encoded bytes to write. The size of this parameter determines
+how many bytes should be written.
+
+@end table
+
+@subsubheading @value{GDBN} Command
+
+There's no corresponding @value{GDBN} command.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd"
+^done
+(gdb)
+@end smallexample
+
+
@c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@node GDB/MI Tracepoint Commands
@section @sc{gdb/mi} Tracepoint Commands
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 8441e17..a6a884f 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -51,7 +51,9 @@ struct mi_cmd mi_cmds[] =
{ "data-list-register-names", { NULL, 0 }, mi_cmd_data_list_register_names},
{ "data-list-register-values", { NULL, 0 }, mi_cmd_data_list_register_values},
{ "data-read-memory", { NULL, 0 }, mi_cmd_data_read_memory},
+ { "data-read-memory-bytes", { NULL, 0 }, mi_cmd_data_read_memory_bytes},
{ "data-write-memory", { NULL, 0 }, mi_cmd_data_write_memory},
+ { "data-write-memory-bytes", {NULL, 0}, mi_cmd_data_write_memory_bytes},
{ "data-write-register-values", { NULL, 0 }, mi_cmd_data_write_register_values},
{ "enable-timings", { NULL, 0 }, mi_cmd_enable_timings},
{ "enable-pretty-printing", { NULL, 0 }, mi_cmd_enable_pretty_printing},
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 5954aef..b357de6 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -47,7 +47,9 @@ extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;
extern mi_cmd_argv_ftype mi_cmd_data_list_register_values;
extern mi_cmd_argv_ftype mi_cmd_data_list_changed_registers;
extern mi_cmd_argv_ftype mi_cmd_data_read_memory;
+extern mi_cmd_argv_ftype mi_cmd_data_read_memory_bytes;
extern mi_cmd_argv_ftype mi_cmd_data_write_memory;
+extern mi_cmd_argv_ftype mi_cmd_data_write_memory_bytes;
extern mi_cmd_argv_ftype mi_cmd_data_write_register_values;
extern mi_cmd_argv_ftype mi_cmd_enable_timings;
extern mi_cmd_argv_ftype mi_cmd_env_cd;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 85a3f99..f7f28fa 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1352,9 +1352,9 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
/* Dispatch memory reads to the topmost target, not the flattened
current_target. */
- nr_bytes = target_read_until_error (current_target.beneath,
- TARGET_OBJECT_MEMORY, NULL, mbuf,
- addr, total_bytes);
+ nr_bytes = target_read (current_target.beneath,
+ TARGET_OBJECT_MEMORY, NULL, mbuf,
+ addr, total_bytes);
if (nr_bytes <= 0)
error ("Unable to read memory.");
@@ -1437,6 +1437,88 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
do_cleanups (cleanups);
}
+void
+mi_cmd_data_read_memory_bytes (char *command, char **argv, int argc)
+{
+ struct gdbarch *gdbarch = get_current_arch ();
+ struct cleanup *cleanups;
+ CORE_ADDR addr;
+ LONGEST length;
+ memory_read_result_s *read_result;
+ int ix;
+ VEC(memory_read_result_s) *result;
+ long offset = 0;
+ int optind = 0;
+ char *optarg;
+ enum opt
+ {
+ OFFSET_OPT
+ };
+ static struct mi_opt opts[] =
+ {
+ {"o", OFFSET_OPT, 1},
+ { 0, 0, 0 }
+ };
+
+ while (1)
+ {
+ int opt = mi_getopt ("mi_cmd_data_read_memory_bytes", argc, argv, opts,
+ &optind, &optarg);
+ if (opt < 0)
+ break;
+ switch ((enum opt) opt)
+ {
+ case OFFSET_OPT:
+ offset = atol (optarg);
+ break;
+ }
+ }
+ argv += optind;
+ argc -= optind;
+
+ if (argc != 2)
+ error ("Usage: [ -o OFFSET ] ADDR LENGTH.");
+
+ addr = parse_and_eval_address (argv[0]) + offset;
+ length = atol (argv[1]);
+
+ result = read_memory_robust (current_target.beneath, addr, length);
+
+ cleanups = make_cleanup (free_memory_read_result_vector, result);
+
+ if (VEC_length (memory_read_result_s, result) == 0)
+ error ("Unable to read memory.");
+
+ make_cleanup_ui_out_list_begin_end (uiout, "memory");
+ for (ix = 0;
+ VEC_iterate (memory_read_result_s, result, ix, read_result);
+ ++ix)
+ {
+ struct cleanup *t = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+ char *data, *p;
+ int i;
+
+ ui_out_field_core_addr (uiout, "begin", gdbarch, read_result->begin);
+ ui_out_field_core_addr (uiout, "offset", gdbarch, read_result->begin
+ - addr);
+ ui_out_field_core_addr (uiout, "end", gdbarch, read_result->end);
+
+ data = xmalloc ((read_result->end - read_result->begin) * 2 + 1);
+
+ for (i = 0, p = data;
+ i < (read_result->end - read_result->begin);
+ ++i, p += 2)
+ {
+ sprintf (p, "%02x", read_result->data[i]);
+ }
+ ui_out_field_string (uiout, "contents", data);
+ xfree (data);
+ do_cleanups (t);
+ }
+ do_cleanups (cleanups);
+}
+
+
/* DATA-MEMORY-WRITE:
COLUMN_OFFSET: optional argument. Must be preceeded by '-o'. The
@@ -1523,6 +1605,44 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
do_cleanups (old_chain);
}
+/* DATA-MEMORY-WRITE-RAW:
+
+ ADDR: start address
+ DATA: string of bytes to write at that address. */
+void
+mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
+{
+ CORE_ADDR addr;
+ char *cdata;
+ gdb_byte *data;
+ int len, r, i;
+ struct cleanup *back_to;
+
+ if (argc != 2)
+ error ("Usage: ADDR DATA.");
+
+ addr = parse_and_eval_address (argv[0]);
+ cdata = argv[1];
+ len = strlen (cdata)/2;
+
+ data = xmalloc (len);
+ back_to = make_cleanup (xfree, data);
+
+ for (i = 0; i < len; ++i)
+ {
+ int x;
+ sscanf (cdata + i * 2, "%02x", &x);
+ data[i] = (gdb_byte)x;
+ }
+
+ r = target_write_memory (addr, data, len);
+ if (r != 0)
+ error (_("Could not write memory"));
+
+ do_cleanups (back_to);
+}
+
+
void
mi_cmd_enable_timings (char *command, char **argv, int argc)
{
diff --git a/gdb/target.c b/gdb/target.c
index 7aa77e6..cf01b6c 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1751,73 +1751,204 @@ target_read (struct target_ops *ops,
return len;
}
-LONGEST
-target_read_until_error (struct target_ops *ops,
- enum target_object object,
- const char *annex, gdb_byte *buf,
- ULONGEST offset, LONGEST len)
+/** Assuming that the entire [begin, end) range of memory cannot be read,
+ try to read whatever subrange is possible to read.
+
+ The function results, in RESULT, either zero or one memory block.
+ If there's a readable subrange at the beginning, it is completely
+ read and returned. Any further readable subrange will not be read.
+ Otherwise, if there's a readable subrange at the end, it will be
+ completely read and returned. Any readable subranges before it (obviously,
+ not starting at the beginning), will be ignored. In other cases --
+ either no readable subrange, or readable subrange (s) that is neither
+ at the beginning, or end, nothing is returned.
+
+ The purpose of this function is to handle a read across a boundary of
+ accessible memory in a case when memory map is not available. The above
+ restrictions are fine for this case, but will give incorrect results if
+ the memory is 'patchy'. However, supporting 'patchy' memory would require
+ trying to read every single byte, and it seems unacceptable solution.
+ Explicit memory map is recommended for this case -- and
+ target_read_memory_robust will take care of reading multiple ranges then. */
+
+static void
+read_whatever_is_readable (struct target_ops *ops, ULONGEST begin, ULONGEST end,
+ VEC(memory_read_result_s) **result)
{
- LONGEST xfered = 0;
+ gdb_byte *buf = xmalloc (end-begin);
+ ULONGEST current_begin = begin;
+ ULONGEST current_end = end;
+ int forward;
+ memory_read_result_s r;
+
+ /* If we previously failed to read 1 byte, nothing can be done here. */
+ if (end - begin <= 1)
+ return;
+
+ /* Check that either first or the last byte is readable, and give up
+ if not. This heuristic is meant to permit reading accessible memory
+ at the boundary of accessible region. */
+ if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf, begin, 1) == 1)
+ {
+ forward = 1;
+ ++current_begin;
+ }
+ else if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf + (end-begin) - 1, end - 1, 1) == 1)
+ {
+ forward = 0;
+ --current_end;
+ }
+ else
+ {
+ return;
+ }
+
+ /* Loop invariant is that the [current_begin, current_end) was previously
+ found to be not readable as a whole.
+
+ Note loop condition -- if the range has 1 byte, we can't divide the range
+ so there's no point trying further. */
+ while (current_end - current_begin > 1)
+ {
+ 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;
+ if (forward)
+ {
+ first_half_begin = current_begin;
+ first_half_end = middle;
+ second_half_begin = middle;
+ second_half_end = current_end;
+ }
+ else
+ {
+ first_half_begin = middle;
+ first_half_end = current_end;
+ second_half_begin = current_begin;
+ second_half_end = middle;
+ }
+
+ xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf + (first_half_begin - begin),
+ first_half_begin,
+ first_half_end - first_half_begin);
+
+ if (xfer == first_half_end - first_half_begin)
+ {
+ /* This half reads up fine. So, the error must be in the other half. */
+ current_begin = second_half_begin;
+ current_end = second_half_end;
+ }
+ 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
+ iteration to divide again and try to read.
+
+ We don't handle the other half, because this function only tries
+ to read a single readable subrange. */
+ current_begin = first_half_begin;
+ current_end = first_half_end;
+ }
+ }
+
+ if (forward)
+ {
+ /* The [begin, current_begin) range has been read. */
+ r.begin = begin;
+ r.end = current_begin;
+ r.data = buf;
+ }
+ else
+ {
+ /* The [current_end, end) range has been read. */
+ LONGEST rlen = end - current_end;
+ r.data = xmalloc (rlen);
+ memcpy (r.data, buf + current_end - begin, rlen);
+ r.begin = current_end;
+ r.end = end;
+ xfree (buf);
+ }
+ VEC_safe_push(memory_read_result_s, (*result), &r);
+}
+
+void
+free_memory_read_result_vector (void *x)
+{
+ VEC(memory_read_result_s) *v = x;
+ memory_read_result_s *current;
+ int ix;
+
+ for (ix = 0; VEC_iterate (memory_read_result_s, v, ix, current); ++ix)
+ {
+ xfree (current->data);
+ }
+ VEC_free (memory_read_result_s, v);
+}
+VEC(memory_read_result_s) *
+read_memory_robust (struct target_ops *ops, ULONGEST offset, LONGEST len)
+{
+ VEC(memory_read_result_s) *result = 0;
+
+ LONGEST xfered = 0;
while (xfered < len)
{
- LONGEST xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- offset + xfered, len - xfered);
+ struct mem_region *region = lookup_mem_region (offset + xfered);
+ LONGEST rlen;
- /* Call an observer, notifying them of the xfer progress? */
- if (xfer == 0)
- return xfered;
- if (xfer < 0)
+ /* If there is no explicit region, a fake one should be created. */
+ gdb_assert (region);
+
+ if (region->hi == 0)
+ rlen = len - xfered;
+ else
+ rlen = region->hi - offset;
+
+ if (region->attrib.mode == MEM_NONE || region->attrib.mode == MEM_WO)
{
- /* We've got an error. Try to read in smaller blocks. */
- ULONGEST start = offset + xfered;
- ULONGEST remaining = len - xfered;
- ULONGEST half;
-
- /* If an attempt was made to read a random memory address,
- it's likely that the very first byte is not accessible.
- Try reading the first byte, to avoid doing log N tries
- below. */
- xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered, start, 1);
+ /* 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;
+ }
+ else
+ {
+ LONGEST to_read = min (len - xfered, rlen);
+ gdb_byte *buffer = (gdb_byte *)xmalloc (to_read);
+
+ LONGEST xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ (gdb_byte *) buffer,
+ offset + xfered, to_read);
+ /* Call an observer, notifying them of the xfer progress? */
if (xfer <= 0)
- return xfered;
- start += 1;
- remaining -= 1;
- half = remaining/2;
-
- while (half > 0)
{
- xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- start, half);
- if (xfer == 0)
- return xfered;
- if (xfer < 0)
- {
- remaining = half;
- }
- else
- {
- /* We have successfully read the first half. So, the
- error must be in the second half. Adjust start and
- remaining to point at the second half. */
- xfered += xfer;
- start += xfer;
- remaining -= xfer;
- }
- half = remaining/2;
+ /* 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;
}
-
- return xfered;
+ else
+ {
+ struct memory_read_result r;
+ r.data = buffer;
+ r.begin = offset + xfered;
+ r.end = r.begin + xfer;
+ VEC_safe_push (memory_read_result_s, result, &r);
+ xfered += xfer;
+ }
+ QUIT;
}
- xfered += xfer;
- QUIT;
}
- return len;
+ return result;
}
+
/* An alternative to target_write with progress callbacks. */
LONGEST
diff --git a/gdb/target.h b/gdb/target.h
index 870a1eb..e0a63ca 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -292,10 +292,23 @@ extern LONGEST target_read (struct target_ops *ops,
const char *annex, gdb_byte *buf,
ULONGEST offset, LONGEST len);
-extern LONGEST target_read_until_error (struct target_ops *ops,
- enum target_object object,
- const char *annex, gdb_byte *buf,
- ULONGEST offset, LONGEST len);
+struct memory_read_result
+ {
+ /* First address that was read. */
+ ULONGEST begin;
+ /* Past-the-end address. */
+ ULONGEST end;
+ /* The data. */
+ gdb_byte *data;
+};
+typedef struct memory_read_result memory_read_result_s;
+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);
extern LONGEST target_write (struct target_ops *ops,
enum target_object object,
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-08-11 12:38 ` Vladimir Prus
@ 2010-08-11 18:00 ` Eli Zaretskii
2010-08-12 7:25 ` Vladimir Prus
0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2010-08-11 18:00 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Wed, 11 Aug 2010 16:37:49 +0400
> Cc: gdb-patches@sources.redhat.com
>
> > > +The output of the command has a result record named @samp{memory},
> > ^^^^^^^^^^^^^^^^^^^
> > Perhaps "is a result record"?
>
> Nope. "result record" is actually a nonterminal in MI grammar, and output
> of a command may have result records but is never a result record itself.
>
> > And what is the importance of naming
> > this record `memory'? why is the name important here?
>
> Because for frontend to access a result record in a command output, it
> has to know its name.
Then perhaps
The result record (@pxref{GDB/MI Result Records}) that is output of
the command includes a field named @samp{memory} whose content is a
list of tuples ...
> > > +@item contents
> > > +The contents of the memory block, as hex-encoded string of bytes.
> >
> > But your example shows this:
> >
> > > + contents="01000000020000000300"@}]
> >
> > This doesn't look like a ``string of bytes'' to me. Or maybe I don't
> > understand what you meant by that?
>
> It seems very much like a hex-encoded string of bytes. Maybe, "hex-encoded
> sequence of bytes" will work better?
I suggest just
The contents of the memory block, in hex.
> > > +@item @var{contents}
> > > +The hex-encoded bytes to write. The size of this parameter determines
> > > +how many bytes should be written.^^^^^^^^
> >
> > You probably meant "the value", not "the size".
>
> Actually, "the size". A parameter is a string, a string has a size
A string has a length, not size, so please use that. Actually,
perhaps this sentence should be simply removed, as it seems to say
something trivial, doesn't it?
> What about the attached revision?
Okay, with the above changes and two more comments:
> +This command attempts to read all accessible memory regions in the
> +specified range. First, all regions marked as unreadable in the memory
> +map (if one is defined) will be skipped.
^^^^^^^^^^^^^^^^^^^^^^^
I asked for a cross-reference here to where memory maps are
described.
> +At present, if multiple
What happened here?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-08-11 12:38 ` Vladimir Prus
@ 2010-08-11 21:10 ` Daniel Jacobowitz
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Jacobowitz @ 2010-08-11 21:10 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
On Wed, Aug 11, 2010 at 04:38:24PM +0400, Vladimir Prus wrote:
> Here's a revised version.
Code looks OK to me.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-08-11 18:00 ` Eli Zaretskii
@ 2010-08-12 7:25 ` Vladimir Prus
2010-08-12 17:04 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Prus @ 2010-08-12 7:25 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On Wednesday 11 August 2010 22:00:21 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Wed, 11 Aug 2010 16:37:49 +0400
> > Cc: gdb-patches@sources.redhat.com
> >
> > > > +The output of the command has a result record named @samp{memory},
> > > ^^^^^^^^^^^^^^^^^^^
> > > Perhaps "is a result record"?
> >
> > Nope. "result record" is actually a nonterminal in MI grammar, and output
> > of a command may have result records but is never a result record itself.
> >
> > > And what is the importance of naming
> > > this record `memory'? why is the name important here?
> >
> > Because for frontend to access a result record in a command output, it
> > has to know its name.
>
> Then perhaps
>
> The result record (@pxref{GDB/MI Result Records}) that is output of
> the command includes a field named @samp{memory} whose content is a
> list of tuples ...
This is still not accurate. The output of the command is:
- the character sequence "^done"
- zero, one, or more result results, separated with spaces.
So, it's still an command output that "has" a result record named "memory"
as opposed to "result record .. is .. output of the command"
> > > > +@item contents
> > > > +The contents of the memory block, as hex-encoded string of bytes.
> > >
> > > But your example shows this:
> > >
> > > > + contents="01000000020000000300"@}]
> > >
> > > This doesn't look like a ``string of bytes'' to me. Or maybe I don't
> > > understand what you meant by that?
> >
> > It seems very much like a hex-encoded string of bytes. Maybe, "hex-encoded
> > sequence of bytes" will work better?
> I suggest just
>
> The contents of the memory block, in hex.
OK.
> > > > +@item @var{contents}
> > > > +The hex-encoded bytes to write. The size of this parameter determines
> > > > +how many bytes should be written.^^^^^^^^
> > >
> > > You probably meant "the value", not "the size".
> >
> > Actually, "the size". A parameter is a string, a string has a size
>
> A string has a length, not size, so please use that. Actually,
> perhaps this sentence should be simply removed, as it seems to say
> something trivial, doesn't it?
It's your call. It seemed to me that clarifying that there's no explicit option
for size might help users, who otherwise might decide the docs are
incompletely.
>
> > What about the attached revision?
>
> Okay, with the above changes and two more comments:
>
> > +This command attempts to read all accessible memory regions in the
> > +specified range. First, all regions marked as unreadable in the memory
> > +map (if one is defined) will be skipped.
> ^^^^^^^^^^^^^^^^^^^^^^^
> I asked for a cross-reference here to where memory maps are
> described.
Added. (I did not find a relevant section on the first try, and then
forgot)
>
> > +At present, if multiple
>
> What happened here?
>
I meant to clarify that more than one memory block can be returned,
as requested by Dan, but then realize the previous sequence actually
say that already. I've removed this bit now.
Thanks,
--
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-08-12 7:25 ` Vladimir Prus
@ 2010-08-12 17:04 ` Eli Zaretskii
2010-08-13 13:23 ` Vladimir Prus
0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2010-08-12 17:04 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Thu, 12 Aug 2010 11:25:02 +0400
> Cc: gdb-patches@sources.redhat.com
>
> On Wednesday 11 August 2010 22:00:21 Eli Zaretskii wrote:
>
> > > From: Vladimir Prus <vladimir@codesourcery.com>
> > > Date: Wed, 11 Aug 2010 16:37:49 +0400
> > > Cc: gdb-patches@sources.redhat.com
> > >
> > > > > +The output of the command has a result record named @samp{memory},
> > > > ^^^^^^^^^^^^^^^^^^^
> > > > Perhaps "is a result record"?
> > >
> > > Nope. "result record" is actually a nonterminal in MI grammar, and output
> > > of a command may have result records but is never a result record itself.
> > >
> > > > And what is the importance of naming
> > > > this record `memory'? why is the name important here?
> > >
> > > Because for frontend to access a result record in a command output, it
> > > has to know its name.
> >
> > Then perhaps
> >
> > The result record (@pxref{GDB/MI Result Records}) that is output of
> > the command includes a field named @samp{memory} whose content is a
> > list of tuples ...
>
> This is still not accurate. The output of the command is:
>
> - the character sequence "^done"
> - zero, one, or more result results, separated with spaces.
>
> So, it's still an command output that "has" a result record named "memory"
> as opposed to "result record .. is .. output of the command"
This is a misunderstanding. In the text I suggested, "record that is
output", the "output" part is a verb, not a noun. (I should have said
"output by the command", though.) IOW, the text says that the command
outputs some stuff, and part of that stuff is the result vector with a
field named "memory".
Saying "output has the result record" is not good English, because the
result record is part of the output, not some attribute of it.
I could try to be more accurate, if "that is output by the command" is
not good enough, but please note that our current definition of what
exactly is a "result record" is quite vague. In fact, we don't say
what it is at all. For starters, it talks about "result indications":
In addition to a number of out-of-band notifications, the response to a
@sc{gdb/mi} command includes one of the following result indications:
@table @code
@findex ^done
@item "^done" [ "," @var{results} ]
The synchronous operation was successful, @code{@var{results}} are the return
values.
@item "^running"
@findex ^running
This result record is equivalent to @samp{^done}. Historically, it
was output instead of @samp{^done} if the command has resumed the
target. This behaviour is maintained for backward compatibility, but
all frontends should treat @samp{^done} and @samp{^running}
identically and rely on the @samp{*running} output record to determine
which threads are resumed.
My reading of this is that ^done, ^running, etc. _are_ examples of
result records. By contrast, you say above that a result record is
everything _after_ these indications. This inconsistency is the main
reason why I deliberately left the text I suggested slightly vague.
I'm open to other suggestions, but if we want to be rigorous, we
should be more consistent than we are now regarding what exactly is a
result record.
> > > > > +@item @var{contents}
> > > > > +The hex-encoded bytes to write. The size of this parameter determines
> > > > > +how many bytes should be written.^^^^^^^^
> > > >
> > > > You probably meant "the value", not "the size".
> > >
> > > Actually, "the size". A parameter is a string, a string has a size
> >
> > A string has a length, not size, so please use that. Actually,
> > perhaps this sentence should be simply removed, as it seems to say
> > something trivial, doesn't it?
>
> It's your call. It seemed to me that clarifying that there's no explicit option
> for size might help users, who otherwise might decide the docs are
> incompletely.
I don't mind leaving it, if you think it's important.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Better MI memory commands
2010-08-12 17:04 ` Eli Zaretskii
@ 2010-08-13 13:23 ` Vladimir Prus
0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Prus @ 2010-08-13 13:23 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
[-- Attachment #1: Type: Text/Plain, Size: 3869 bytes --]
On Thursday 12 August 2010 21:04:36 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Thu, 12 Aug 2010 11:25:02 +0400
> > Cc: gdb-patches@sources.redhat.com
> >
> > On Wednesday 11 August 2010 22:00:21 Eli Zaretskii wrote:
> >
> > > > From: Vladimir Prus <vladimir@codesourcery.com>
> > > > Date: Wed, 11 Aug 2010 16:37:49 +0400
> > > > Cc: gdb-patches@sources.redhat.com
> > > >
> > > > > > +The output of the command has a result record named @samp{memory},
> > > > > ^^^^^^^^^^^^^^^^^^^
> > > > > Perhaps "is a result record"?
> > > >
> > > > Nope. "result record" is actually a nonterminal in MI grammar, and output
> > > > of a command may have result records but is never a result record itself.
> > > >
> > > > > And what is the importance of naming
> > > > > this record `memory'? why is the name important here?
> > > >
> > > > Because for frontend to access a result record in a command output, it
> > > > has to know its name.
> > >
> > > Then perhaps
> > >
> > > The result record (@pxref{GDB/MI Result Records}) that is output of
> > > the command includes a field named @samp{memory} whose content is a
> > > list of tuples ...
> >
> > This is still not accurate. The output of the command is:
> >
> > - the character sequence "^done"
> > - zero, one, or more result results, separated with spaces.
> >
> > So, it's still an command output that "has" a result record named "memory"
> > as opposed to "result record .. is .. output of the command"
>
> This is a misunderstanding. In the text I suggested, "record that is
> output", the "output" part is a verb, not a noun. (I should have said
> "output by the command", though.) IOW, the text says that the command
> outputs some stuff, and part of that stuff is the result vector with a
> field named "memory".
>
> Saying "output has the result record" is not good English, because the
> result record is part of the output, not some attribute of it.
>
> I could try to be more accurate, if "that is output by the command" is
> not good enough, but please note that our current definition of what
> exactly is a "result record" is quite vague. In fact, we don't say
> what it is at all. For starters, it talks about "result indications":
Ouch. I don't think we have "result indications" anywhere else.
> In addition to a number of out-of-band notifications, the response to a
> @sc{gdb/mi} command includes one of the following result indications:
>
> @table @code
> @findex ^done
> @item "^done" [ "," @var{results} ]
> The synchronous operation was successful, @code{@var{results}} are the return
> values.
>
> @item "^running"
> @findex ^running
> This result record is equivalent to @samp{^done}. Historically, it
> was output instead of @samp{^done} if the command has resumed the
> target. This behaviour is maintained for backward compatibility, but
> all frontends should treat @samp{^done} and @samp{^running}
> identically and rely on the @samp{*running} output record to determine
> which threads are resumed.
>
> My reading of this is that ^done, ^running, etc. _are_ examples of
> result records. By contrast, you say above that a result record is
> everything _after_ these indications. This inconsistency is the main
> reason why I deliberately left the text I suggested slightly vague.
It looks like the grammar disagrees with me -- it say result record is
indeed the entire output, which has 'results'.
> I'm open to other suggestions, but if we want to be rigorous, we
> should be more consistent than we are now regarding what exactly is a
> result record.
Yeah, this is rather confusing, so I'll use your wording (with 'by')
I've checked in the below to CVS.
Thanks,
--
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722
[-- Attachment #2: bytes.diff --]
[-- Type: text/x-patch, Size: 20469 bytes --]
commit c03302295114d6efffd77b6342315b4040e10cac
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu Jun 17 20:23:17 2010 +0400
Easier and more stubborn MI memory read commands.
gdb/
* mi/mi-cmds.c (mi_cmds): Register data-read-memory-bytes
and data-write-memory-bytes.
* mi/mi-cmds.h (mi_cmd_data_read_memory_bytes)
(mi_cmd_data_write_memory_bytes): New.
* mi/mi-main.c (mi_cmd_data_read_memory): Use regular target_read.
(mi_cmd_data_read_memory_bytes, mi_cmd_data_write_memory_bytes):
New.
(mi_cmd_list_features): Add "data-read-memory-bytes" feature.
* target.c (target_read_until_error): Remove.
(read_whatever_is_readable, free_memory_read_result_vector)
(read_memory_robust): New.
* target.h (target_read_until_error): Remove.
(struct memory_read_result, free_memory_read_result_vector)
(read_memory_robust): New.
gdb/doc
* gdb.texinfo (GDB/MI Data Manipulation): Document
-data-read-memory-raw and -data-write-memory-raw.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index cbd636f..93008cb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26895,6 +26895,8 @@ don't appear in the actual output):
@subheading The @code{-data-read-memory} Command
@findex -data-read-memory
+This command is deprecated, use @code{-data-read-memory-bytes} instead.
+
@subsubheading Synopsis
@smallexample
@@ -27006,6 +27008,128 @@ next-page="0x000013c0",prev-page="0x00001380",memory=[
(gdb)
@end smallexample
+@subheading The @code{-data-read-memory-bytes} Command
+@findex -data-read-memory-bytes
+
+@subsubheading Synopsis
+
+@smallexample
+ -data-read-memory-bytes [ -o @var{byte-offset} ]
+ @var{address} @var{count}
+@end smallexample
+
+@noindent
+where:
+
+@table @samp
+@item @var{address}
+An expression specifying the address of the first memory word 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.
+
+@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.
+
+@end table
+
+This command attempts to read all accessible memory regions in the
+specified range. First, all regions marked as unreadable in the memory
+map (if one is defined) will be skipped. @xref{Memory Region
+Attributes}. Second, @value{GDBN} will attempt to read the remaining
+regions. For each one, if reading full region results in an errors,
+@value{GDBN} will try to read a subset of the region.
+
+In general, every single byte in the region may be readable or not,
+and the only way to read every readable byte is to try a read at
+every address, which is not practical. Therefore, @value{GDBN} will
+attempt to read all accessible bytes at either beginning or the end
+of the region, using a binary division scheme. This heuristic works
+well for reading accross a memory map boundary. Note that if a region
+has a readable range that is neither at the beginning or the end,
+@value{GDBN} will not read it.
+
+The result record (@pxref{GDB/MI Result Records}) that is output of
+the command includes a field named @samp{memory} whose content is a
+list of tuples. Each tuple represent a successfully read memory block
+and has the following fields:
+
+@table @code
+@item begin
+The start address of the memory block, as hexadecimal literal.
+
+@item end
+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}.
+
+@item contents
+The contents of the memory block, in hex.
+
+@end table
+
+
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{x}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-data-read-memory-bytes &a 10
+^done,memory=[@{begin="0xbffff154",offset="0x00000000",
+ end="0xbffff15e",
+ contents="01000000020000000300"@}]
+(gdb)
+@end smallexample
+
+
+@subheading The @code{-data-write-memory-bytes} Command
+@findex -data-write-memory-bytes
+
+@subsubheading Synopsis
+
+@smallexample
+ -data-write-memory-bytes @var{address} @var{contents}
+@end smallexample
+
+@noindent
+where:
+
+@table @samp
+@item @var{address}
+An expression specifying the address of the first memory word to be
+read. Complex expressions containing embedded white space should be
+quoted using the C convention.
+
+@item @var{contents}
+The hex-encoded bytes to write.
+
+@end table
+
+@subsubheading @value{GDBN} Command
+
+There's no corresponding @value{GDBN} command.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd"
+^done
+(gdb)
+@end smallexample
+
+
@c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@node GDB/MI Tracepoint Commands
@section @sc{gdb/mi} Tracepoint Commands
@@ -28344,6 +28468,9 @@ pretty-printing commands, and possible presence of the
@samp{display_hint} field in the output of @code{-var-list-children}
@item thread-info
Indicates presence of the @code{-thread-info} command.
+@item data-read-memory-bytes
+Indicates presense of the @code{-data-read-memory-bytes} and the
+@code{-data-write-memory-bytes} commands.
@end table
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 8441e17..a6a884f 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -51,7 +51,9 @@ struct mi_cmd mi_cmds[] =
{ "data-list-register-names", { NULL, 0 }, mi_cmd_data_list_register_names},
{ "data-list-register-values", { NULL, 0 }, mi_cmd_data_list_register_values},
{ "data-read-memory", { NULL, 0 }, mi_cmd_data_read_memory},
+ { "data-read-memory-bytes", { NULL, 0 }, mi_cmd_data_read_memory_bytes},
{ "data-write-memory", { NULL, 0 }, mi_cmd_data_write_memory},
+ { "data-write-memory-bytes", {NULL, 0}, mi_cmd_data_write_memory_bytes},
{ "data-write-register-values", { NULL, 0 }, mi_cmd_data_write_register_values},
{ "enable-timings", { NULL, 0 }, mi_cmd_enable_timings},
{ "enable-pretty-printing", { NULL, 0 }, mi_cmd_enable_pretty_printing},
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 5954aef..b357de6 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -47,7 +47,9 @@ extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;
extern mi_cmd_argv_ftype mi_cmd_data_list_register_values;
extern mi_cmd_argv_ftype mi_cmd_data_list_changed_registers;
extern mi_cmd_argv_ftype mi_cmd_data_read_memory;
+extern mi_cmd_argv_ftype mi_cmd_data_read_memory_bytes;
extern mi_cmd_argv_ftype mi_cmd_data_write_memory;
+extern mi_cmd_argv_ftype mi_cmd_data_write_memory_bytes;
extern mi_cmd_argv_ftype mi_cmd_data_write_register_values;
extern mi_cmd_argv_ftype mi_cmd_enable_timings;
extern mi_cmd_argv_ftype mi_cmd_env_cd;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 85a3f99..c91c860 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1352,9 +1352,9 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
/* Dispatch memory reads to the topmost target, not the flattened
current_target. */
- nr_bytes = target_read_until_error (current_target.beneath,
- TARGET_OBJECT_MEMORY, NULL, mbuf,
- addr, total_bytes);
+ nr_bytes = target_read (current_target.beneath,
+ TARGET_OBJECT_MEMORY, NULL, mbuf,
+ addr, total_bytes);
if (nr_bytes <= 0)
error ("Unable to read memory.");
@@ -1437,6 +1437,88 @@ mi_cmd_data_read_memory (char *command, char **argv, int argc)
do_cleanups (cleanups);
}
+void
+mi_cmd_data_read_memory_bytes (char *command, char **argv, int argc)
+{
+ struct gdbarch *gdbarch = get_current_arch ();
+ struct cleanup *cleanups;
+ CORE_ADDR addr;
+ LONGEST length;
+ memory_read_result_s *read_result;
+ int ix;
+ VEC(memory_read_result_s) *result;
+ long offset = 0;
+ int optind = 0;
+ char *optarg;
+ enum opt
+ {
+ OFFSET_OPT
+ };
+ static struct mi_opt opts[] =
+ {
+ {"o", OFFSET_OPT, 1},
+ { 0, 0, 0 }
+ };
+
+ while (1)
+ {
+ int opt = mi_getopt ("mi_cmd_data_read_memory_bytes", argc, argv, opts,
+ &optind, &optarg);
+ if (opt < 0)
+ break;
+ switch ((enum opt) opt)
+ {
+ case OFFSET_OPT:
+ offset = atol (optarg);
+ break;
+ }
+ }
+ argv += optind;
+ argc -= optind;
+
+ if (argc != 2)
+ error ("Usage: [ -o OFFSET ] ADDR LENGTH.");
+
+ addr = parse_and_eval_address (argv[0]) + offset;
+ length = atol (argv[1]);
+
+ result = read_memory_robust (current_target.beneath, addr, length);
+
+ cleanups = make_cleanup (free_memory_read_result_vector, result);
+
+ if (VEC_length (memory_read_result_s, result) == 0)
+ error ("Unable to read memory.");
+
+ make_cleanup_ui_out_list_begin_end (uiout, "memory");
+ for (ix = 0;
+ VEC_iterate (memory_read_result_s, result, ix, read_result);
+ ++ix)
+ {
+ struct cleanup *t = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+ char *data, *p;
+ int i;
+
+ ui_out_field_core_addr (uiout, "begin", gdbarch, read_result->begin);
+ ui_out_field_core_addr (uiout, "offset", gdbarch, read_result->begin
+ - addr);
+ ui_out_field_core_addr (uiout, "end", gdbarch, read_result->end);
+
+ data = xmalloc ((read_result->end - read_result->begin) * 2 + 1);
+
+ for (i = 0, p = data;
+ i < (read_result->end - read_result->begin);
+ ++i, p += 2)
+ {
+ sprintf (p, "%02x", read_result->data[i]);
+ }
+ ui_out_field_string (uiout, "contents", data);
+ xfree (data);
+ do_cleanups (t);
+ }
+ do_cleanups (cleanups);
+}
+
+
/* DATA-MEMORY-WRITE:
COLUMN_OFFSET: optional argument. Must be preceeded by '-o'. The
@@ -1523,6 +1605,44 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
do_cleanups (old_chain);
}
+/* DATA-MEMORY-WRITE-RAW:
+
+ ADDR: start address
+ DATA: string of bytes to write at that address. */
+void
+mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
+{
+ CORE_ADDR addr;
+ char *cdata;
+ gdb_byte *data;
+ int len, r, i;
+ struct cleanup *back_to;
+
+ if (argc != 2)
+ error ("Usage: ADDR DATA.");
+
+ addr = parse_and_eval_address (argv[0]);
+ cdata = argv[1];
+ len = strlen (cdata)/2;
+
+ data = xmalloc (len);
+ back_to = make_cleanup (xfree, data);
+
+ for (i = 0; i < len; ++i)
+ {
+ int x;
+ sscanf (cdata + i * 2, "%02x", &x);
+ data[i] = (gdb_byte)x;
+ }
+
+ r = target_write_memory (addr, data, len);
+ if (r != 0)
+ error (_("Could not write memory"));
+
+ do_cleanups (back_to);
+}
+
+
void
mi_cmd_enable_timings (char *command, char **argv, int argc)
{
@@ -1557,6 +1677,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
ui_out_field_string (uiout, NULL, "frozen-varobjs");
ui_out_field_string (uiout, NULL, "pending-breakpoints");
ui_out_field_string (uiout, NULL, "thread-info");
+ ui_out_field_string (uiout, NULL, "data-read-memory-bytes");
#if HAVE_PYTHON
ui_out_field_string (uiout, NULL, "python");
diff --git a/gdb/target.c b/gdb/target.c
index 7aa77e6..cf01b6c 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1751,73 +1751,204 @@ target_read (struct target_ops *ops,
return len;
}
-LONGEST
-target_read_until_error (struct target_ops *ops,
- enum target_object object,
- const char *annex, gdb_byte *buf,
- ULONGEST offset, LONGEST len)
+/** Assuming that the entire [begin, end) range of memory cannot be read,
+ try to read whatever subrange is possible to read.
+
+ The function results, in RESULT, either zero or one memory block.
+ If there's a readable subrange at the beginning, it is completely
+ read and returned. Any further readable subrange will not be read.
+ Otherwise, if there's a readable subrange at the end, it will be
+ completely read and returned. Any readable subranges before it (obviously,
+ not starting at the beginning), will be ignored. In other cases --
+ either no readable subrange, or readable subrange (s) that is neither
+ at the beginning, or end, nothing is returned.
+
+ The purpose of this function is to handle a read across a boundary of
+ accessible memory in a case when memory map is not available. The above
+ restrictions are fine for this case, but will give incorrect results if
+ the memory is 'patchy'. However, supporting 'patchy' memory would require
+ trying to read every single byte, and it seems unacceptable solution.
+ Explicit memory map is recommended for this case -- and
+ target_read_memory_robust will take care of reading multiple ranges then. */
+
+static void
+read_whatever_is_readable (struct target_ops *ops, ULONGEST begin, ULONGEST end,
+ VEC(memory_read_result_s) **result)
{
- LONGEST xfered = 0;
+ gdb_byte *buf = xmalloc (end-begin);
+ ULONGEST current_begin = begin;
+ ULONGEST current_end = end;
+ int forward;
+ memory_read_result_s r;
+
+ /* If we previously failed to read 1 byte, nothing can be done here. */
+ if (end - begin <= 1)
+ return;
+
+ /* Check that either first or the last byte is readable, and give up
+ if not. This heuristic is meant to permit reading accessible memory
+ at the boundary of accessible region. */
+ if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf, begin, 1) == 1)
+ {
+ forward = 1;
+ ++current_begin;
+ }
+ else if (target_read_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf + (end-begin) - 1, end - 1, 1) == 1)
+ {
+ forward = 0;
+ --current_end;
+ }
+ else
+ {
+ return;
+ }
+
+ /* Loop invariant is that the [current_begin, current_end) was previously
+ found to be not readable as a whole.
+
+ Note loop condition -- if the range has 1 byte, we can't divide the range
+ so there's no point trying further. */
+ while (current_end - current_begin > 1)
+ {
+ 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;
+ if (forward)
+ {
+ first_half_begin = current_begin;
+ first_half_end = middle;
+ second_half_begin = middle;
+ second_half_end = current_end;
+ }
+ else
+ {
+ first_half_begin = middle;
+ first_half_end = current_end;
+ second_half_begin = current_begin;
+ second_half_end = middle;
+ }
+
+ xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ buf + (first_half_begin - begin),
+ first_half_begin,
+ first_half_end - first_half_begin);
+
+ if (xfer == first_half_end - first_half_begin)
+ {
+ /* This half reads up fine. So, the error must be in the other half. */
+ current_begin = second_half_begin;
+ current_end = second_half_end;
+ }
+ 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
+ iteration to divide again and try to read.
+
+ We don't handle the other half, because this function only tries
+ to read a single readable subrange. */
+ current_begin = first_half_begin;
+ current_end = first_half_end;
+ }
+ }
+
+ if (forward)
+ {
+ /* The [begin, current_begin) range has been read. */
+ r.begin = begin;
+ r.end = current_begin;
+ r.data = buf;
+ }
+ else
+ {
+ /* The [current_end, end) range has been read. */
+ LONGEST rlen = end - current_end;
+ r.data = xmalloc (rlen);
+ memcpy (r.data, buf + current_end - begin, rlen);
+ r.begin = current_end;
+ r.end = end;
+ xfree (buf);
+ }
+ VEC_safe_push(memory_read_result_s, (*result), &r);
+}
+
+void
+free_memory_read_result_vector (void *x)
+{
+ VEC(memory_read_result_s) *v = x;
+ memory_read_result_s *current;
+ int ix;
+
+ for (ix = 0; VEC_iterate (memory_read_result_s, v, ix, current); ++ix)
+ {
+ xfree (current->data);
+ }
+ VEC_free (memory_read_result_s, v);
+}
+VEC(memory_read_result_s) *
+read_memory_robust (struct target_ops *ops, ULONGEST offset, LONGEST len)
+{
+ VEC(memory_read_result_s) *result = 0;
+
+ LONGEST xfered = 0;
while (xfered < len)
{
- LONGEST xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- offset + xfered, len - xfered);
+ struct mem_region *region = lookup_mem_region (offset + xfered);
+ LONGEST rlen;
- /* Call an observer, notifying them of the xfer progress? */
- if (xfer == 0)
- return xfered;
- if (xfer < 0)
+ /* If there is no explicit region, a fake one should be created. */
+ gdb_assert (region);
+
+ if (region->hi == 0)
+ rlen = len - xfered;
+ else
+ rlen = region->hi - offset;
+
+ if (region->attrib.mode == MEM_NONE || region->attrib.mode == MEM_WO)
{
- /* We've got an error. Try to read in smaller blocks. */
- ULONGEST start = offset + xfered;
- ULONGEST remaining = len - xfered;
- ULONGEST half;
-
- /* If an attempt was made to read a random memory address,
- it's likely that the very first byte is not accessible.
- Try reading the first byte, to avoid doing log N tries
- below. */
- xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered, start, 1);
+ /* 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;
+ }
+ else
+ {
+ LONGEST to_read = min (len - xfered, rlen);
+ gdb_byte *buffer = (gdb_byte *)xmalloc (to_read);
+
+ LONGEST xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
+ (gdb_byte *) buffer,
+ offset + xfered, to_read);
+ /* Call an observer, notifying them of the xfer progress? */
if (xfer <= 0)
- return xfered;
- start += 1;
- remaining -= 1;
- half = remaining/2;
-
- while (half > 0)
{
- xfer = target_read_partial (ops, object, annex,
- (gdb_byte *) buf + xfered,
- start, half);
- if (xfer == 0)
- return xfered;
- if (xfer < 0)
- {
- remaining = half;
- }
- else
- {
- /* We have successfully read the first half. So, the
- error must be in the second half. Adjust start and
- remaining to point at the second half. */
- xfered += xfer;
- start += xfer;
- remaining -= xfer;
- }
- half = remaining/2;
+ /* 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;
}
-
- return xfered;
+ else
+ {
+ struct memory_read_result r;
+ r.data = buffer;
+ r.begin = offset + xfered;
+ r.end = r.begin + xfer;
+ VEC_safe_push (memory_read_result_s, result, &r);
+ xfered += xfer;
+ }
+ QUIT;
}
- xfered += xfer;
- QUIT;
}
- return len;
+ return result;
}
+
/* An alternative to target_write with progress callbacks. */
LONGEST
diff --git a/gdb/target.h b/gdb/target.h
index 870a1eb..e0a63ca 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -292,10 +292,23 @@ extern LONGEST target_read (struct target_ops *ops,
const char *annex, gdb_byte *buf,
ULONGEST offset, LONGEST len);
-extern LONGEST target_read_until_error (struct target_ops *ops,
- enum target_object object,
- const char *annex, gdb_byte *buf,
- ULONGEST offset, LONGEST len);
+struct memory_read_result
+ {
+ /* First address that was read. */
+ ULONGEST begin;
+ /* Past-the-end address. */
+ ULONGEST end;
+ /* The data. */
+ gdb_byte *data;
+};
+typedef struct memory_read_result memory_read_result_s;
+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);
extern LONGEST target_write (struct target_ops *ops,
enum target_object object,
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-08-13 13:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-25 8:33 Better MI memory commands Vladimir Prus
2010-06-25 16:05 ` Eli Zaretskii
2010-08-11 12:38 ` Vladimir Prus
2010-08-11 18:00 ` Eli Zaretskii
2010-08-12 7:25 ` Vladimir Prus
2010-08-12 17:04 ` Eli Zaretskii
2010-08-13 13:23 ` Vladimir Prus
2010-07-07 16:30 ` Daniel Jacobowitz
2010-07-09 18:54 ` Vladimir Prus
2010-07-09 18:59 ` Daniel Jacobowitz
2010-08-11 12:38 ` Vladimir Prus
2010-08-11 21:10 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).