From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22913 invoked by alias); 11 Aug 2010 12:38:10 -0000 Received: (qmail 22857 invoked by uid 22791); 11 Aug 2010 12:38:02 -0000 X-SWARE-Spam-Status: No, hits=-0.7 required=5.0 tests=AWL,BAYES_50,TW_CP,TW_RG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 11 Aug 2010 12:37:53 +0000 Received: (qmail 13212 invoked from network); 11 Aug 2010 12:37:51 -0000 Received: from unknown (HELO wind.localnet) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 11 Aug 2010 12:37:51 -0000 From: Vladimir Prus To: Eli Zaretskii Subject: Re: Better MI memory commands Date: Wed, 11 Aug 2010 12:38:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-22-generic-pae; KDE/4.3.2; i686; ; ) Cc: gdb-patches@sources.redhat.com References: <201006251232.55281.vladimir@codesourcery.com> <837hlndrni.fsf@gnu.org> In-Reply-To: <837hlndrni.fsf@gnu.org> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_dmpYMgFlT27BfAe" Message-Id: <201008111637.49621.vladimir@codesourcery.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-08/txt/msg00140.txt.bz2 --Boundary-00=_dmpYMgFlT27BfAe Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-length: 3792 On Friday 25 June 2010 20:05:21 Eli Zaretskii wrote: > > From: Vladimir Prus > > 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 --Boundary-00=_dmpYMgFlT27BfAe Content-Type: text/x-patch; charset="UTF-8"; name="data-memory-bytes.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="data-memory-bytes.diff" Content-length: 19648 commit 97fb083bbb395f6bb9e79f5f7dba6d1caeb9e8b2 Author: Vladimir Prus 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, --Boundary-00=_dmpYMgFlT27BfAe--