[PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Klee Dienes]

The following patch converts several instances of read_memory_integer  
to read_memory_unsigned_integer.
In all cases except one, the value is being read into a CORE_ADDR,  
which will cause sign-extension lossage if read_memory_integer is used  
with a 64-bit bfd (the final case is reading into an unsigned int, but  
I believe the change is still correct).

2002-10-05  Klee Dienes  <kdienes@apple.com>

         * blockframe.c (sigtramp_saved_pc): Use
         read_memory_unsigned_integer to read a value destined for a
         CORE_ADDR, not read_memory_integer.
         * f-valprint.c (f77_get_dynamic_upperbound): Ditto.
         (f77_get_dynamic_lowerbound): Ditto.
         * symfile.c (simple_read_overlay_region_table): Ditto (this
         function is reading to an unsigned int, not a CORE_ADDR, and is
         commented out, but I believe reading as unsigned is still
         correct).

diff -ru --exclude=Makefile.in --exclude=configure.in  
--exclude=configure --exclude=aclocal.m4 --exclude=config.in  
--exclude=gconfig.in --exclude=config.h.in --exclude=Make-in  
--exclude=CVS  
/Volumes/Storage/Users/kdienes/source/cygnus.pristine-current/src/gdb/ 
blockframe.c ./blockframe.c
---  
/Volumes/Storage/Users/kdienes/source/cygnus.pristine-current/src/gdb/ 
blockframe.c	Sun Sep 22 12:47:43 2002
+++ ./blockframe.c	Sat Oct  5 21:05:32 2002
@@ -1035,14 +1035,14 @@
    buf = alloca (ptrbytes);
    /* Get sigcontext address, it is the third parameter on the stack.   
*/
    if (frame->next)
-    sigcontext_addr = read_memory_integer (FRAME_ARGS_ADDRESS  
(frame->next)
-					   + FRAME_ARGS_SKIP
-					   + sigcontext_offs,
-					   ptrbytes);
+    sigcontext_addr = read_memory_unsigned_integer (FRAME_ARGS_ADDRESS  
(frame->next)
+						    + FRAME_ARGS_SKIP
+						    + sigcontext_offs,
+						    ptrbytes);
    else
-    sigcontext_addr = read_memory_integer (read_register (SP_REGNUM)
-					   + sigcontext_offs,
-					   ptrbytes);
+    sigcontext_addr = read_memory_unsigned_integer (read_register  
(SP_REGNUM)
+						    + sigcontext_offs,
+						    ptrbytes);

    /* Don't cause a memory_error when accessing sigcontext in case the  
stack
       layout has changed or the stack is corrupt.  */
diff -ru --exclude=Makefile.in --exclude=configure.in  
--exclude=configure --exclude=aclocal.m4 --exclude=config.in  
--exclude=gconfig.in --exclude=config.h.in --exclude=Make-in  
--exclude=CVS  
/Volumes/Storage/Users/kdienes/source/cygnus.pristine-current/src/gdb/ 
f-valprint.c ./f-valprint.c
---  
/Volumes/Storage/Users/kdienes/source/cygnus.pristine-current/src/gdb/ 
f-valprint.c	Mon Aug  5 17:39:21 2002
+++ ./f-valprint.c	Sat Oct  5 20:24:29 2002
@@ -77,10 +77,8 @@
        current_frame_addr = selected_frame->frame;
        if (current_frame_addr > 0)
  	{
-	  *lower_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_LOWER_BOUND_VALUE (type),
-				 4);
+	  *lower_bound = read_memory_unsigned_integer
+	    (current_frame_addr + TYPE_ARRAY_LOWER_BOUND_VALUE (type), 4);
  	}
        else
  	{
@@ -101,11 +99,9 @@
        current_frame_addr = selected_frame->frame;
        if (current_frame_addr > 0)
  	{
-	  ptr_to_lower_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_LOWER_BOUND_VALUE (type),
-				 4);
-	  *lower_bound = read_memory_integer (ptr_to_lower_bound, 4);
+	  ptr_to_lower_bound = read_memory_unsigned_integer
+	    (current_frame_addr + TYPE_ARRAY_LOWER_BOUND_VALUE (type), 4);
+	  *lower_bound = read_memory_unsigned_integer (ptr_to_lower_bound, 4);
  	}
        else
  	{
@@ -135,10 +131,8 @@
        current_frame_addr = selected_frame->frame;
        if (current_frame_addr > 0)
  	{
-	  *upper_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_UPPER_BOUND_VALUE (type),
-				 4);
+	  *upper_bound = read_memory_unsigned_integer
+	    (current_frame_addr + TYPE_ARRAY_UPPER_BOUND_VALUE (type), 4);
  	}
        else
  	{
@@ -164,11 +158,9 @@
        current_frame_addr = selected_frame->frame;
        if (current_frame_addr > 0)
  	{
-	  ptr_to_upper_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_UPPER_BOUND_VALUE (type),
-				 4);
-	  *upper_bound = read_memory_integer (ptr_to_upper_bound, 4);
+	  ptr_to_upper_bound = read_memory_unsigned_integer
+	    (current_frame_addr + TYPE_ARRAY_UPPER_BOUND_VALUE (type), 4);
+	  *upper_bound = read_memory_unsigned_integer (ptr_to_upper_bound, 4);
  	}
        else
  	{
diff -ru --exclude=Makefile.in --exclude=configure.in  
--exclude=configure --exclude=aclocal.m4 --exclude=config.in  
--exclude=gconfig.in --exclude=config.h.in --exclude=Make-in  
--exclude=CVS  
/Volumes/Storage/Users/kdienes/source/cygnus.pristine-current/src/gdb/ 
symfile.c ./symfile.c
---  
/Volumes/Storage/Users/kdienes/source/cygnus.pristine-current/src/gdb/ 
symfile.c	Sun Sep 22 12:47:51 2002
+++ ./symfile.c	Sun Sep 29 02:09:10 2002
@@ -3137,7 +3377,7 @@
    simple_free_overlay_region_table ();
    msym = lookup_minimal_symbol ("_novly_regions", NULL, NULL);
    if (msym != NULL)
-    cache_novly_regions = read_memory_integer (SYMBOL_VALUE_ADDRESS  
(msym), 4);
+    cache_novly_regions = read_memory_unsigned_integer  
(SYMBOL_VALUE_ADDRESS (msym), 4);
    else
      return 0;			/* failure */
    cache_ovly_region_table = (void *) xmalloc (cache_novly_regions *  
12);

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Daniel Jacobowitz]

On Sat, Oct 05, 2002 at 10:48:35PM -0400, Klee Dienes wrote:
> The following patch converts several instances of read_memory_integer  
> to read_memory_unsigned_integer.
> In all cases except one, the value is being read into a CORE_ADDR,  
> which will cause sign-extension lossage if read_memory_integer is used  
> with a 64-bit bfd (the final case is reading into an unsigned int, but  
> I believe the change is still correct).
> 
> 2002-10-05  Klee Dienes  <kdienes@apple.com>
> 
>         * blockframe.c (sigtramp_saved_pc): Use
>         read_memory_unsigned_integer to read a value destined for a
>         CORE_ADDR, not read_memory_integer.
>         * f-valprint.c (f77_get_dynamic_upperbound): Ditto.
>         (f77_get_dynamic_lowerbound): Ditto.
>         * symfile.c (simple_read_overlay_region_table): Ditto (this
>         function is reading to an unsigned int, not a CORE_ADDR, and is
>         commented out, but I believe reading as unsigned is still
>         correct).

Well, I'm a little concerned about this.  MIPS generally wants the
result to be sign extended; the MIPS 32-bit ABIs map onto the lowest
and highest 2G segments of the 64-bit address space, not the low 4G
segment.  Isn't there an appropriate function somewhere to read a CORE_ADDR?

Answer: Yes, there is, but it has the same problem.  You're just
extending current practice, so I don't see a problem with your patch.

Andrew, is my understanding right?  Is there some reason this isn't a
problem?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Klee Dienes]

Perhaps something along the lines of the following?

{
   unsigned char buf[sizeof (CORE_ADDR)];
   if (target_read_memory (memaddr, buf, TYPE_LENGTH 
(builtin_type_void_data_ptr)) != 0)
	error ("Unable to determine lower bound of array.");
   *lower_bound = extract_typed_address (buf, 
builtin_type_void_data_ptr);
}

That's a bit verbose, and it's not clear to me if sizeof (CORE_ADDR) is 
guaranteed to be large enough to contain a raw target pointer, but I 
can't really think of any better alternatives.

Aside from the intimidating comments in findvar.c, this would seem a 
good candidate for extract_address ... though a similar modification 
would have to be made to it to handle the sign-extension as well.

Or alternately, I could just leave well-enough alone, and be careful to 
truncate CORE_ADDRs to 32 bits in our target xfer_memory code (or the 
MIPS code could sign-extend it in the same place).

On Saturday, October 5, 2002, at 10:57 PM, Daniel Jacobowitz wrote:
>
> Well, I'm a little concerned about this.  MIPS generally wants the
> result to be sign extended; the MIPS 32-bit ABIs map onto the lowest
> and highest 2G segments of the 64-bit address space, not the low 4G
> segment.  Isn't there an appropriate function somewhere to read a 
> CORE_ADDR?
>
> Answer: Yes, there is, but it has the same problem.  You're just
> extending current practice, so I don't see a problem with your patch.
>
> Andrew, is my understanding right?  Is there some reason this isn't a
> problem?

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Daniel Jacobowitz]

On Sun, Oct 06, 2002 at 02:52:57AM -0400, Klee Dienes wrote:
> Perhaps something along the lines of the following?
> 
> {
>   unsigned char buf[sizeof (CORE_ADDR)];
>   if (target_read_memory (memaddr, buf, TYPE_LENGTH 
> (builtin_type_void_data_ptr)) != 0)
> 	error ("Unable to determine lower bound of array.");
>   *lower_bound = extract_typed_address (buf, 
> builtin_type_void_data_ptr);
> }
> 
> That's a bit verbose, and it's not clear to me if sizeof (CORE_ADDR) is 
> guaranteed to be large enough to contain a raw target pointer, but I 
> can't really think of any better alternatives.

Might want to use alloca (TYPE_LENGTH (builtin_type_void_data_ptr)).

> Aside from the intimidating comments in findvar.c, this would seem a 
> good candidate for extract_address ... though a similar modification 
> would have to be made to it to handle the sign-extension as well.
> 
> Or alternately, I could just leave well-enough alone, and be careful to 
> truncate CORE_ADDRs to 32 bits in our target xfer_memory code (or the 
> MIPS code could sign-extend it in the same place).

Well, I think a general read_memory_typed_address (returning CORE_ADDR,
and honoroing POINTER_TO_ADDRESS) would be the way to go here.


> 
> On Saturday, October 5, 2002, at 10:57 PM, Daniel Jacobowitz wrote:
> >
> >Well, I'm a little concerned about this.  MIPS generally wants the
> >result to be sign extended; the MIPS 32-bit ABIs map onto the lowest
> >and highest 2G segments of the 64-bit address space, not the low 4G
> >segment.  Isn't there an appropriate function somewhere to read a 
> >CORE_ADDR?
> >
> >Answer: Yes, there is, but it has the same problem.  You're just
> >extending current practice, so I don't see a problem with your patch.
> >
> >Andrew, is my understanding right?  Is there some reason this isn't a
> >problem?
> 
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Klee Dienes]

On Sunday, October 6, 2002, at 11:09 AM, Daniel Jacobowitz wrote:

>
> Might want to use alloca (TYPE_LENGTH (builtin_type_void_data_ptr)).

Love to; I had been under the (apparently mistaken) impression that 
alloca was deprecated for new code.

>> Aside from the intimidating comments in findvar.c, this would seem a
>> good candidate for extract_address ... though a similar modification
>> would have to be made to it to handle the sign-extension as well.
>>
>> Or alternately, I could just leave well-enough alone, and be careful 
>> to
>> truncate CORE_ADDRs to 32 bits in our target xfer_memory code (or the
>> MIPS code could sign-extend it in the same place).
>
> Well, I think a general read_memory_typed_address (returning CORE_ADDR,
> and honoroing POINTER_TO_ADDRESS) would be the way to go here.

That sounds good to me; how about the following?  I removed the patch 
to symfile.c, as it's commented out anyway, and probably not worth the 
effort.  My one concern is that I don't currently have a good system to 
test the part of the patch that affects blockframe.c.

2002-10-06  Klee Dienes  <kdienes@apple.com>

         * findvar.c (read_memory_typed_address): New function.
         * gdbcore.h (read_memory_typed_address): Add prototype.
         * blockframe.c (sigtramp_saved_pc): Use
         read_memory_typed_address to read a value destined for a
         CORE_ADDR, not read_memory_integer.
         * f-valprint.c (f77_get_dynamic_upperbound): Ditto.
         (f77_get_dynamic_lowerbound): Ditto.

diff -u /home/klee/source/cygnus.cygnus/gdb/gdbcore.h ./gdbcore.h
--- /home/klee/source/cygnus.cygnus/gdb/gdbcore.h	Wed Dec 19 15:54:26 
2001
+++ ./gdbcore.h	Sun Oct  6 09:18:11 2002
@@ -64,8 +64,15 @@

  /* Read a null-terminated string from the debuggee's memory, given 
address,
   * a buffer into which to place the string, and the maximum available 
space */
+
  extern void read_memory_string (CORE_ADDR, char *, int);

+/* Read the pointer of type TYPE at BUF, and return the address it
+   represents. */
+
+CORE_ADDR
+read_memory_typed_address (CORE_ADDR addr, struct type *type);
+
  /* This takes a char *, not void *.  This is probably right, because
     passing in an int * or whatever is wrong with respect to
     byteswapping, alignment, different sizes for host vs. target types,
diff -u /home/klee/source/cygnus.cygnus/gdb/corefile.c ./corefile.c
--- /home/klee/source/cygnus.cygnus/gdb/corefile.c	Thu May 30 15:41:24 
2002
+++ ./corefile.c	Sun Oct  6 09:18:11 2002
@@ -356,6 +356,14 @@
      }
  }

+CORE_ADDR
+read_memory_typed_address (CORE_ADDR addr, struct type *type)
+{
+  char *buf = alloca (TYPE_LENGTH (type));
+  read_memory (addr, buf, TYPE_LENGTH (type));
+  return extract_typed_address (buf, type);
+}
+
  /* Same as target_write_memory, but report an error if can't write.  */
  void
  write_memory (CORE_ADDR memaddr, char *myaddr, int len)
diff -u /home/klee/source/cygnus.cygnus/gdb/blockframe.c ./blockframe.c
--- /home/klee/source/cygnus.cygnus/gdb/blockframe.c	Mon Sep 30 
18:24:01 2002
+++ ./blockframe.c	Sun Oct  6 09:35:42 2002
@@ -1035,19 +1035,17 @@
    buf = alloca (ptrbytes);
    /* Get sigcontext address, it is the third parameter on the stack.  
*/
    if (frame->next)
-    sigcontext_addr = read_memory_integer (FRAME_ARGS_ADDRESS 
(frame->next)
-					   + FRAME_ARGS_SKIP
-					   + sigcontext_offs,
-					   ptrbytes);
+    sigcontext_addr = read_memory_typed_address
+      (FRAME_ARGS_ADDRESS (frame->next) + FRAME_ARGS_SKIP + 
sigcontext_offs,
+       builtin_type_void_data_ptr);
    else
-    sigcontext_addr = read_memory_integer (read_register (SP_REGNUM)
-					   + sigcontext_offs,
-					   ptrbytes);
+    sigcontext_addr = read_memory_typed_address
+      (read_register (SP_REGNUM) + sigcontext_offs, 
builtin_type_void_data_ptr);

    /* Don't cause a memory_error when accessing sigcontext in case the 
stack
       layout has changed or the stack is corrupt.  */
    target_read_memory (sigcontext_addr + SIGCONTEXT_PC_OFFSET, buf, 
ptrbytes);
-  return extract_unsigned_integer (buf, ptrbytes);
+  return extract_typed_address (buf, builtin_type_void_data_ptr);
  }
  #endif /* SIGCONTEXT_PC_OFFSET */

diff -u /home/klee/source/cygnus.cygnus/gdb/f-valprint.c ./f-valprint.c
--- /home/klee/source/cygnus.cygnus/gdb/f-valprint.c	Tue Mar  6 
18:57:08 2001
+++ ./f-valprint.c	Sun Oct  6 09:18:11 2002
@@ -102,9 +102,9 @@
        if (current_frame_addr > 0)
  	{
  	  ptr_to_lower_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_LOWER_BOUND_VALUE (type),
-				 4);
+	    read_memory_typed_address (current_frame_addr +
+				       TYPE_ARRAY_LOWER_BOUND_VALUE (type),
+				       builtin_type_void_data_ptr);
  	  *lower_bound = read_memory_integer (ptr_to_lower_bound, 4);
  	}
        else
@@ -165,9 +165,9 @@
        if (current_frame_addr > 0)
  	{
  	  ptr_to_upper_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_UPPER_BOUND_VALUE (type),
-				 4);
+	    read_memory_typed_address (current_frame_addr +
+				       TYPE_ARRAY_UPPER_BOUND_VALUE (type),
+				       builtin_type_void_data_ptr);
  	  *upper_bound = read_memory_integer (ptr_to_upper_bound, 4);
  	}
        else

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Daniel Jacobowitz]

On Sun, Oct 06, 2002 at 12:41:42PM -0400, Klee Dienes wrote:
> On Sunday, October 6, 2002, at 11:09 AM, Daniel Jacobowitz wrote:
> 
> >
> >Might want to use alloca (TYPE_LENGTH (builtin_type_void_data_ptr)).
> 
> Love to; I had been under the (apparently mistaken) impression that 
> alloca was deprecated for new code.

Check the "Coding" chapter of gdbint.texinfo:

   GDB can use the non-portable function `alloca' for the allocation of
small temporary values (such as strings).

I think at least one of the other GNU tools doesn't like alloca -
probably it's GCC and binutils both.

> >>Aside from the intimidating comments in findvar.c, this would seem a
> >>good candidate for extract_address ... though a similar modification
> >>would have to be made to it to handle the sign-extension as well.
> >>
> >>Or alternately, I could just leave well-enough alone, and be careful 
> >>to
> >>truncate CORE_ADDRs to 32 bits in our target xfer_memory code (or the
> >>MIPS code could sign-extend it in the same place).
> >
> >Well, I think a general read_memory_typed_address (returning CORE_ADDR,
> >and honoroing POINTER_TO_ADDRESS) would be the way to go here.
> 
> That sounds good to me; how about the following?  I removed the patch 
> to symfile.c, as it's commented out anyway, and probably not worth the 
> effort.  My one concern is that I don't currently have a good system to 
> test the part of the patch that affects blockframe.c.

Can't the Apple tools group lodge a formal complaint about Apple Mail,
or something? :)  Your patch is line wrapped.

Other than that, I can't approve this sort of patch, but it looks like
exactly what I wanted.  Thanks for bearing with me.

> 
> 2002-10-06  Klee Dienes  <kdienes@apple.com>
> 
>         * findvar.c (read_memory_typed_address): New function.
>         * gdbcore.h (read_memory_typed_address): Add prototype.
>         * blockframe.c (sigtramp_saved_pc): Use
>         read_memory_typed_address to read a value destined for a
>         CORE_ADDR, not read_memory_integer.
>         * f-valprint.c (f77_get_dynamic_upperbound): Ditto.
>         (f77_get_dynamic_lowerbound): Ditto.
> 
> diff -u /home/klee/source/cygnus.cygnus/gdb/gdbcore.h ./gdbcore.h
> --- /home/klee/source/cygnus.cygnus/gdb/gdbcore.h	Wed Dec 19 15:54:26 
> 2001
> +++ ./gdbcore.h	Sun Oct  6 09:18:11 2002
> @@ -64,8 +64,15 @@
> 
>  /* Read a null-terminated string from the debuggee's memory, given 
> address,
>   * a buffer into which to place the string, and the maximum available 
> space */
> +
>  extern void read_memory_string (CORE_ADDR, char *, int);
> 
> +/* Read the pointer of type TYPE at BUF, and return the address it
> +   represents. */
> +
> +CORE_ADDR
> +read_memory_typed_address (CORE_ADDR addr, struct type *type);
> +
>  /* This takes a char *, not void *.  This is probably right, because
>     passing in an int * or whatever is wrong with respect to
>     byteswapping, alignment, different sizes for host vs. target types,
> diff -u /home/klee/source/cygnus.cygnus/gdb/corefile.c ./corefile.c
> --- /home/klee/source/cygnus.cygnus/gdb/corefile.c	Thu May 30 15:41:24 
> 2002
> +++ ./corefile.c	Sun Oct  6 09:18:11 2002
> @@ -356,6 +356,14 @@
>      }
>  }
> 
> +CORE_ADDR
> +read_memory_typed_address (CORE_ADDR addr, struct type *type)
> +{
> +  char *buf = alloca (TYPE_LENGTH (type));
> +  read_memory (addr, buf, TYPE_LENGTH (type));
> +  return extract_typed_address (buf, type);
> +}
> +
>  /* Same as target_write_memory, but report an error if can't write.  */
>  void
>  write_memory (CORE_ADDR memaddr, char *myaddr, int len)
> diff -u /home/klee/source/cygnus.cygnus/gdb/blockframe.c ./blockframe.c
> --- /home/klee/source/cygnus.cygnus/gdb/blockframe.c	Mon Sep 30 
> 18:24:01 2002
> +++ ./blockframe.c	Sun Oct  6 09:35:42 2002
> @@ -1035,19 +1035,17 @@
>    buf = alloca (ptrbytes);
>    /* Get sigcontext address, it is the third parameter on the stack.  
> */
>    if (frame->next)
> -    sigcontext_addr = read_memory_integer (FRAME_ARGS_ADDRESS 
> (frame->next)
> -					   + FRAME_ARGS_SKIP
> -					   + sigcontext_offs,
> -					   ptrbytes);
> +    sigcontext_addr = read_memory_typed_address
> +      (FRAME_ARGS_ADDRESS (frame->next) + FRAME_ARGS_SKIP + 
> sigcontext_offs,
> +       builtin_type_void_data_ptr);
>    else
> -    sigcontext_addr = read_memory_integer (read_register (SP_REGNUM)
> -					   + sigcontext_offs,
> -					   ptrbytes);
> +    sigcontext_addr = read_memory_typed_address
> +      (read_register (SP_REGNUM) + sigcontext_offs, 
> builtin_type_void_data_ptr);
> 
>    /* Don't cause a memory_error when accessing sigcontext in case the 
> stack
>       layout has changed or the stack is corrupt.  */
>    target_read_memory (sigcontext_addr + SIGCONTEXT_PC_OFFSET, buf, 
> ptrbytes);
> -  return extract_unsigned_integer (buf, ptrbytes);
> +  return extract_typed_address (buf, builtin_type_void_data_ptr);
>  }
>  #endif /* SIGCONTEXT_PC_OFFSET */
> 
> diff -u /home/klee/source/cygnus.cygnus/gdb/f-valprint.c ./f-valprint.c
> --- /home/klee/source/cygnus.cygnus/gdb/f-valprint.c	Tue Mar  6 
> 18:57:08 2001
> +++ ./f-valprint.c	Sun Oct  6 09:18:11 2002
> @@ -102,9 +102,9 @@
>        if (current_frame_addr > 0)
>  	{
>  	  ptr_to_lower_bound =
> -	    read_memory_integer (current_frame_addr +
> -				 TYPE_ARRAY_LOWER_BOUND_VALUE (type),
> -				 4);
> +	    read_memory_typed_address (current_frame_addr +
> +				       TYPE_ARRAY_LOWER_BOUND_VALUE (type),
> +				       builtin_type_void_data_ptr);
>  	  *lower_bound = read_memory_integer (ptr_to_lower_bound, 4);
>  	}
>        else
> @@ -165,9 +165,9 @@
>        if (current_frame_addr > 0)
>  	{
>  	  ptr_to_upper_bound =
> -	    read_memory_integer (current_frame_addr +
> -				 TYPE_ARRAY_UPPER_BOUND_VALUE (type),
> -				 4);
> +	    read_memory_typed_address (current_frame_addr +
> +				       TYPE_ARRAY_UPPER_BOUND_VALUE (type),
> +				       builtin_type_void_data_ptr);
>  	  *upper_bound = read_memory_integer (ptr_to_upper_bound, 4);
>  	}
>        else
> 
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Jim Blandy]

This patch looks good to me, too.  I think it's fine to commit it.

Klee Dienes <klee@apple.com> writes:

> On Sunday, October 6, 2002, at 11:09 AM, Daniel Jacobowitz wrote:
> 
> >
> > Might want to use alloca (TYPE_LENGTH (builtin_type_void_data_ptr)).
> 
> Love to; I had been under the (apparently mistaken) impression that
> alloca was deprecated for new code.
> 
> >> Aside from the intimidating comments in findvar.c, this would seem a
> >> good candidate for extract_address ... though a similar modification
> >> would have to be made to it to handle the sign-extension as well.
> >>
> >> Or alternately, I could just leave well-enough alone, and be
> >> careful to
> >> truncate CORE_ADDRs to 32 bits in our target xfer_memory code (or the
> >> MIPS code could sign-extend it in the same place).
> >
> > Well, I think a general read_memory_typed_address (returning CORE_ADDR,
> > and honoroing POINTER_TO_ADDRESS) would be the way to go here.
> 
> That sounds good to me; how about the following?  I removed the patch
> to symfile.c, as it's commented out anyway, and probably not worth the
> effort.  My one concern is that I don't currently have a good system
> to test the part of the patch that affects blockframe.c.
> 
> 2002-10-06  Klee Dienes  <kdienes@apple.com>
> 
>          * findvar.c (read_memory_typed_address): New function.
>          * gdbcore.h (read_memory_typed_address): Add prototype.
>          * blockframe.c (sigtramp_saved_pc): Use
>          read_memory_typed_address to read a value destined for a
>          CORE_ADDR, not read_memory_integer.
>          * f-valprint.c (f77_get_dynamic_upperbound): Ditto.
>          (f77_get_dynamic_lowerbound): Ditto.
> 
> diff -u /home/klee/source/cygnus.cygnus/gdb/gdbcore.h ./gdbcore.h
> --- /home/klee/source/cygnus.cygnus/gdb/gdbcore.h	Wed Dec 19
> 15:54:26 2001
> +++ ./gdbcore.h	Sun Oct  6 09:18:11 2002
> @@ -64,8 +64,15 @@
> 
>   /* Read a null-terminated string from the debuggee's memory, given
> address,
>    * a buffer into which to place the string, and the maximum
> available space */
> +
>   extern void read_memory_string (CORE_ADDR, char *, int);
> 
> +/* Read the pointer of type TYPE at BUF, and return the address it
> +   represents. */
> +
> +CORE_ADDR
> +read_memory_typed_address (CORE_ADDR addr, struct type *type);
> +
>   /* This takes a char *, not void *.  This is probably right, because
>      passing in an int * or whatever is wrong with respect to
>      byteswapping, alignment, different sizes for host vs. target types,
> diff -u /home/klee/source/cygnus.cygnus/gdb/corefile.c ./corefile.c
> --- /home/klee/source/cygnus.cygnus/gdb/corefile.c	Thu May 30
> 15:41:24 2002
> +++ ./corefile.c	Sun Oct  6 09:18:11 2002
> @@ -356,6 +356,14 @@
>       }
>   }
> 
> +CORE_ADDR
> +read_memory_typed_address (CORE_ADDR addr, struct type *type)
> +{
> +  char *buf = alloca (TYPE_LENGTH (type));
> +  read_memory (addr, buf, TYPE_LENGTH (type));
> +  return extract_typed_address (buf, type);
> +}
> +
>   /* Same as target_write_memory, but report an error if can't write.  */
>   void
>   write_memory (CORE_ADDR memaddr, char *myaddr, int len)
> diff -u /home/klee/source/cygnus.cygnus/gdb/blockframe.c ./blockframe.c
> --- /home/klee/source/cygnus.cygnus/gdb/blockframe.c	Mon Sep 30
> 18:24:01 2002
> +++ ./blockframe.c	Sun Oct  6 09:35:42 2002
> @@ -1035,19 +1035,17 @@
>     buf = alloca (ptrbytes);
>     /* Get sigcontext address, it is the third parameter on the stack.
> */
>     if (frame->next)
> -    sigcontext_addr = read_memory_integer (FRAME_ARGS_ADDRESS
> (frame->next)
> -					   + FRAME_ARGS_SKIP
> -					   + sigcontext_offs,
> -					   ptrbytes);
> +    sigcontext_addr = read_memory_typed_address
> +      (FRAME_ARGS_ADDRESS (frame->next) + FRAME_ARGS_SKIP +
> sigcontext_offs,
> +       builtin_type_void_data_ptr);
>     else
> -    sigcontext_addr = read_memory_integer (read_register (SP_REGNUM)
> -					   + sigcontext_offs,
> -					   ptrbytes);
> +    sigcontext_addr = read_memory_typed_address
> +      (read_register (SP_REGNUM) + sigcontext_offs,
> builtin_type_void_data_ptr);
> 
>     /* Don't cause a memory_error when accessing sigcontext in case
> the stack
>        layout has changed or the stack is corrupt.  */
>     target_read_memory (sigcontext_addr + SIGCONTEXT_PC_OFFSET, buf,
> ptrbytes);
> -  return extract_unsigned_integer (buf, ptrbytes);
> +  return extract_typed_address (buf, builtin_type_void_data_ptr);
>   }
>   #endif /* SIGCONTEXT_PC_OFFSET */
> 
> diff -u /home/klee/source/cygnus.cygnus/gdb/f-valprint.c ./f-valprint.c
> --- /home/klee/source/cygnus.cygnus/gdb/f-valprint.c	Tue Mar  6
> 18:57:08 2001
> +++ ./f-valprint.c	Sun Oct  6 09:18:11 2002
> @@ -102,9 +102,9 @@
>         if (current_frame_addr > 0)
>   	{
>   	  ptr_to_lower_bound =
> -	    read_memory_integer (current_frame_addr +
> -				 TYPE_ARRAY_LOWER_BOUND_VALUE (type),
> -				 4);
> +	    read_memory_typed_address (current_frame_addr +
> +				       TYPE_ARRAY_LOWER_BOUND_VALUE (type),
> +				       builtin_type_void_data_ptr);
>   	  *lower_bound = read_memory_integer (ptr_to_lower_bound, 4);
>   	}
>         else
> @@ -165,9 +165,9 @@
>         if (current_frame_addr > 0)
>   	{
>   	  ptr_to_upper_bound =
> -	    read_memory_integer (current_frame_addr +
> -				 TYPE_ARRAY_UPPER_BOUND_VALUE (type),
> -				 4);
> +	    read_memory_typed_address (current_frame_addr +
> +				       TYPE_ARRAY_UPPER_BOUND_VALUE (type),
> +				       builtin_type_void_data_ptr);
>   	  *upper_bound = read_memory_integer (ptr_to_upper_bound, 4);
>   	}
>         else

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Andrew Cagney]

>    /* Don't cause a memory_error when accessing sigcontext in case the stack
>       layout has changed or the stack is corrupt.  */
>    target_read_memory (sigcontext_addr + SIGCONTEXT_PC_OFFSET, buf, ptrbytes);
> -  return extract_unsigned_integer (buf, ptrbytes);
> +  return extract_typed_address (buf, builtin_type_void_data_ptr);

This should be builtin_type_void_code_ptr since it is extracting a code 
pointer (the PC).  It, also, might as well use read_memory_typed_address().

Andrew

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Klee Dienes]

I used extract_type_address to honor the comment about not causing a 
memory_error.

I'll check in the change from builtin_type_void_data_ptr to 
builtin_type_void_code_ptr.

Unless someone objects, I'll also change

   int ptrbytes = TARGET_PTR_BIT / TARGET_CHAR_BIT;

to

   int ptrbytes = TYPE_LENGTH (builtin_type_void_code_ptr);

at the start of the function.

On Monday, October 21, 2002, at 03:05 PM, Andrew Cagney wrote:

>>    /* Don't cause a memory_error when accessing sigcontext in case 
>> the stack
>>       layout has changed or the stack is corrupt.  */
>>    target_read_memory (sigcontext_addr + SIGCONTEXT_PC_OFFSET, buf, 
>> ptrbytes);
>> -  return extract_unsigned_integer (buf, ptrbytes);
>> +  return extract_typed_address (buf, builtin_type_void_data_ptr);
>
> This should be builtin_type_void_code_ptr since it is extracting a 
> code pointer (the PC).  It, also, might as well use 
> read_memory_typed_address().
>
> Andrew
>
>

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Andrew Cagney]

> I used extract_type_address to honor the comment about not causing a memory_error.
> 
> I'll check in the change from builtin_type_void_data_ptr to builtin_type_void_code_ptr.

Thanks!

> Unless someone objects, I'll also change
> 
>   int ptrbytes = TARGET_PTR_BIT / TARGET_CHAR_BIT;
> 
> to
> 
>   int ptrbytes = TYPE_LENGTH (builtin_type_void_code_ptr);

Now this I like so much, I'm going to add it to the ARI as a recommendation.

Andrew

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Andrew Cagney]

> Perhaps something along the lines of the following?
> 
> {
>   unsigned char buf[sizeof (CORE_ADDR)];
>   if (target_read_memory (memaddr, buf, TYPE_LENGTH (builtin_type_void_data_ptr)) != 0)
>     error ("Unable to determine lower bound of array.");
>   *lower_bound = extract_typed_address (buf, builtin_type_void_data_ptr);
> }
> 
> That's a bit verbose, and it's not clear to me if sizeof (CORE_ADDR) is guaranteed to be large enough to contain a raw target pointer, but I can't really think of any better alternatives.
> 
> Aside from the intimidating comments in findvar.c, this would seem a good candidate for extract_address ... though a similar modification would have to be made to it to handle the sign-extension as well.
> 
> Or alternately, I could just leave well-enough alone, and be careful to truncate CORE_ADDRs to 32 bits in our target xfer_memory code (or the MIPS code could sign-extend it in the same place).

Just FYI, the MIPS tried to do this, and it lost badly!  It also turned 
out that the MIPS case was just a simplification of the more generic 
harvard case and hence the core of GDB is being modified, as people 
notice, to use the pointer to address and address to pointer functions.

Andrew

Re: [PATCH] Use read_memory_unsigned_integer when reading to CORE_ADDR

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> Andrew, is my understanding right?  Is there some reason this isn't a
> problem?

My memory of what Andrew has said in the past agrees with what you've
said.