Check memory accesses

Check memory accesses

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]


This patch makes gdb, when target memory map is available, optionally reject 
all memory accesses outside of regions in memory map. The rationale is that:

	1. It's sometimes better to get error that have the write silently do
        nothing or read returning garbase.

	2. For some targets, accesses to non-existent memory "wedge" them.

Since memory maps are still rare, this behaviour is enabled only by a new 
command.

OK?

- Volodya

	* memattr.h (enum mem_access_mode): New value
	MEM_NONE.
	* memattr.c (unknown_mem_attrib): New.
	(inaccessible_by_default): New.
	(show_inaccessible_by_default): New.
	(lookup_mem_region): Check inaccessible_by_default.
	(dummy_cmd): New.
	(mem_set_cmdlist, mem_show_cmdlist): New.
	(_initialize_mem): Register new "set" and "show"
	commands.
	* target.c (memory_xfer_partial): If memory type
	is MEM_NONE, return an error.
	Clip to region size when calling to_xfer_partial.
	If upper limit of memory range is 0, don't clip
	anything.

[-- Attachment #2: memcheck.diff --]
[-- Type: text/x-diff, Size: 5815 bytes --]

Index: memattr.c
===================================================================
RCS file: /cvs/src/src/gdb/memattr.c,v
retrieving revision 1.25
diff -u -r1.25 memattr.c
--- memattr.c	21 Sep 2006 14:00:53 -0000	1.25
+++ memattr.c	9 Nov 2006 20:28:25 -0000
@@ -29,6 +29,7 @@
 #include "language.h"
 #include "vec.h"
 #include "gdb_string.h"
+#include "gdb_locale.h"
 
 const struct mem_attrib default_mem_attrib =
 {
@@ -40,6 +41,16 @@
   -1 /* Flash blocksize not specified.  */
 };
 
+const struct mem_attrib unknown_mem_attrib =
+{
+  MEM_NONE,			/* mode */
+  MEM_WIDTH_UNSPECIFIED,
+  0,				/* hwbreak */
+  0,				/* cache */
+  0,				/* verify */
+  -1 /* Flash blocksize not specified.  */
+};
+
 VEC(mem_region_s) *mem_region_list, *target_mem_region_list;
 static int mem_number = 0;
 
@@ -53,6 +64,25 @@
    empty, then the target can't supply memory regions.  */
 static int target_mem_regions_valid;
 
+/* If this flag is set, gdb will assume that memory ranges not
+   specified by the memory map have type MEM_NONE, and will
+   emit errors on all accesses to that memory.  */
+static int inaccessible_by_default = 0;
+
+static void
+show_inaccessible_by_default (struct ui_file *file, int from_tty,
+			      struct cmd_list_element *c,
+			      const char *value)
+{
+  if (inaccessible_by_default)
+    fprintf_filtered (file, _("\
+Unknown memory addresses will be treated as inaccessible.\n"));
+  else
+    fprintf_filtered (file, _("\
+Unknown memory addresses will be treated as RAM.\n"));          
+}
+
+
 /* Predicate function which returns true if LHS should sort before RHS
    in a list of memory regions, useful for VEC_lower_bound.  */
 
@@ -215,13 +245,17 @@
   lo = 0;
   hi = 0;
 
-  /* If we ever want to support a huge list of memory regions, this
+  /* Either find memory range containing ADDRESS, or set LO and HI
+     to the nearest boundaries of an existing memory range.
+     
+     If we ever want to support a huge list of memory regions, this
      check should be replaced with a binary search (probably using
      VEC_lower_bound).  */
   for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
     {
       if (m->enabled_p == 1)
 	{
+	  /* If address is in memory return, return that memory range.  */
 	  if (addr >= m->lo && (addr < m->hi || m->hi == 0))
 	    return m;
 
@@ -243,7 +277,15 @@
      was learned above.  */
   region.lo = lo;
   region.hi = hi;
-  region.attrib = default_mem_attrib;
+
+  /* When no memory map is defined at all, we always set 'default_mem_attrib',
+     so that we do not make the all memory inaccessible for targets that don't
+     provide a memory map.  */
+  if (inaccessible_by_default && !VEC_empty (mem_region_s, mem_region_list))
+    region.attrib = unknown_mem_attrib;
+  else
+    region.attrib = default_mem_attrib;
+
   return &region;
 }
 
@@ -674,9 +716,17 @@
 
   dont_repeat ();
 }
+
+static void
+dummy_cmd (char *args, int from_tty)
+{
+}
 \f
 extern initialize_file_ftype _initialize_mem; /* -Wmissing-prototype */
 
+static struct cmd_list_element *mem_set_cmdlist;
+static struct cmd_list_element *mem_show_cmdlist;
+
 void
 _initialize_mem (void)
 {
@@ -709,4 +759,25 @@
 
   add_info ("mem", mem_info_command,
 	    _("Memory region attributes"));
+
+  add_prefix_cmd ("mem", class_vars, dummy_cmd, _("\
+Memory regions settings"),
+		  &mem_set_cmdlist, "set mem ",
+		  0/* allow-unknown */, &setlist);
+  add_prefix_cmd ("mem", class_vars, dummy_cmd, _("\
+Memory regions settings"),
+		  &mem_show_cmdlist, "show mem  ",
+		  0/* allow-unknown */, &showlist);
+
+  add_setshow_boolean_cmd ("inaccessible-by-default", no_class,
+				  &inaccessible_by_default, _("\
+Set handling of unknown memory regions."), _("\
+Show handling of unknown memory regions."), _("\
+If on, and some memory map is defined, debugger will emit errors on\n\
+accesses to memory not defined in the memory map. If off, accesses to all\n\
+memory addresses will be allowed."),
+				NULL,
+				show_inaccessible_by_default,
+				&mem_set_cmdlist,
+				&mem_show_cmdlist);
 }
Index: memattr.h
===================================================================
RCS file: /cvs/src/src/gdb/memattr.h,v
retrieving revision 1.7
diff -u -r1.7 memattr.h
--- memattr.h	21 Sep 2006 13:54:02 -0000	1.7
+++ memattr.h	9 Nov 2006 20:28:25 -0000
@@ -26,6 +26,7 @@
 
 enum mem_access_mode
 {
+  MEM_NONE,                     /* Memory that is not physically present. */
   MEM_RW,			/* read/write */
   MEM_RO,			/* read only */
   MEM_WO,			/* write only */
@@ -76,7 +77,10 @@
 
 struct mem_region 
 {
+  /* Lowest address in the region.  */
   CORE_ADDR lo;
+  /* Address past the highest address of the region. 
+     If 0, upper bound is "infinity".  */
   CORE_ADDR hi;
 
   /* Item number of this memory region. */
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.128
diff -u -r1.128 target.c
--- target.c	17 Oct 2006 21:55:23 -0000	1.128
+++ target.c	9 Nov 2006 20:28:26 -0000
@@ -1015,7 +1015,8 @@
 
   /* Try GDB's internal data cache.  */
   region = lookup_mem_region (memaddr);
-  if (memaddr + len < region->hi)
+  /* region->hi means there's no upper bound. */
+  if (memaddr + len < region->hi || region->hi == 0)
     reg_len = len;
   else
     reg_len = region->hi - memaddr;
@@ -1037,6 +1038,9 @@
       if (writebuf != NULL)
 	error (_("Writing to flash memory forbidden in this context"));
       break;
+
+    case MEM_NONE:
+      return -1;
     }
 
   if (region->attrib.cache)
@@ -1072,7 +1076,7 @@
   do
     {
       res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
-				  readbuf, writebuf, memaddr, len);
+				  readbuf, writebuf, memaddr, reg_len);
       if (res > 0)
 	return res;
 

Re: Check memory accesses

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Thu, 9 Nov 2006 23:36:22 +0300
> 
> This patch makes gdb, when target memory map is available, optionally reject 
> all memory accesses outside of regions in memory map. The rationale is that:
> 
> 	1. It's sometimes better to get error that have the write silently do
>         nothing or read returning garbase.
> 
> 	2. For some targets, accesses to non-existent memory "wedge" them.
> 
> Since memory maps are still rare, this behaviour is enabled only by a new 
> command.
> 
> OK?

This patch introduces a new user command, but doesn't include a patch
for the user manual.  Please provide one.

Re: Check memory accesses

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

Eli Zaretskii wrote:

>> From: Vladimir Prus <vladimir@codesourcery.com>
>> Date: Thu, 9 Nov 2006 23:36:22 +0300
>> 
>> This patch makes gdb, when target memory map is available, optionally
>> reject all memory accesses outside of regions in memory map. The
>> rationale is that:
>> 
>> 1. It's sometimes better to get error that have the write silently do
>>         nothing or read returning garbase.
>> 
>> 2. For some targets, accesses to non-existent memory "wedge" them.
>> 
>> Since memory maps are still rare, this behaviour is enabled only by a new
>> command.
>> 
>> OK?
> 
> This patch introduces a new user command, but doesn't include a patch
> for the user manual.  Please provide one.

Sorry. This version of the patch includes documentation.

- Volodya

        gdb/
        * memattr.h (enum mem_access_mode): New value
        MEM_NONE.
        * memattr.c (unknown_mem_attrib): New.
        (inaccessible_by_default): New.
        (show_inaccessible_by_default): New.
        (lookup_mem_region): Check inaccessible_by_default.
        (dummy_cmd): New.
        (mem_set_cmdlist, mem_show_cmdlist): New.
        (_initialize_mem): Register new "set" and "show"
        commands.
        * target.c (memory_xfer_partial): If memory type
        is MEM_NONE, return an error.
        Clip to region size when calling to_xfer_partial.
        If upper limit of memory range is 0, don't clip
        anything.

        gdb/doc/
        * gdb.texinfo (Memory Access Checking): New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: memcheck__gdb_mainline.diff --]
[-- Type: text/x-diff; name="memcheck__gdb_mainline.diff", Size: 7141 bytes --]

=== gdb/doc/gdb.texinfo
==================================================================
--- gdb/doc/gdb.texinfo	(/mirrors/gdb_mainline)	(revision 2094)
+++ gdb/doc/gdb.texinfo	(/patches/gdb/memcheck/gdb_mainline)	(revision 2094)
@@ -6894,6 +6894,26 @@
 Disable @value{GDBN} from caching target memory.  This is the default.
 @end table
 
+@subsection Memory Access Checking
+@value{GDBN} can be instructed to refuse accesses to memory that is
+not explicitly described.  This can be useful if accessing such
+regions has undesired effects for a specific target, or to provide
+better error checking.  The following commands control this behaviour.
+
+@table @code
+@kindex set mem inaccessible-by-default
+@item set mem inaccessible-by-default [on|off]
+If @code{on} is specified, make  @value{GDBN} treat memory not
+explicitly described by the memory ranges as non-existent and refuse accesses
+to such memory.  The checks are only performed if there's at least one
+memory range defined.  If @code{off} is specified, make @value{GDBN}
+treat the memory not explicitly described by the memory ranges as RAM.
+@kindex show mem inaccessible-by-default
+@item show mem inaccessible-by-default
+Show the current handling of accesses to unknown memory.
+@end table
+
+
 @c @subsubsection Memory Write Verification
 @c The memory write verification attributes set whether @value{GDBN}
 @c will re-reads data after each write to verify the write was successful.
=== gdb/target.c
==================================================================
--- gdb/target.c	(/mirrors/gdb_mainline)	(revision 2094)
+++ gdb/target.c	(/patches/gdb/memcheck/gdb_mainline)	(revision 2094)
@@ -1015,7 +1015,8 @@
 
   /* Try GDB's internal data cache.  */
   region = lookup_mem_region (memaddr);
-  if (memaddr + len < region->hi)
+  /* region->hi means there's no upper bound. */
+  if (memaddr + len < region->hi || region->hi == 0)
     reg_len = len;
   else
     reg_len = region->hi - memaddr;
@@ -1037,6 +1038,9 @@
       if (writebuf != NULL)
 	error (_("Writing to flash memory forbidden in this context"));
       break;
+
+    case MEM_NONE:
+      return -1;
     }
 
   if (region->attrib.cache)
@@ -1072,7 +1076,7 @@
   do
     {
       res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
-				  readbuf, writebuf, memaddr, len);
+				  readbuf, writebuf, memaddr, reg_len);
       if (res > 0)
 	return res;
 
=== gdb/memattr.c
==================================================================
--- gdb/memattr.c	(/mirrors/gdb_mainline)	(revision 2094)
+++ gdb/memattr.c	(/patches/gdb/memcheck/gdb_mainline)	(revision 2094)
@@ -29,6 +29,7 @@
 #include "language.h"
 #include "vec.h"
 #include "gdb_string.h"
+#include "gdb_locale.h"
 
 const struct mem_attrib default_mem_attrib =
 {
@@ -40,6 +41,17 @@
   -1 /* Flash blocksize not specified.  */
 };
 
+const struct mem_attrib unknown_mem_attrib =
+{
+  MEM_NONE,			/* mode */
+  MEM_WIDTH_UNSPECIFIED,
+  0,				/* hwbreak */
+  0,				/* cache */
+  0,				/* verify */
+  -1 /* Flash blocksize not specified.  */
+};
+
+
 VEC(mem_region_s) *mem_region_list, *target_mem_region_list;
 static int mem_number = 0;
 
@@ -53,6 +65,25 @@
    empty, then the target can't supply memory regions.  */
 static int target_mem_regions_valid;
 
+/* If this flag is set, gdb will assume that memory ranges not
+   specified by the memory map have type MEM_NONE, and will
+   emit errors on all accesses to that memory.  */
+static int inaccessible_by_default = 0;
+
+static void
+show_inaccessible_by_default (struct ui_file *file, int from_tty,
+			      struct cmd_list_element *c,
+			      const char *value)
+{
+  if (inaccessible_by_default)
+    fprintf_filtered (file, _("\
+Unknown memory addresses will be treated as inaccessible.\n"));
+  else
+    fprintf_filtered (file, _("\
+Unknown memory addresses will be treated as RAM.\n"));          
+}
+
+
 /* Predicate function which returns true if LHS should sort before RHS
    in a list of memory regions, useful for VEC_lower_bound.  */
 
@@ -215,13 +246,17 @@
   lo = 0;
   hi = 0;
 
-  /* If we ever want to support a huge list of memory regions, this
+  /* Either find memory range containing ADDRESS, or set LO and HI
+     to the nearest boundaries of an existing memory range.
+     
+     If we ever want to support a huge list of memory regions, this
      check should be replaced with a binary search (probably using
      VEC_lower_bound).  */
   for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
     {
       if (m->enabled_p == 1)
 	{
+	  /* If address is in memory return, return that memory range.  */
 	  if (addr >= m->lo && (addr < m->hi || m->hi == 0))
 	    return m;
 
@@ -243,7 +278,15 @@
      was learned above.  */
   region.lo = lo;
   region.hi = hi;
-  region.attrib = default_mem_attrib;
+
+  /* When no memory map is defined at all, we always set 'default_mem_attrib',
+     so that we do not make the all memory inaccessible for targets that don't
+     provide a memory map.  */
+  if (inaccessible_by_default && !VEC_empty (mem_region_s, mem_region_list))
+    region.attrib = unknown_mem_attrib;
+  else
+    region.attrib = default_mem_attrib;
+
   return &region;
 }
 
@@ -674,9 +717,17 @@
 
   dont_repeat ();
 }
+
+static void
+dummy_cmd (char *args, int from_tty)
+{
+}
 \f
 extern initialize_file_ftype _initialize_mem; /* -Wmissing-prototype */
 
+static struct cmd_list_element *mem_set_cmdlist;
+static struct cmd_list_element *mem_show_cmdlist;
+
 void
 _initialize_mem (void)
 {
@@ -709,4 +760,25 @@
 
   add_info ("mem", mem_info_command,
 	    _("Memory region attributes"));
+
+  add_prefix_cmd ("mem", class_vars, dummy_cmd, _("\
+Memory regions settings"),
+		  &mem_set_cmdlist, "set mem ",
+		  0/* allow-unknown */, &setlist);
+  add_prefix_cmd ("mem", class_vars, dummy_cmd, _("\
+Memory regions settings"),
+		  &mem_show_cmdlist, "show mem  ",
+		  0/* allow-unknown */, &showlist);
+
+  add_setshow_boolean_cmd ("inaccessible-by-default", no_class,
+				  &inaccessible_by_default, _("\
+Set handling of unknown memory regions."), _("\
+Show handling of unknown memory regions."), _("\
+If on, and some memory map is defined, debugger will emit errors on\n\
+accesses to memory not defined in the memory map. If off, accesses to all\n\
+memory addresses will be allowed."),
+				NULL,
+				show_inaccessible_by_default,
+				&mem_set_cmdlist,
+				&mem_show_cmdlist);
 }
=== gdb/memattr.h
==================================================================
--- gdb/memattr.h	(/mirrors/gdb_mainline)	(revision 2094)
+++ gdb/memattr.h	(/patches/gdb/memcheck/gdb_mainline)	(revision 2094)
@@ -26,6 +26,7 @@
 
 enum mem_access_mode
 {
+  MEM_NONE,                     /* Memory that is not physically present. */
   MEM_RW,			/* read/write */
   MEM_RO,			/* read only */
   MEM_WO,			/* write only */
@@ -76,7 +77,10 @@
 
 struct mem_region 
 {
+  /* Lowest address in the region.  */
   CORE_ADDR lo;
+  /* Address past the highest address of the region. 
+     If 0, upper bound is "infinity".  */
   CORE_ADDR hi;
 
   /* Item number of this memory region. */

Re: Check memory accesses

[Eli Zaretskii]

> From:  Vladimir Prus <ghost@cs.msu.su>
> Date:  Wed, 15 Nov 2006 15:24:07 +0300
> > 
> > This patch introduces a new user command, but doesn't include a patch
> > for the user manual.  Please provide one.
> 
> Sorry. This version of the patch includes documentation.

Thanks, but please state in the doco patch what is the default behavior.

Otherwise, it's okay with me.

Re: Check memory accesses

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

On Wednesday 15 November 2006 21:20, Eli Zaretskii wrote:
> > From:  Vladimir Prus <ghost@cs.msu.su>
> > Date:  Wed, 15 Nov 2006 15:24:07 +0300
> >
> > > This patch introduces a new user command, but doesn't include a patch
> > > for the user manual.  Please provide one.
> >
> > Sorry. This version of the patch includes documentation.
>
> Thanks, but please state in the doco patch what is the default behavior.
>
> Otherwise, it's okay with me.

Thanks. Adjusted and committed. Attached is the final version of the patch 
that went to CVS.

- Volodya



[-- Attachment #2: memcheck_committed.diff --]
[-- Type: text/x-diff, Size: 9152 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.7972
diff -u -p -r1.7972 ChangeLog
--- ChangeLog	21 Nov 2006 05:12:44 -0000	1.7972
+++ ChangeLog	21 Nov 2006 16:48:21 -0000
@@ -1,3 +1,21 @@
+2006-11-21  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* memattr.h (enum mem_access_mode): New value
+	MEM_NONE.
+	* memattr.c (unknown_mem_attrib): New.
+	(inaccessible_by_default): New.
+	(show_inaccessible_by_default): New.
+	(lookup_mem_region): Check inaccessible_by_default.
+	(dummy_cmd): New.
+	(mem_set_cmdlist, mem_show_cmdlist): New.
+	(_initialize_mem): Register new "set" and "show"
+	commands.
+	* target.c (memory_xfer_partial): If memory type
+	is MEM_NONE, return an error.
+	Clip to region size when calling to_xfer_partial.
+	If upper limit of memory range is 0, don't clip
+	anything.
+
 2006-11-20  Joel Brobecker  <brobecker@adacore.com>
 
 	* README: Remove obsolete information.
Index: memattr.c
===================================================================
RCS file: /cvs/src/src/gdb/memattr.c,v
retrieving revision 1.25
diff -u -p -r1.25 memattr.c
--- memattr.c	21 Sep 2006 14:00:53 -0000	1.25
+++ memattr.c	21 Nov 2006 16:48:21 -0000
@@ -40,6 +40,17 @@ const struct mem_attrib default_mem_attr
   -1 /* Flash blocksize not specified.  */
 };
 
+const struct mem_attrib unknown_mem_attrib =
+{
+  MEM_NONE,			/* mode */
+  MEM_WIDTH_UNSPECIFIED,
+  0,				/* hwbreak */
+  0,				/* cache */
+  0,				/* verify */
+  -1 /* Flash blocksize not specified.  */
+};
+
+
 VEC(mem_region_s) *mem_region_list, *target_mem_region_list;
 static int mem_number = 0;
 
@@ -53,6 +64,25 @@ static int mem_use_target = 1;
    empty, then the target can't supply memory regions.  */
 static int target_mem_regions_valid;
 
+/* If this flag is set, gdb will assume that memory ranges not
+   specified by the memory map have type MEM_NONE, and will
+   emit errors on all accesses to that memory.  */
+static int inaccessible_by_default = 0;
+
+static void
+show_inaccessible_by_default (struct ui_file *file, int from_tty,
+			      struct cmd_list_element *c,
+			      const char *value)
+{
+  if (inaccessible_by_default)
+    fprintf_filtered (file, _("\
+Unknown memory addresses will be treated as inaccessible.\n"));
+  else
+    fprintf_filtered (file, _("\
+Unknown memory addresses will be treated as RAM.\n"));          
+}
+
+
 /* Predicate function which returns true if LHS should sort before RHS
    in a list of memory regions, useful for VEC_lower_bound.  */
 
@@ -215,13 +245,17 @@ lookup_mem_region (CORE_ADDR addr)
   lo = 0;
   hi = 0;
 
-  /* If we ever want to support a huge list of memory regions, this
+  /* Either find memory range containing ADDRESS, or set LO and HI
+     to the nearest boundaries of an existing memory range.
+     
+     If we ever want to support a huge list of memory regions, this
      check should be replaced with a binary search (probably using
      VEC_lower_bound).  */
   for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
     {
       if (m->enabled_p == 1)
 	{
+	  /* If the address is in the memory region, return that memory range.  */
 	  if (addr >= m->lo && (addr < m->hi || m->hi == 0))
 	    return m;
 
@@ -243,7 +277,15 @@ lookup_mem_region (CORE_ADDR addr)
      was learned above.  */
   region.lo = lo;
   region.hi = hi;
-  region.attrib = default_mem_attrib;
+
+  /* When no memory map is defined at all, we always return 
+     'default_mem_attrib', so that we do not make all memory 
+     inaccessible for targets that don't provide a memory map.  */
+  if (inaccessible_by_default && !VEC_empty (mem_region_s, mem_region_list))
+    region.attrib = unknown_mem_attrib;
+  else
+    region.attrib = default_mem_attrib;
+
   return &region;
 }
 
@@ -674,9 +716,17 @@ mem_delete_command (char *args, int from
 
   dont_repeat ();
 }
+
+static void
+dummy_cmd (char *args, int from_tty)
+{
+}
 \f
 extern initialize_file_ftype _initialize_mem; /* -Wmissing-prototype */
 
+static struct cmd_list_element *mem_set_cmdlist;
+static struct cmd_list_element *mem_show_cmdlist;
+
 void
 _initialize_mem (void)
 {
@@ -709,4 +759,25 @@ Do \"info mem\" to see current list of c
 
   add_info ("mem", mem_info_command,
 	    _("Memory region attributes"));
+
+  add_prefix_cmd ("mem", class_vars, dummy_cmd, _("\
+Memory regions settings"),
+		  &mem_set_cmdlist, "set mem ",
+		  0/* allow-unknown */, &setlist);
+  add_prefix_cmd ("mem", class_vars, dummy_cmd, _("\
+Memory regions settings"),
+		  &mem_show_cmdlist, "show mem  ",
+		  0/* allow-unknown */, &showlist);
+
+  add_setshow_boolean_cmd ("inaccessible-by-default", no_class,
+				  &inaccessible_by_default, _("\
+Set handling of unknown memory regions."), _("\
+Show handling of unknown memory regions."), _("\
+If on, and some memory map is defined, debugger will emit errors on\n\
+accesses to memory not defined in the memory map. If off, accesses to all\n\
+memory addresses will be allowed."),
+				NULL,
+				show_inaccessible_by_default,
+				&mem_set_cmdlist,
+				&mem_show_cmdlist);
 }
Index: memattr.h
===================================================================
RCS file: /cvs/src/src/gdb/memattr.h,v
retrieving revision 1.7
diff -u -p -r1.7 memattr.h
--- memattr.h	21 Sep 2006 13:54:02 -0000	1.7
+++ memattr.h	21 Nov 2006 16:48:21 -0000
@@ -26,6 +26,7 @@
 
 enum mem_access_mode
 {
+  MEM_NONE,                     /* Memory that is not physically present. */
   MEM_RW,			/* read/write */
   MEM_RO,			/* read only */
   MEM_WO,			/* write only */
@@ -76,7 +77,10 @@ struct mem_attrib 
 
 struct mem_region 
 {
+  /* Lowest address in the region.  */
   CORE_ADDR lo;
+  /* Address past the highest address of the region. 
+     If 0, upper bound is "infinity".  */
   CORE_ADDR hi;
 
   /* Item number of this memory region. */
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.128
diff -u -p -r1.128 target.c
--- target.c	17 Oct 2006 21:55:23 -0000	1.128
+++ target.c	21 Nov 2006 16:48:22 -0000
@@ -1015,7 +1015,8 @@ memory_xfer_partial (struct target_ops *
 
   /* Try GDB's internal data cache.  */
   region = lookup_mem_region (memaddr);
-  if (memaddr + len < region->hi)
+  /* region->hi == 0 means there's no upper bound.  */
+  if (memaddr + len < region->hi || region->hi == 0)
     reg_len = len;
   else
     reg_len = region->hi - memaddr;
@@ -1037,6 +1038,9 @@ memory_xfer_partial (struct target_ops *
       if (writebuf != NULL)
 	error (_("Writing to flash memory forbidden in this context"));
       break;
+
+    case MEM_NONE:
+      return -1;
     }
 
   if (region->attrib.cache)
@@ -1072,7 +1076,7 @@ memory_xfer_partial (struct target_ops *
   do
     {
       res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
-				  readbuf, writebuf, memaddr, len);
+				  readbuf, writebuf, memaddr, reg_len);
       if (res > 0)
 	return res;
 
Index: doc/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
retrieving revision 1.623
diff -u -p -r1.623 ChangeLog
--- doc/ChangeLog	16 Nov 2006 15:08:22 -0000	1.623
+++ doc/ChangeLog	21 Nov 2006 16:48:24 -0000
@@ -1,3 +1,7 @@
+2006-11-21  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* gdb.texinfo (Memory Access Checking): New.
+
 2006-11-16  Daniel Jacobowitz  <dan@codesourcery.com>
 
 	* gdb.texinfo (Remote configuration): Mention
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.369
diff -u -p -r1.369 gdb.texinfo
--- doc/gdb.texinfo	16 Nov 2006 15:08:23 -0000	1.369
+++ doc/gdb.texinfo	21 Nov 2006 16:48:35 -0000
@@ -6894,6 +6894,27 @@ Enable @value{GDBN} to cache target memo
 Disable @value{GDBN} from caching target memory.  This is the default.
 @end table
 
+@subsection Memory Access Checking
+@value{GDBN} can be instructed to refuse accesses to memory that is
+not explicitly described.  This can be useful if accessing such
+regions has undesired effects for a specific target, or to provide
+better error checking.  The following commands control this behaviour.
+
+@table @code
+@kindex set mem inaccessible-by-default
+@item set mem inaccessible-by-default [on|off]
+If @code{on} is specified, make  @value{GDBN} treat memory not
+explicitly described by the memory ranges as non-existent and refuse accesses
+to such memory.  The checks are only performed if there's at least one
+memory range defined.  If @code{off} is specified, make @value{GDBN}
+treat the memory not explicitly described by the memory ranges as RAM.
+The default value is @code{off}.
+@kindex show mem inaccessible-by-default
+@item show mem inaccessible-by-default
+Show the current handling of accesses to unknown memory.
+@end table
+
+
 @c @subsubsection Memory Write Verification
 @c The memory write verification attributes set whether @value{GDBN}
 @c will re-reads data after each write to verify the write was successful.

Re: Check memory accesses

[Daniel Jacobowitz]

On Tue, Nov 21, 2006 at 07:51:29PM +0300, Vladimir Prus wrote:
> On Wednesday 15 November 2006 21:20, Eli Zaretskii wrote:
> > > From:  Vladimir Prus <ghost@cs.msu.su>
> > > Date:  Wed, 15 Nov 2006 15:24:07 +0300
> > >
> > > > This patch introduces a new user command, but doesn't include a patch
> > > > for the user manual.  Please provide one.
> > >
> > > Sorry. This version of the patch includes documentation.
> >
> > Thanks, but please state in the doco patch what is the default behavior.
> >
> > Otherwise, it's okay with me.
> 
> Thanks. Adjusted and committed. Attached is the final version of the patch 
> that went to CVS.

This is a new command; should we add all new commands to NEWS?

-- 
Daniel Jacobowitz
CodeSourcery

Re: Check memory accesses

[Eli Zaretskii]

> Date: Tue, 21 Nov 2006 12:03:20 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sources.redhat.com
> 
> This is a new command; should we add all new commands to NEWS?

We didn't do this religiously until now, AFAIK; but I won't mind if we
started from now on.  Thanks for the suggestion.

Re: Check memory accesses

[Daniel Jacobowitz]

On Wed, Nov 15, 2006 at 03:24:07PM +0300, Vladimir Prus wrote:
> +  /* region->hi means there's no upper bound. */
> +  if (memaddr + len < region->hi || region->hi == 0)

"region->hi == 0", you mean (in the comment)?  Also, two spaces after
period.

> +++ gdb/memattr.c	(/patches/gdb/memcheck/gdb_mainline)	(revision 2094)
> @@ -29,6 +29,7 @@
>  #include "language.h"
>  #include "vec.h"
>  #include "gdb_string.h"
> +#include "gdb_locale.h"

Shouldn't need this?  It's part of defs.h (deliberately, since
everywhere should use _()).

>        if (m->enabled_p == 1)
>  	{
> +	  /* If address is in memory return, return that memory range.  */

I think you mean "if the adderss is in the memory region".

> +  /* When no memory map is defined at all, we always set 'default_mem_attrib',
> +     so that we do not make the all memory inaccessible for targets that don't
> +     provide a memory map.  */

Always return, rather than "always set"?  Also, "make all memory".


-- 
Daniel Jacobowitz
CodeSourcery