[PATCH] memattr bounds

[PATCH] memattr bounds

[Don Howard]

The following addresses edge conditions in the mem command by making a
special case for upper bound == 0.  When the upper bound is zero, it is
assumed that the user wants an upper bound of max CORE_ADDR+1. Currently,
it's not possible to define a memory region with zero as it's upper bound, 
so this should not conflict with any current usage.

The patch also corrects a bug that allowes the definition of overlapping
memory regions, where the new region starts below an existing region and
extends above it. (Or does someone think that is a feature?)


2002-06-21  Don Howard  <dhoward@redhat.com>

	* memattr.c (create_mem_region): Treat hi == 0 as a special case
	that means max CORE_ADDR+1.
	(lookup_mem_region): Ditto.
	(mem_info_command): Ditto.


Index: memattr.c
===================================================================
RCS file: /cvs/src/src/gdb/memattr.c,v
retrieving revision 1.11
diff -p -u -w -r1.11 memattr.c
--- memattr.c	12 May 2002 04:20:05 -0000	1.11
+++ memattr.c	21 Jun 2002 20:47:15 -0000
@@ -47,7 +47,7 @@ create_mem_region (CORE_ADDR lo, CORE_AD
   struct mem_region *n, *new;
 
   /* lo == hi is a useless empty region */
-  if (lo >= hi)
+  if (lo >= hi && hi != 0)
     {
       printf_unfiltered ("invalid memory region: low >= high\n");
       return NULL;
@@ -57,8 +57,9 @@ create_mem_region (CORE_ADDR lo, CORE_AD
   while (n)
     {
       /* overlapping node */
-      if ((lo >= n->lo && lo < n->hi) ||
-	  (hi > n->lo && hi <= n->hi))
+      if ((lo >= n->lo && (lo < n->hi || n->hi == 0)) ||
+	  (hi > n->lo && (hi <= n->hi || n->hi == 0)) ||
+	  (lo <= n->lo && (hi >= n->hi || hi == 0)))
 	{
 	  printf_unfiltered ("overlapping memory region\n");
 	  return NULL;
@@ -111,7 +112,7 @@ lookup_mem_region (CORE_ADDR addr)
     {
       if (m->enabled_p == 1)
 	{
-	  if (addr >= m->lo && addr < m->hi)
+	  if (addr >= m->lo && (addr < m->hi || m->hi == 0))
 	    return m;
 
 	  if (addr >= m->hi && lo < m->hi)
@@ -246,9 +247,9 @@ mem_info_command (char *args, int from_t
       printf_filtered ("%s ", tmp);
       
       if (TARGET_ADDR_BIT <= 32)
-	tmp = local_hex_string_custom ((unsigned long) m->hi, "08l");
+	tmp = local_hex_string_custom ((unsigned long) (m->hi ? m->hi : ~0), "08l");
       else
-	tmp = local_hex_string_custom ((unsigned long) m->hi, "016l");
+	tmp = local_hex_string_custom ((unsigned long) (m->hi ? m->hi : ~0), "016l");
       
       printf_filtered ("%s ", tmp);
 


-- 
dhoward@redhat.com
gdb engineering

Re: [PATCH] memattr bounds

[Andrew Cagney]

> The following addresses edge conditions in the mem command by making a
> special case for upper bound == 0.  When the upper bound is zero, it is
> assumed that the user wants an upper bound of max CORE_ADDR+1. Currently,
> it's not possible to define a memory region with zero as it's upper bound, 
> so this should not conflict with any current usage.

Don't forget to follow up with something for the doco.

> The patch also corrects a bug that allowes the definition of overlapping
> memory regions, where the new region starts below an existing region and
> extends above it. (Or does someone think that is a feature?)

I would treat it as a bug.

Some of the simulators have a mechanism that is similar to memattr where 
the address map is layered and each address range is assigned to a 
layer.  Within a layer things can't overlap but between layers they can 
- this making the search well defined.   Adding such a mechanism to 
memattr would be a useful but perhaps best left to the reader :-)

> 2002-06-21  Don Howard  <dhoward@redhat.com>
> 
> 	* memattr.c (create_mem_region): Treat hi == 0 as a special case
> 	that means max CORE_ADDR+1.
> 	(lookup_mem_region): Ditto.
> 	(mem_info_command): Ditto.
> 
> 


Yes,

1.5 tweaks:

> -      if ((lo >= n->lo && lo < n->hi) ||
> -	  (hi > n->lo && hi <= n->hi))
> +      if ((lo >= n->lo && (lo < n->hi || n->hi == 0)) ||
> +	  (hi > n->lo && (hi <= n->hi || n->hi == 0)) ||
> +	  (lo <= n->lo && (hi >= n->hi || hi == 0)))

The ``||'' goes at the start of the line (the old code was also wrong :-)

>        if (TARGET_ADDR_BIT <= 32)
> -	tmp = local_hex_string_custom ((unsigned long) m->hi, "08l");
> +	tmp = local_hex_string_custom ((unsigned long) (m->hi ? m->hi : ~0), "08l");
>        else
> -	tmp = local_hex_string_custom ((unsigned long) m->hi, "016l");
> +	tmp = local_hex_string_custom ((unsigned long) (m->hi ? m->hi : ~0), "016l");
>        

Consider lifting the ``(m->hi ? m->hi : ~0)'' out to before the if().

which ever, it can then go straight in.
Andrew

Re: [PATCH] memattr bounds

[Don Howard]

On Fri, 21 Jun 2002, Andrew Cagney wrote:

> > The following addresses edge conditions in the mem command by making a
> > special case for upper bound == 0.  When the upper bound is zero, it is
> > assumed that the user wants an upper bound of max CORE_ADDR+1. Currently,
> > it's not possible to define a memory region with zero as it's upper bound, 
> > so this should not conflict with any current usage.
> 

I've committed the memattr.c changes, with the requested tweaks.

Is the doco descriptive enough?  I'm trying to be brief without being
terse...


2002-06-24  Don Howard  <dhoward@redhat.com>

        * memattr.c (create_mem_region): Treat hi == 0 as a special case
        that means max CORE_ADDR+1.
        (lookup_mem_region): Ditto.
        (mem_info_command): Ditto.



Index: memattr.c
===================================================================
RCS file: /cvs/src/src/gdb/memattr.c,v
retrieving revision 1.11
diff -p -u -w -r1.11 memattr.c
--- memattr.c	12 May 2002 04:20:05 -0000	1.11
+++ memattr.c	24 Jun 2002 21:58:54 -0000
@@ -47,7 +47,7 @@ create_mem_region (CORE_ADDR lo, CORE_AD
   struct mem_region *n, *new;
 
   /* lo == hi is a useless empty region */
-  if (lo >= hi)
+  if (lo >= hi && hi != 0)
     {
       printf_unfiltered ("invalid memory region: low >= high\n");
       return NULL;
@@ -57,8 +57,9 @@ create_mem_region (CORE_ADDR lo, CORE_AD
   while (n)
     {
       /* overlapping node */
-      if ((lo >= n->lo && lo < n->hi) ||
-	  (hi > n->lo && hi <= n->hi))
+      if ((lo >= n->lo && (lo < n->hi || n->hi == 0)) 
+	  || (hi > n->lo && (hi <= n->hi || n->hi == 0))
+	  || (lo <= n->lo && (hi >= n->hi || hi == 0)))
 	{
 	  printf_unfiltered ("overlapping memory region\n");
 	  return NULL;
@@ -111,7 +112,7 @@ lookup_mem_region (CORE_ADDR addr)
     {
       if (m->enabled_p == 1)
 	{
-	  if (addr >= m->lo && addr < m->hi)
+	  if (addr >= m->lo && (addr < m->hi || m->hi == 0))
 	    return m;
 
 	  if (addr >= m->hi && lo < m->hi)
@@ -234,6 +235,7 @@ mem_info_command (char *args, int from_t
 
   for (m = mem_region_chain; m; m = m->next)
     {
+      CORE_ADDR hi;
       char *tmp;
       printf_filtered ("%-3d %-3c\t",
 		       m->number,
@@ -244,11 +246,12 @@ mem_info_command (char *args, int from_t
 	tmp = local_hex_string_custom ((unsigned long) m->lo, "016l");
       
       printf_filtered ("%s ", tmp);
+      hi = (m->hi == 0 ? ~0 : m->hi);
       
       if (TARGET_ADDR_BIT <= 32)
-	tmp = local_hex_string_custom ((unsigned long) m->hi, "08l");
+	tmp = local_hex_string_custom ((unsigned long) hi, "08l");
       else
-	tmp = local_hex_string_custom ((unsigned long) m->hi, "016l");
+	tmp = local_hex_string_custom ((unsigned long) hi, "016l");
       
       printf_filtered ("%s ", tmp);
 
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.102
diff -p -u -w -r1.102 gdb.texinfo
--- doc/gdb.texinfo	11 Jun 2002 20:36:57 -0000	1.102
+++ doc/gdb.texinfo	24 Jun 2002 21:59:06 -0000
@@ -5601,9 +5601,10 @@ to enable, disable, or remove a memory r
 
 @table @code
 @kindex mem
-@item mem @var{address1} @var{address2} @var{attributes}@dots{}
-Define memory region bounded by @var{address1} and @var{address2}
-with attributes @var{attributes}@dots{}.
+@item mem @var{lower} @var{upper} @var{attributes}@dots{}
+Define memory region bounded by @var{lower} and @var{upper} with
+attributes @var{attributes}@dots{}.  Note that @var{upper} == 0 is a
+special case: it indicates the max memory address.
 
 @kindex delete mem
 @item delete mem @var{nums}@dots{}


-- 
dhoward@redhat.com
gdb engineering

Re: [PATCH] memattr bounds

[Eli Zaretskii]

On Mon, 24 Jun 2002, Don Howard wrote:

> Is the doco descriptive enough?  I'm trying to be brief without being
> terse...

It's okay, but please say a word ore two about what "max memory address" 
means.

Otherwise, approved.

Thanks.

Re: [PATCH] memattr bounds

[Don Howard]

On Tue, 25 Jun 2002, Eli Zaretskii wrote:

> 
> On Mon, 24 Jun 2002, Don Howard wrote:
> 
> > Is the doco descriptive enough?  I'm trying to be brief without being
> > terse...
> 
> It's okay, but please say a word ore two about what "max memory address" 
> means.
> 
> Otherwise, approved.
> 
> Thanks.
> 


I've checked in the following.  Hopefully it describes "max memory 
address" better.  If not, let me know.



2002-06-25  Don Howard  <dhoward@redhat.com>

       * gdb.texinfo (Memory Region Attributes): Document new behavior
       for 'mem' command.



Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.102
diff -p -u -w -r1.102 gdb.texinfo
--- doc/gdb.texinfo     11 Jun 2002 20:36:57 -0000      1.102
+++ doc/gdb.texinfo     25 Jun 2002 07:18:31 -0000
@@ -5601,9 +5601,11 @@ to enable, disable, or remove a memory r
 
 @table @code
 @kindex mem
-@item mem @var{address1} @var{address2} @var{attributes}@dots{}
-Define memory region bounded by @var{address1} and @var{address2}
-with attributes @var{attributes}@dots{}.
+@item mem @var{lower} @var{upper} @var{attributes}@dots{}
+Define memory region bounded by @var{lower} and @var{upper} with
+attributes @var{attributes}@dots{}.  Note that @var{upper} == 0 is a
+special case: it is treated as the the target's maximum memory address.
+(0xffff on 16 bit targets, 0xffffffff on 32 bit targets, etc.)

-- 
dhoward@redhat.com
gdb engineering

Re: [PATCH] memattr bounds

[Eli Zaretskii]

On Tue, 25 Jun 2002, Don Howard wrote:

> I've checked in the following.  Hopefully it describes "max memory 
> address" better.

It does, thanks.

Re: [PATCH] memattr bounds

[Jim Blandy]

Don Howard <dhoward@redhat.com> writes:
> Is the doco descriptive enough?  I'm trying to be brief without being
> terse...
> Index: doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.102
> diff -p -u -w -r1.102 gdb.texinfo
> --- doc/gdb.texinfo	11 Jun 2002 20:36:57 -0000	1.102
> +++ doc/gdb.texinfo	24 Jun 2002 21:59:06 -0000
> @@ -5601,9 +5601,10 @@ to enable, disable, or remove a memory r
>  
>  @table @code
>  @kindex mem
> -@item mem @var{address1} @var{address2} @var{attributes}@dots{}
> -Define memory region bounded by @var{address1} and @var{address2}
> -with attributes @var{attributes}@dots{}.
> +@item mem @var{lower} @var{upper} @var{attributes}@dots{}
> +Define memory region bounded by @var{lower} and @var{upper} with
> +attributes @var{attributes}@dots{}.  Note that @var{upper} == 0 is a
> +special case: it indicates the max memory address.

Why try to be terse or brief?  This is the reference manual; it's more
important to be clear and complete, no?

Also, I don't think it's cool to use `==' in the reference manual;
it's supposed to be English.  :)

        As a special case, if @var{upper} is zero, then the memory
        region ends at the top of the address space.