[RFA] hpread.c cleanups

[RFA] hpread.c cleanups

[Randolph Chung]

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

The hppa target used to use a objfile private field to store some shared
object information on HPUX. Most of that have been cleaned up, but there
is one more instance in hpread.c. In fact, the current code in cvs will
cause a crash because the objfile private field is not initialized. The
patch changes hpread.c to use the solist interface. Tested lightly on
hppa64-hp-hpux11.11. OK?

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

[-- Attachment #2: hpread.diff --]
[-- Type: text/plain, Size: 2651 bytes --]

2006-03-01  Randolph Chung  <tausq@debian.org>

	* hpread.c (hpread_process_one_debug_symbol): Use solist interface to
	retrieve shared object information.
	* hppa-tdep.h (hppa_objfile_private): Remove unused so_info field.
	* Makefile.in (hpread.o): Update dependencies.

Index: hpread.c
===================================================================
RCS file: /cvs/src/src/gdb/hpread.c,v
retrieving revision 1.58
diff -u -p -r1.58 hpread.c
--- hpread.c	17 Dec 2005 22:34:01 -0000	1.58
+++ hpread.c	1 Mar 2006 05:52:14 -0000
@@ -36,6 +36,7 @@
 #include "gdbtypes.h"
 #include "demangle.h"
 #include "solib-som.h"
+#include "solist.h"
 #include "gdb_assert.h"
 #include "hppa-tdep.h"
 
@@ -5764,13 +5765,9 @@ hpread_process_one_debug_symbol (union d
 	       */
 	      struct so_list *so;
               struct hppa_objfile_private *priv;
+	      struct target_so_ops *ops = current_target_so_ops;
 
-              priv = (struct hppa_objfile_private *)
-	        objfile_data (objfile, hppa_objfile_priv_data);
-	      if (priv == NULL)
-		error (_("Internal error in reading shared library information."));
-
-	      so = ((struct hppa_objfile_private *) priv)->so_info;
+	      so = ops->current_sos ();
 	      if (so == NULL)
 		error (_("Internal error in reading shared library information."));
 
Index: hppa-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.h,v
retrieving revision 1.26
diff -u -p -r1.26 hppa-tdep.h
--- hppa-tdep.h	1 Mar 2006 05:43:03 -0000	1.26
+++ hppa-tdep.h	1 Mar 2006 05:52:14 -0000
@@ -211,7 +211,6 @@ struct hppa_unwind_info
 struct hppa_objfile_private
   {
     struct hppa_unwind_info *unwind_info;	/* a pointer */
-    struct so_list *so_info;	/* a pointer  */
     CORE_ADDR dp;
 
     int dummy_call_sequence_reg;
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.789
diff -u -p -r1.789 Makefile.in
--- Makefile.in	28 Feb 2006 22:38:03 -0000	1.789
+++ Makefile.in	1 Mar 2006 05:55:07 -0000
@@ -2039,7 +2039,8 @@ hppa-tdep.o: hppa-tdep.c $(defs_h) $(bfd
 hpread.o: hpread.c $(defs_h) $(bfd_h) $(gdb_string_h) $(hp_symtab_h) \
 	$(syms_h) $(symtab_h) $(symfile_h) $(objfiles_h) $(buildsym_h) \
 	$(complaints_h) $(gdb_stabs_h) $(gdbtypes_h) $(demangle_h) \
-	$(solib_som_h) $(gdb_assert_h) $(hppa_tdep_h) $(gdb_string_h)
+	$(solib_som_h) $(gdb_assert_h) $(hppa_tdep_h) $(gdb_string_h) \
+	$(solist_h)
 hpux-thread.o: hpux-thread.c $(defs_h) $(gdbthread_h) $(target_h) \
 	$(inferior_h) $(regcache_h) $(gdb_stat_h) $(gdbcore_h) \
 	$(hppa_tdep_h)

Re: [RFA] hpread.c cleanups

[Daniel Jacobowitz]

On Wed, Mar 01, 2006 at 01:58:39PM +0800, Randolph Chung wrote:
> The hppa target used to use a objfile private field to store some shared
> object information on HPUX. Most of that have been cleaned up, but there
> is one more instance in hpread.c. In fact, the current code in cvs will
> cause a crash because the objfile private field is not initialized. The
> patch changes hpread.c to use the solist interface. Tested lightly on
> hppa64-hp-hpux11.11. OK?

> 2006-03-01  Randolph Chung  <tausq@debian.org>
> 
> 	* hpread.c (hpread_process_one_debug_symbol): Use solist interface to
> 	retrieve shared object information.
> 	* hppa-tdep.h (hppa_objfile_private): Remove unused so_info field.
> 	* Makefile.in (hpread.o): Update dependencies.

It looks plausible to me, but you may want to check with Kevin if using
solist.h this way is OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] hpread.c cleanups

[Kevin Buettner]

On Wed, 01 Mar 2006 13:58:39 +0800
Randolph Chung <randolph@tausq.org> wrote:

> @@ -5764,13 +5765,9 @@ hpread_process_one_debug_symbol (union d
>  	       */
>  	      struct so_list *so;
>                struct hppa_objfile_private *priv;
> +	      struct target_so_ops *ops = current_target_so_ops;
>  
> -              priv = (struct hppa_objfile_private *)
> -	        objfile_data (objfile, hppa_objfile_priv_data);
> -	      if (priv == NULL)
> -		error (_("Internal error in reading shared library information."));
> -
> -	      so = ((struct hppa_objfile_private *) priv)->so_info;
> +	      so = ops->current_sos ();
>  	      if (so == NULL)
>  		error (_("Internal error in reading shared library information."));

I don't think it's a good idea to call current_sos() here.  Could you
try calling master_so_list() instead?  (It's declared in solist.h.)

The problem with calling current_sos() is 1) it's slow since it hits
target memory, and 2) you haven't freed the storage allocated by calling
current_sos().  If you find that you really need current_sos() for some
reason, you should add some code to deallocate the storage when you're
done with it.

Kevin

Re: [RFA] hpread.c cleanups

[Randolph Chung]

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

> On Wed, Mar 01, 2006 at 01:58:39PM +0800, Randolph Chung wrote:
> 
>>The hppa target used to use a objfile private field to store some shared
>>object information on HPUX. Most of that have been cleaned up, but there
>>is one more instance in hpread.c. In fact, the current code in cvs will
>>cause a crash because the objfile private field is not initialized. The
>>patch changes hpread.c to use the solist interface. Tested lightly on
>>hppa64-hp-hpux11.11. OK?

I've redone this patch to be cleaner and also fixed a bug in my original 
proposal. The old code actually would return the TLS address from the 
so_list object that corresponded to a specific objfile, whereas my last 
patch made it return the TLS address from the first so_list object in 
the master_so_list(). This new patch:

- removes the dependency on obj_private data
- passes the objfile from the symbol reader to the solib interface to 
look up the corresponding TLS information

Tested on hppa2.0w-hp-hpux11.11 and hppa64-hp-hpux11.11. ok?

randolph

[-- Attachment #2: tls.diff --]
[-- Type: text/x-patch, Size: 4581 bytes --]

2006-03-26  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.h (gdbarch_tdep): Make solib_thread_start_addr take
	an objfile argument instead of a so_list one.
	(hppa_objfile_private): Remove unusued so_info argument.
	* hpread.c (hpread_process_one_debug_symbol): Use objfile to call into
	solib_thread_start_addr method to get TLS address.  Cleanup comments.
	* solib-pa64.c (pa64_solib_thread_start_addr): Take objfile argument;
	return TLS address from corresponding so_list object.
	* solib-som.c (som_solib_thread_start_addr): Likewise.

Index: hppa-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.h,v
retrieving revision 1.26
diff -u -p -r1.26 hppa-tdep.h
--- hppa-tdep.h	1 Mar 2006 05:43:03 -0000	1.26
+++ hppa-tdep.h	26 Mar 2006 08:22:26 -0000
@@ -110,7 +110,7 @@ struct gdbarch_tdep
 
   /* These are solib-dependent methods.  They are really HPUX only, but
      we don't have a HPUX-specific tdep vector at the moment.  */
-  CORE_ADDR (*solib_thread_start_addr) (struct so_list *so);
+  CORE_ADDR (*solib_thread_start_addr) (struct objfile *objfile);
   CORE_ADDR (*solib_get_got_by_pc) (CORE_ADDR addr);
   CORE_ADDR (*solib_get_solib_by_pc) (CORE_ADDR addr);
   CORE_ADDR (*solib_get_text_base) (struct objfile *objfile);
@@ -211,7 +211,6 @@ struct hppa_unwind_info
 struct hppa_objfile_private
   {
     struct hppa_unwind_info *unwind_info;	/* a pointer */
-    struct so_list *so_info;	/* a pointer  */
     CORE_ADDR dp;
 
     int dummy_call_sequence_reg;
Index: hpread.c
===================================================================
RCS file: /cvs/src/src/gdb/hpread.c,v
retrieving revision 1.58
diff -u -p -r1.58 hpread.c
--- hpread.c	17 Dec 2005 22:34:01 -0000	1.58
+++ hpread.c	26 Mar 2006 08:22:27 -0000
@@ -5759,28 +5758,13 @@ hpread_process_one_debug_symbol (union d
 	       * This variable is not only thread local but
 	       * in a shared library.
 	       *
-	       * Alas, the shared lib structures are private
-	       * to "somsolib.c".  But C lets us point to one.
-	       */
-	      struct so_list *so;
-              struct hppa_objfile_private *priv;
-
-              priv = (struct hppa_objfile_private *)
-	        objfile_data (objfile, hppa_objfile_priv_data);
-	      if (priv == NULL)
-		error (_("Internal error in reading shared library information."));
-
-	      so = ((struct hppa_objfile_private *) priv)->so_info;
-	      if (so == NULL)
-		error (_("Internal error in reading shared library information."));
-
-	      /* Thread-locals in shared libraries do NOT have the
+	       * Thread-locals in shared libraries do NOT have the
 	       * standard offset ("data_offset"), so we re-calculate
 	       * where to look for this variable, using a call-back
 	       * to interpret the private shared-library data.
 	       */
 	      SYMBOL_VALUE_ADDRESS (sym) = dn_bufp->dsvar.location +
-	        gdbarch_tdep (current_gdbarch)->solib_thread_start_addr (so);
+	        gdbarch_tdep (current_gdbarch)->solib_thread_start_addr (objfile);
 	    }
 	}
       break;
Index: solib-pa64.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-pa64.c,v
retrieving revision 1.5
diff -u -p -r1.5 solib-pa64.c
--- solib-pa64.c	17 Dec 2005 22:34:02 -0000	1.5
+++ solib-pa64.c	26 Mar 2006 08:22:27 -0000
@@ -594,9 +595,15 @@ pa64_solib_get_got_by_pc (CORE_ADDR addr
 
 /* Get some HPUX-specific data from a shared lib.  */
 static CORE_ADDR
-pa64_solib_thread_start_addr (struct so_list *so)
+pa64_solib_thread_start_addr (struct objfile *objfile)
 {
-  return so->lm_info->desc.tls_start_addr;
+  struct so_list *so;
+
+  for (so = master_so_list (); so; so = so->next)
+    if (so->objfile == objfile)
+      return so->lm_info->desc.tls_start_addr;
+  
+  return 0;
 }
 
 
Index: solib-som.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-som.c,v
retrieving revision 1.9
diff -u -p -r1.9 solib-som.c
--- solib-som.c	24 Mar 2006 23:49:56 -0000	1.9
+++ solib-som.c	26 Mar 2006 08:22:27 -0000
@@ -695,9 +698,15 @@ som_free_so (struct so_list *so)
 }
 
 static CORE_ADDR
-som_solib_thread_start_addr (struct so_list *so)
+som_solib_thread_start_addr (struct objfile *objfile)
 {
-  return so->lm_info->tsd_start_addr;
+  struct so_list *so;
+
+  for (so = master_so_list (); so; so = so->next)
+    if (so->objfile == objfile)
+      return so->lm_info->tsd_start_addr;
+  
+  return 0;
 }
 
 /* Return the GOT value for the shared library in which ADDR belongs.  If