[patch rfc] s/extract_address/extract_unsigned_integer/ for solib

[patch rfc] s/extract_address/extract_unsigned_integer/ for solib

[Andrew Cagney]

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

Hello,

This does the s/extract_address/extract_unsigned_integer/ change to the 
solib files.

One bit bugs me - IRIX.  It's doing an unsigned extract when MIPS is 
ment to always sign extend an address.  It's very tempting to instead 
make that code always do signextended extracts (I don't have access to 
an IRIX 4 box though).

thoughts?
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 7916 bytes --]

2003-05-22  Andrew Cagney  <cagney@redhat.com>

	* solib-irix.c (extract_mips_address): Inline extract_address,
	replacing it with extract_unsigned_integer.
	* solib-svr4.c (SOLIB_EXTRACT_ADDRESS): Ditto.
	(LM_NAME, IGNORE_FIRST_LINK_MAP_ENTRY): Ditto.
	(first_link_map_member, open_symbol_file_object): Ditto.
	(svr4_fetch_objfile_link_map, svr4_fetch_objfile_link_map): Ditto.
	* solib-sunos.c (SOLIB_EXTRACT_ADDRESS): Ditto.
	(LM_NEXT, LM_NAME): Ditto.

Index: solib-irix.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-irix.c,v
retrieving revision 1.3
diff -u -r1.3 solib-irix.c
--- solib-irix.c	8 Apr 2003 19:21:15 -0000	1.3
+++ solib-irix.c	22 May 2003 18:57:41 -0000
@@ -124,7 +124,7 @@
 /* MIPS sign extends its 32 bit addresses.  We could conceivably use
    extract_typed_address here, but to do so, we'd have to construct an
    appropriate type.  Calling extract_signed_integer or
-   extract_address seems simpler.  */
+   extract_unsigned_integer seems simpler.  */
 
 static CORE_ADDR
 extract_mips_address (void *addr, int len)
@@ -132,7 +132,11 @@
   if (len <= 32)
     return extract_signed_integer (addr, len);
   else
-    return extract_address (addr, len);
+    /* FIXME: cagney/2003-05-22: Is this right?  MIPS addresses are
+       always signed extended right?  Originally this was a call to
+       extract_address but that was just extract_unsigned_integer so
+       something weired is going on.  */
+    return extract_unsigned_integer (addr, len);
 }
 
 /* Fetch and return the link map data associated with ADDR.  Note that
Index: solib-sunos.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-sunos.c,v
retrieving revision 1.9
diff -u -r1.9 solib-sunos.c
--- solib-sunos.c	8 Apr 2003 19:21:15 -0000	1.9
+++ solib-sunos.c	22 May 2003 18:57:42 -0000
@@ -68,14 +68,16 @@
   NULL
 };
 
-/* Macro to extract an address from a solib structure.
-   When GDB is configured for some 32-bit targets (e.g. Solaris 2.7
-   sparc), BFD is configured to handle 64-bit targets, so CORE_ADDR is
-   64 bits.  We have to extract only the significant bits of addresses
-   to get the right address when accessing the core file BFD.  */
+/* Macro to extract an address from a solib structure.  When GDB is
+   configured for some 32-bit targets (e.g. Solaris 2.7 sparc), BFD is
+   configured to handle 64-bit targets, so CORE_ADDR is 64 bits.  We
+   have to extract only the significant bits of addresses to get the
+   right address when accessing the core file BFD.
+
+   Assume that the address is unsigned.  */
 
 #define SOLIB_EXTRACT_ADDRESS(MEMBER) \
-	extract_address (&(MEMBER), sizeof (MEMBER))
+	extract_unsigned_integer (&(MEMBER), sizeof (MEMBER))
 
 /* local data declarations */
 
@@ -108,7 +110,9 @@
   int lm_next_offset = offsetof (struct link_map, lm_next);
   int lm_next_size = fieldsize (struct link_map, lm_next);
 
-  return extract_address (so->lm_info->lm + lm_next_offset, lm_next_size);
+  /* Assume that the address is unsigned.  */
+  return extract_unsigned_integer (so->lm_info->lm + lm_next_offset,
+				   lm_next_size);
 }
 
 static CORE_ADDR
@@ -117,7 +121,9 @@
   int lm_name_offset = offsetof (struct link_map, lm_name);
   int lm_name_size = fieldsize (struct link_map, lm_name);
 
-  return extract_address (so->lm_info->lm + lm_name_offset, lm_name_size);
+  /* Assume that the address is unsigned.  */
+  return extract_unsigned_integer (so->lm_info->lm + lm_name_offset,
+				   lm_name_size);
 }
 
 static CORE_ADDR debug_base;	/* Base of dynamic linker structures */
Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.32
diff -u -r1.32 solib-svr4.c
--- solib-svr4.c	15 Apr 2003 00:28:23 -0000	1.32
+++ solib-svr4.c	22 May 2003 18:57:42 -0000
@@ -105,14 +105,16 @@
   NULL
 };
 
-/* Macro to extract an address from a solib structure.
-   When GDB is configured for some 32-bit targets (e.g. Solaris 2.7
-   sparc), BFD is configured to handle 64-bit targets, so CORE_ADDR is
-   64 bits.  We have to extract only the significant bits of addresses
-   to get the right address when accessing the core file BFD.  */
+/* Macro to extract an address from a solib structure.  When GDB is
+   configured for some 32-bit targets (e.g. Solaris 2.7 sparc), BFD is
+   configured to handle 64-bit targets, so CORE_ADDR is 64 bits.  We
+   have to extract only the significant bits of addresses to get the
+   right address when accessing the core file BFD.
+
+   Assume that the address is unsigned.  */
 
 #define SOLIB_EXTRACT_ADDRESS(MEMBER) \
-	extract_address (&(MEMBER), sizeof (MEMBER))
+	extract_unsigned_integer (&(MEMBER), sizeof (MEMBER))
 
 /* local data declarations */
 
@@ -132,7 +134,9 @@
 {
   struct link_map_offsets *lmo = SVR4_FETCH_LINK_MAP_OFFSETS ();
 
-  return extract_address (so->lm_info->lm + lmo->l_next_offset, lmo->l_next_size);
+  /* Assume that the address is unsigned.  */
+  return extract_unsigned_integer (so->lm_info->lm + lmo->l_next_offset,
+				   lmo->l_next_size);
 }
 
 static CORE_ADDR
@@ -140,7 +144,9 @@
 {
   struct link_map_offsets *lmo = SVR4_FETCH_LINK_MAP_OFFSETS ();
 
-  return extract_address (so->lm_info->lm + lmo->l_name_offset, lmo->l_name_size);
+  /* Assume that the address is unsigned.  */
+  return extract_unsigned_integer (so->lm_info->lm + lmo->l_name_offset,
+				   lmo->l_name_size);
 }
 
 static int
@@ -148,8 +154,9 @@
 {
   struct link_map_offsets *lmo = SVR4_FETCH_LINK_MAP_OFFSETS ();
 
-  return extract_address (so->lm_info->lm + lmo->l_prev_offset,
-                          lmo->l_prev_size) == 0;
+  /* Assume that the address is unsigned.  */
+  return extract_unsigned_integer (so->lm_info->lm + lmo->l_prev_offset,
+				   lmo->l_prev_size) == 0;
 }
 
 static CORE_ADDR debug_base;	/* Base of dynamic linker structures */
@@ -587,7 +594,8 @@
 
   read_memory (debug_base + lmo->r_map_offset, r_map_buf, lmo->r_map_size);
 
-  lm = extract_address (r_map_buf, lmo->r_map_size);
+  /* Assume that the address is unsigned.  */
+  lm = extract_unsigned_integer (r_map_buf, lmo->r_map_size);
 
   /* FIXME:  Perhaps we should validate the info somehow, perhaps by
      checking r_version for a known version number, or r_state for
@@ -645,8 +653,9 @@
   /* Read address of name from target memory to GDB.  */
   read_memory (lm + lmo->l_name_offset, l_name_buf, lmo->l_name_size);
 
-  /* Convert the address to host format.  */
-  l_name = extract_address (l_name_buf, lmo->l_name_size);
+  /* Convert the address to host format.  Assume that the address is
+     unsigned.  */
+  l_name = extract_unsigned_integer (l_name_buf, lmo->l_name_size);
 
   /* Free l_name_buf.  */
   do_cleanups (cleanups);
@@ -820,9 +829,9 @@
       /* Read address of name from target memory to GDB.  */
       read_memory (lm + lmo->l_name_offset, l_name_buf, lmo->l_name_size);
 
-      /* Extract this object's name.  */
-      name_address = extract_address (l_name_buf,
-				      lmo->l_name_size);
+      /* Extract this object's name.  Assume that the address is
+         unsigned.  */
+      name_address = extract_unsigned_integer (l_name_buf, lmo->l_name_size);
       target_read_string (name_address, &buffer,
       			  SO_NAME_MAX_PATH_SIZE - 1, &errcode);
       make_cleanup (xfree, buffer);
@@ -843,9 +852,10 @@
     	      return lm;
       	    }
   	}
-      /* Not the file we wanted, continue checking.  */
-      lm = extract_address (objfile_lm_info.lm + lmo->l_next_offset,
-			    lmo->l_next_size);
+      /* Not the file we wanted, continue checking.  Assume that the
+         address is unsigned.  */
+      lm = extract_unsigned_integer (objfile_lm_info.lm + lmo->l_next_offset,
+				     lmo->l_next_size);
       do_cleanups (old_chain);
     }
   return 0;

Re: [patch rfc] s/extract_address/extract_unsigned_integer/ for solib

[Kevin Buettner]

On May 22,  3:01pm, Andrew Cagney wrote:

> This does the s/extract_address/extract_unsigned_integer/ change to the 
> solib files.
> 
> One bit bugs me - IRIX.  It's doing an unsigned extract when MIPS is 
> ment to always sign extend an address.  It's very tempting to instead 
> make that code always do signextended extracts (I don't have access to 
> an IRIX 4 box though).
> 
> thoughts?

It looks okay to me.

I agree that extract_mips_address() in solib-irix.c probably ought to
be using extract_signed_integer(), but I too am reluctant to change it
without testing.

Why do you need an IRIX 4 box?  Wouldn't IRIX 6 work?

Kevin

Re: [patch rfc] s/extract_address/extract_unsigned_integer/ for solib

[Andrew Cagney]

> On May 22,  3:01pm, Andrew Cagney wrote:
> 
> 
>> This does the s/extract_address/extract_unsigned_integer/ change to the 
>> solib files.
>> 
>> One bit bugs me - IRIX.  It's doing an unsigned extract when MIPS is 
>> ment to always sign extend an address.  It's very tempting to instead 
>> make that code always do signextended extracts (I don't have access to 
>> an IRIX 4 box though).
>> 
>> thoughts?
> 
> 
> It looks okay to me.
> 
> I agree that extract_mips_address() in solib-irix.c probably ought to
> be using extract_signed_integer(), but I too am reluctant to change it
> without testing.
> 
> Why do you need an IRIX 4 box?  Wouldn't IRIX 6 work?

Do you have a good test case?  On IRIX 6, shlib-call.exp fails with:

FAIL: gdb.base/shlib-call.exp: print shr1(1)
FAIL: gdb.base/shlib-call.exp: print shr1(g)
FAIL: gdb.base/shlib-call.exp: run until breakpoint set at a function
FAIL: gdb.base/shlib-call.exp: print shr1(1) 2nd time
FAIL: gdb.base/shlib-call.exp: step out of shr2
FAIL: gdb.base/shlib-call.exp: print  mainshr1(1) from main
FAIL: gdb.base/shlib-call.exp: step into mainshr1

with/without the change.  The other solib* tests are hp specific :-/

Andrew

Re: [patch rfc] s/extract_address/extract_unsigned_integer/ for solib

[Andrew Cagney]

> On May 22,  3:01pm, Andrew Cagney wrote:
> 
> 
> This does the s/extract_address/extract_unsigned_integer/ change to the solib files.
> 
> One bit bugs me - IRIX.  It's doing an unsigned extract when MIPS is ment to always sign extend an address.  It's very tempting to instead make that code always do signextended extracts (I don't have access to an IRIX 4 box though).
> 
> thoughts?
> 
> 
> It looks okay to me.
> 
> I agree that extract_mips_address() in solib-irix.c probably ought to
> be using extract_signed_integer(), but I too am reluctant to change it
> without testing.
> 
> Why do you need an IRIX 4 box?  Wouldn't IRIX 6 work?
> 
> Do you have a good test case?  On IRIX 6, shlib-call.exp fails with:
> 
> FAIL: gdb.base/shlib-call.exp: print shr1(1)
> FAIL: gdb.base/shlib-call.exp: print shr1(g)
> FAIL: gdb.base/shlib-call.exp: run until breakpoint set at a function
> FAIL: gdb.base/shlib-call.exp: print shr1(1) 2nd time
> FAIL: gdb.base/shlib-call.exp: step out of shr2
> FAIL: gdb.base/shlib-call.exp: print  mainshr1(1) from main
> FAIL: gdb.base/shlib-call.exp: step into mainshr1
> 
> with/without the change.  The other solib* tests are hp specific :-/

I fixed these failures.  It now passes, even with extract_signed_address 
so I've committed that.

Andrew