This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Include access size in find command error message
Hi Andrew,
Thanks for doing this. It looks all good to me. Minor comments below.
On 09/26/2012 01:28 PM, Andrew Burgess wrote:
> gdb/gdbserver/ChangeLog
>
> 2012-09-26 Andrew Burgess <aburgess@broadcom.com>
>
> * server.c (handle_search_memory_1): Include access length in error
> message.
Pedantically, it's a warning message, not an error though.
This code should be using the proper paddress/pulongest/%u,etc. for the
types instead of the (long) casts, but it's precedent, so subject for
a separate fix.
> gdb/ChangeLog
>
> 2012-09-26 Andrew Burgess <aburgess@broadcom.com>
>
> * target.c (simple_search_memory): Include access length in error
> message.
Ditto.
>
> diff --git a/gdb/target.c b/gdb/target.c
> index 1fc8802..cea3d10 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -2874,8 +2874,9 @@ simple_search_memory (struct target_ops *ops,
> if (target_read (ops, TARGET_OBJECT_MEMORY, NULL,
> search_buf, start_addr, search_buf_size) != search_buf_size)
> {
> - warning (_("Unable to access target memory at %s, halting search."),
> - hex_string (start_addr));
> + warning (_("Unable to access %s bytes of target "
> + "memory at %s, halting search."),
> + plongest (search_buf_size), hex_string (start_addr));
> do_cleanups (old_cleanups);
> return -1;
> }
> @@ -2928,8 +2929,9 @@ simple_search_memory (struct target_ops *ops,
> search_buf + keep_len, read_addr,
> nr_to_read) != nr_to_read)
> {
> - warning (_("Unable to access target "
> + warning (_("Unable to access %s bytes of target "
> "memory at %s, halting search."),
> + plongest (nr_to_read),
> hex_string (read_addr));
> do_cleanups (old_cleanups);
> return -1;
>
Pedantically, neither search_buf_size nor nr_to_read are
of type LONGEST, but it doesn't really matter. Just mentioning
to be thorough.
> gdb/testsuite/ChangeLog
>
> 2012-09-26 Andrew Burgess <aburgess@broadcom.com>
>
> Test find command on unmapped memory.
> * gdb.base/find-unmapped.c: New file.
> * gdb.base/find-unmapped.exp: New file.
>
> diff --git a/gdb/testsuite/gdb.base/find-unmapped.c b/gdb/testsuite/gdb.base/find-unmapped.c
> new file mode 100644
> index 0000000..70059b1
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/find-unmapped.c
> @@ -0,0 +1,89 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2012 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#define CHUNK_SIZE 16000 /* same as findcmd.c's */
> +
> +void *global_var_0;
> +void *global_var_1;
> +void *global_var_2;
> +
> +void
> +breakpt ()
> +{
> + /* Nothing. */
> +}
> +
> +int
> +main (void)
> +{
> + void *p;
> + size_t pg_size;
> + int pg_count;
> + void *unmapped_page, *last_mapped_page, *first_mapped_page;
> +
> + /*
> + Map enough pages to cover at least CHUNK_SIZE, and one extra page. We
> + then unmap the last page.
> +
> + From gdb we can then perform find commands into unmapped region, gdb
> + should given an error.
> +
> + .----.----.----.----.----.
> + | | | | | |
> + '----'----'----'----'----'
> + |<- CHUNK_SIZE ->|
> + */
> +
> + pg_size = getpagesize ();
> + /* The +2 gives the extra page. */
> + pg_count = (CHUNK_SIZE / pg_size) + 2;
Will this all still work if pg_size is > than CHUNK_SIZE?
> +
> + p = mmap (0, (pg_count * pg_size), PROT_READ|PROT_WRITE,
> + MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> + if (p == MAP_FAILED)
> + {
> + perror ("mmap");
> + return EXIT_FAILURE;
> + }
> +
> + memset (p, 0, (pg_count *pg_size));
Space after * missing.
> +
> + if (munmap (p + ((pg_count - 1) * pg_size), pg_size) == -1)
> + {
> + perror ("munmap");
> + return EXIT_FAILURE;
> + }
> +
> + first_mapped_page = p;
> + last_mapped_page = p + ((pg_count - 2) * pg_size);
> + unmapped_page = last_mapped_page + pg_size;
Unnecessary parens in several places (around * and /).
> +
> + /* Setup global variables we reference from gdb. */
> + global_var_0 = first_mapped_page;
> + global_var_1 = unmapped_page - 16;
> + global_var_2 = unmapped_page + 16;
> +
> + breakpt ();
> +
> + return EXIT_SUCCESS;
> +}
> diff --git a/gdb/testsuite/gdb.base/find-unmapped.exp b/gdb/testsuite/gdb.base/find-unmapped.exp
> new file mode 100644
> index 0000000..341538b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/find-unmapped.exp
> @@ -0,0 +1,48 @@
> +# Copyright 2012 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +if {[is_remote target]} {
> + # gdbserver prints the warning message but expect is parsing only the
> + # GDB output, not the gdbserver output.
> + return 0
> +}
> +
> +standard_testfile
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile}] } {
> + return -1
> +}
> +
> +if ![runto breakpt] {
> + return -1
> +}
> +
> +# Basic attempt to read memory from globals.
> +gdb_test "x/5w global_var_1" \
> + "$hex:\[ \t\]+0\[ \t\]+0\[ \t\]+0\[ \t\]+0\r\n$hex:\[ \t\]+Cannot access memory at address $hex"
> +gdb_test "x/5w global_var_2" \
> + "$hex:\[ \t\]+Cannot access memory at address $hex"
> +
> +# Now try a find starting from each global. The warning message is made
> +# optional here as for gdbserver targets the warning is printed by
> +# gdbserver, but expect is only collecting gdb output.
It looks like the second sentence is stale, given the test is skipped
against remote targets.
> +gdb_test "find global_var_1, (global_var_2 + 16), 0xff" \
> + "warning: Unable to access $decimal bytes of target memory at $hex, halting search\.\r\nPattern not found."
> +
> +gdb_test "find global_var_2, (global_var_2 + 16), 0xff" \
> + "warning: Unable to access $decimal bytes of target memory at $hex, halting search\.\r\nPattern not found."
> +
> +gdb_test "find global_var_0, (global_var_2 + 16), 0xff" \
> + "warning: Unable to access $decimal bytes of target memory at $hex, halting search\.\r\nPattern not found."
--
Pedro Alves