[rfa:solib] Handle start-address descriptors

[rfa:solib] Handle start-address descriptors

[Andrew Cagney]

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

Hello,

The attached patch modifies the solib's computation of the exec's 
entry-point address so that it applies convert_from_func_ptr_addr() to 
the bfd start-address.  That way, when the bfd's start-address points at 
a descriptor, the true exec entry point is still obtained.

Fixes ppc64's ability to debug shared libraries.

Ok?

Andrew

PS: Ref
http://sources.redhat.com/ml/gdb-patches/2003-10/msg00670.html

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

2003-10-27  Andrew Cagney  <cagney@redhat.com>

	* solib-svr4.c: Update copyright.  Include "bfd-target.h" and
	"exec.h".
	(exec_entry_point): New function.
	(enable_break): Create a "tmp_bfd_target", use that and
	entry_point_address when computing the relocation offset.
	(svr4_relocate_main_executable): Ditto with exec_bfd and exec_ops.
	* Makefile.in (solib-svr4.o): Update dependencies.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.463
diff -u -r1.463 Makefile.in
--- Makefile.in	24 Oct 2003 20:24:05 -0000	1.463
+++ Makefile.in	27 Oct 2003 18:39:19 -0000
@@ -2290,7 +2297,8 @@
 	$(bcache_h) $(regcache_h)
 solib-svr4.o: solib-svr4.c $(defs_h) $(elf_external_h) $(elf_common_h) \
 	$(elf_mips_h) $(symtab_h) $(bfd_h) $(symfile_h) $(objfiles_h) \
-	$(gdbcore_h) $(target_h) $(inferior_h) $(solist_h) $(solib_svr4_h)
+	$(gdbcore_h) $(target_h) $(inferior_h) $(solist_h) $(solib_svr4_h) \
+	$(bfd_target_h) $(exec_h)
 sol-thread.o: sol-thread.c $(defs_h) $(gdbthread_h) $(target_h) \
 	$(inferior_h) $(gdb_stat_h) $(gdbcmd_h) $(gdbcore_h) $(regcache_h) \
 	$(symfile_h) $(gdb_string_h) $(gregset_h)
Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.45
diff -u -r1.45 ppc-linux-tdep.c
--- ppc-linux-tdep.c	24 Oct 2003 20:24:06 -0000	1.45
+++ ppc-linux-tdep.c	27 Oct 2003 18:39:20 -0000
@@ -1083,7 +1083,12 @@
 void
 _initialize_ppc_linux_tdep (void)
 {
-  gdbarch_register_osabi (bfd_arch_powerpc, 0, GDB_OSABI_LINUX,
+  /* Accept any machine type, 32-bit or 64-bit, or even POWER.  */
+  gdbarch_register_osabi (bfd_arch_powerpc, bfd_mach_ppc, GDB_OSABI_LINUX,
+			  ppc_linux_init_abi);
+  gdbarch_register_osabi (bfd_arch_powerpc, bfd_mach_ppc64, GDB_OSABI_LINUX,
+			  ppc_linux_init_abi);
+  gdbarch_register_osabi (bfd_arch_rs6000, bfd_mach_rs6k, GDB_OSABI_LINUX,
 			  ppc_linux_init_abi);
   add_core_fns (&ppc_linux_regset_core_fns);
 }
Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.37
diff -u -r1.37 solib-svr4.c
--- solib-svr4.c	26 Aug 2003 23:35:19 -0000	1.37
+++ solib-svr4.c	27 Oct 2003 18:39:20 -0000
@@ -1,7 +1,7 @@
 /* Handle SVR4 shared libraries for GDB, the GNU Debugger.
-   Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1998, 1999, 2000,
-   2001
-   Free Software Foundation, Inc.
+
+   Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1998, 1999,
+   2000, 2001, 2003 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -37,6 +37,9 @@
 #include "solist.h"
 #include "solib-svr4.h"
 
+#include "bfd-target.h"
+#include "exec.h"
+
 #ifndef SVR4_FETCH_LINK_MAP_OFFSETS
 #define SVR4_FETCH_LINK_MAP_OFFSETS() svr4_fetch_link_map_offsets ()
 #endif
@@ -911,6 +914,16 @@
 	  || in_plt_section (pc, NULL));
 }
 
+/* Given an executable's ABFD and target, compute the entry-point
+   address.  */
+
+static CORE_ADDR
+exec_entry_point (struct bfd *abfd, struct target_ops *targ)
+{
+  return gdbarch_convert_from_func_ptr_addr (current_gdbarch,
+					     bfd_get_start_address (abfd),
+					     targ);
+}
 
 /*
 
@@ -984,6 +997,7 @@
       int load_addr_found = 0;
       struct so_list *inferior_sos;
       bfd *tmp_bfd = NULL;
+      struct target_ops *tmp_bfd_target;
       int tmp_fd = -1;
       char *tmp_pathname = NULL;
       CORE_ADDR sym_addr = 0;
@@ -1019,6 +1033,11 @@
 	  goto bkpt_at_symbol;
 	}
 
+      /* Now convert the TMP_BFD into a target.  That way target, as
+         well as BFD operations can be used.  Note that closing the
+         target will also close the underlying bfd.  */
+      tmp_bfd_target = target_bfd_reopen (tmp_bfd);
+
       /* If the entry in _DYNAMIC for the dynamic linker has already
          been filled in, we can read its base address from there. */
       inferior_sos = svr4_current_sos ();
@@ -1042,7 +1061,8 @@
 	 the current pc (which should point at the entry point for the
 	 dynamic linker) and subtracting the offset of the entry point.  */
       if (!load_addr_found)
-	load_addr = read_pc () - tmp_bfd->start_address;
+	load_addr = (read_pc ()
+		     - exec_entry_point (tmp_bfd, tmp_bfd_target));
 
       /* Record the relocated start and end address of the dynamic linker
          text and plt section for svr4_in_dynsym_resolve_code.  */
@@ -1080,8 +1100,9 @@
 	    break;
 	}
 
-      /* We're done with the temporary bfd.  */
-      bfd_close (tmp_bfd);
+      /* We're done with both the temporary bfd and target.  Remember,
+         closing the target closes the underlying bfd.  */
+      target_close (tmp_bfd_target, 0);
 
       if (sym_addr != 0)
 	{
@@ -1200,7 +1221,7 @@
   interp_sect = bfd_get_section_by_name (exec_bfd, ".interp");
   if (interp_sect == NULL 
       && (bfd_get_file_flags (exec_bfd) & DYNAMIC) != 0
-      && bfd_get_start_address (exec_bfd) != pc)
+      && (exec_entry_point (exec_bfd, &exec_ops) != pc))
     {
       struct cleanup *old_chain;
       struct section_offsets *new_offsets;
@@ -1232,7 +1253,7 @@
 	 The same language also appears in Edition 4.0 of the System V
 	 ABI and is left unspecified in some of the earlier editions.  */
 
-      displacement = pc - bfd_get_start_address (exec_bfd);
+      displacement = pc - exec_entry_point (exec_bfd, &exec_ops);
       changed = 0;
 
       new_offsets = xcalloc (symfile_objfile->num_sections,

Re: [rfa:solib] Handle start-address descriptors

[Kevin Buettner]

On Oct 27,  1:45pm, Andrew Cagney wrote:

> 2003-10-27  Andrew Cagney  <cagney@redhat.com>
> 
> 	* solib-svr4.c: Update copyright.  Include "bfd-target.h" and
> 	"exec.h".
> 	(exec_entry_point): New function.
> 	(enable_break): Create a "tmp_bfd_target", use that and
> 	entry_point_address when computing the relocation offset.
> 	(svr4_relocate_main_executable): Ditto with exec_bfd and exec_ops.
> 	* Makefile.in (solib-svr4.o): Update dependencies.
[...]
> Index: ppc-linux-tdep.c

If you're asking for approval for your ppc-linux-tdep.c changes, could
you please include ChangeLog entries for them?

Kevin

Re: [rfa:solib] Handle start-address descriptors

[Andrew Cagney]

> On Oct 27,  1:45pm, Andrew Cagney wrote:
> 
> 
>> 2003-10-27  Andrew Cagney  <cagney@redhat.com>
>> 
>> 	* solib-svr4.c: Update copyright.  Include "bfd-target.h" and
>> 	"exec.h".
>> 	(exec_entry_point): New function.
>> 	(enable_break): Create a "tmp_bfd_target", use that and
>> 	entry_point_address when computing the relocation offset.
>> 	(svr4_relocate_main_executable): Ditto with exec_bfd and exec_ops.
>> 	* Makefile.in (solib-svr4.o): Update dependencies.
> 
> [...]
> 
>> Index: ppc-linux-tdep.c
> 
> 
> If you're asking for approval for your ppc-linux-tdep.c changes, could
> you please include ChangeLog entries for them?

Just what's listed in the ChangeLog (I didn't trim enough fat).  There's 
already an RFA for the ppc-linux-tdep.c change here:
http://sources.redhat.com/ml/gdb-patches/2003-10/msg00711.html

Andrew

Re: [rfa:solib] Handle start-address descriptors

[Kevin Buettner]

On Oct 27,  1:45pm, Andrew Cagney wrote:

> Index: solib-svr4.c
...
> +#include "bfd-target.h"
> +#include "exec.h"

I'm surprised that you needed to include exec.h.  solib-svr4.c already
includes target.h and I would've thought this to be sufficient.  If
exec.h isn't needed, please take it out.  (Don't forget to fix Makefile.in.)

> +/* Given an executable's ABFD and target, compute the entry-point
> +   address.  */
> +
> +static CORE_ADDR
> +exec_entry_point (struct bfd *abfd, struct target_ops *targ)
> +{

Could you add a comment here telling why
gdbarch_convert_from_func_ptr_addr() is needed.  Maybe something like
this?

  /* For most targets, the address returned by bfd_get_start_address()
     is the entry point for the start function.  But, for some targets,
     bfd_get_start_address() returns the address of a function descriptor
     from which the entry point address may be extracted.  This address
     is extracted by gdbarch_convert_from_func_ptr_addr().  The method
     gdbarch_convert_from_func_ptr_addr() is the merely the identify
     function for targets which don't use function descriptors.  */

Hmm, a possible problem...  What happens when the target uses function
descriptors, but not for the exec file's start address?  I'm wondering
(ugh) if a separate gdbarch method is required for obtaining the start
address.

> +  return gdbarch_convert_from_func_ptr_addr (current_gdbarch,
> +					     bfd_get_start_address (abfd),
> +					     targ);
> +}

Kevin

Re: [rfa:solib] Handle start-address descriptors

[Andrew Cagney]

> On Oct 27,  1:45pm, Andrew Cagney wrote:
> 
> 
>> Index: solib-svr4.c
> 
> ...
> 
>> +#include "bfd-target.h"
>> +#include "exec.h"
> 
> 
> I'm surprised that you needed to include exec.h.  solib-svr4.c already
> includes target.h and I would've thought this to be sufficient.  If
> exec.h isn't needed, please take it out.  (Don't forget to fix Makefile.in.)

They are both needed.

>> +/* Given an executable's ABFD and target, compute the entry-point
>> +   address.  */
>> +
>> +static CORE_ADDR
>> +exec_entry_point (struct bfd *abfd, struct target_ops *targ)
>> +{
> 
> 
> Could you add a comment here telling why
> gdbarch_convert_from_func_ptr_addr() is needed.  Maybe something like
> this?

I'll add your comment.

>   /* For most targets, the address returned by bfd_get_start_address()
>      is the entry point for the start function.  But, for some targets,
>      bfd_get_start_address() returns the address of a function descriptor
>      from which the entry point address may be extracted.  This address
>      is extracted by gdbarch_convert_from_func_ptr_addr().  The method
>      gdbarch_convert_from_func_ptr_addr() is the merely the identify
>      function for targets which don't use function descriptors.  */
> 
> Hmm, a possible problem...  What happens when the target uses function
> descriptors, but not for the exec file's start address?  I'm wondering
> (ugh) if a separate gdbarch method is required for obtaining the start
> address.

Fortunatly a previous patch addressed that problem.  The convert 
function is only applied to positively identified descriptors.

>> +  return gdbarch_convert_from_func_ptr_addr (current_gdbarch,
>> +					     bfd_get_start_address (abfd),
>> +					     targ);
>> +}

Andrew

Re: [rfa:solib] Handle start-address descriptors

[Kevin Buettner]

On Oct 27,  6:49pm, Andrew Cagney wrote:

> >> +#include "bfd-target.h"
> >> +#include "exec.h"
> > 
> > I'm surprised that you needed to include exec.h.  solib-svr4.c already
> > includes target.h and I would've thought this to be sufficient.  If
> > exec.h isn't needed, please take it out.  (Don't forget to fix Makefile.in.)
> 
> They are both needed.

Okay, I've looked over your patch again and I see why now -- the
address of exec_ops is being passed to exec_entry_point().  (You could
have mentioned this instead of just saying "They are both needed.")

> > Could you add a comment here telling why
> > gdbarch_convert_from_func_ptr_addr() is needed.  Maybe something like
> > this?
> 
> I'll add your comment.

Thanks.

> > Hmm, a possible problem...  What happens when the target uses function
> > descriptors, but not for the exec file's start address?  I'm wondering
> > (ugh) if a separate gdbarch method is required for obtaining the start
> > address.
> 
> Fortunatly a previous patch addressed that problem.  The convert 
> function is only applied to positively identified descriptors.

If I'm not mistaken, this problem has been addressed for ppc64 and
ia64, but not in a generic fashion.  I'm concerned about some other
ABI (for some other architecture) which has function descriptors, but
has no easy way to discriminate between descriptors and code
addresses.  If this ABI specifies that the entry point should refer to
a code address instead of a descriptor, then we need a more
complicated mechanism -- perhaps that separate gdbarch method that I
mentioned earlier.

I suppose we can wait to do something about this problem until it
actually arises.  But... since we've identified a potential problem,
we should at least document it in some appropriate location.  If
the convert_from_func_ptr_addr() method *must* (due to the way it
is used elsewhere) contain a discrimination mechanism, then this
should be mentioned in the documentation.  (BTW, I don't see this
method documented in gdbint.texinfo.)

Kevin

Re: [rfa:solib] Handle start-address descriptors

[Andrew Cagney]

>> > Hmm, a possible problem...  What happens when the target uses function
>> > descriptors, but not for the exec file's start address?  I'm wondering
>> > (ugh) if a separate gdbarch method is required for obtaining the start
>> > address.
> 
>> 
>> Fortunatly a previous patch addressed that problem.  The convert 
>> function is only applied to positively identified descriptors.
> 
> 
> If I'm not mistaken, this problem has been addressed for ppc64 and
> ia64, but not in a generic fashion.  I'm concerned about some other
> ABI (for some other architecture) which has function descriptors, but
> has no easy way to discriminate between descriptors and code
> addresses.  If this ABI specifies that the entry point should refer to
> a code address instead of a descriptor, then we need a more
> complicated mechanism -- perhaps that separate gdbarch method that I
> mentioned earlier.

> I suppose we can wait to do something about this problem until it
> actually arises.  But... since we've identified a potential problem,
> we should at least document it in some appropriate location.  If
> the convert_from_func_ptr_addr() method *must* (due to the way it
> is used elsewhere) contain a discrimination mechanism, then this
> should be mentioned in the documentation.  (BTW, I don't see this
> method documented in gdbint.texinfo.)

Can I encourage you to write this up and add it to the doco?  The method 
most affects the ia64 and ppc/rs6000 targets that you maintain.  There 
is no value-add in taking it any further though.

I'll commit the solib-svr4.c change stuff once I've drained a few other 
pending patches.

Andrew

Re: [rfa:solib] Handle start-address descriptors

[Andrew Cagney]

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

> Hello,
> 
> The attached patch modifies the solib's computation of the exec's entry-point address so that it applies convert_from_func_ptr_addr() to the bfd start-address.  That way, when the bfd's start-address points at a descriptor, the true exec entry point is still obtained.
> 
> Fixes ppc64's ability to debug shared libraries.

Attached is the patch as committed.  PPC64 GNU/Linux is now "useable".

Andrew

> PS: Ref
> http://sources.redhat.com/ml/gdb-patches/2003-10/msg00670.html
> 
> 


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

2003-10-31  Andrew Cagney  <cagney@redhat.com>

	* solib-svr4.c: Update copyright.  Include "bfd-target.h" and
	"exec.h".
	(exec_entry_point): New function.
	(enable_break): Create a "tmp_bfd_target", use that and
	entry_point_address when computing the relocation offset.
	(svr4_relocate_main_executable): Ditto with exec_bfd and exec_ops.
	* Makefile.in (solib-svr4.o): Update dependencies.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.465
diff -u -r1.465 Makefile.in
--- Makefile.in	31 Oct 2003 19:19:51 -0000	1.465
+++ Makefile.in	31 Oct 2003 21:09:54 -0000
@@ -2298,7 +2298,8 @@
 	$(bcache_h) $(regcache_h)
 solib-svr4.o: solib-svr4.c $(defs_h) $(elf_external_h) $(elf_common_h) \
 	$(elf_mips_h) $(symtab_h) $(bfd_h) $(symfile_h) $(objfiles_h) \
-	$(gdbcore_h) $(target_h) $(inferior_h) $(solist_h) $(solib_svr4_h)
+	$(gdbcore_h) $(target_h) $(inferior_h) $(solist_h) $(solib_svr4_h) \
+	$(bfd_target_h) $(exec_h)
 sol-thread.o: sol-thread.c $(defs_h) $(gdbthread_h) $(target_h) \
 	$(inferior_h) $(gdb_stat_h) $(gdbcmd_h) $(gdbcore_h) $(regcache_h) \
 	$(symfile_h) $(gdb_string_h) $(gregset_h)
Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.37
diff -u -r1.37 solib-svr4.c
--- solib-svr4.c	26 Aug 2003 23:35:19 -0000	1.37
+++ solib-svr4.c	31 Oct 2003 21:09:54 -0000
@@ -1,7 +1,7 @@
 /* Handle SVR4 shared libraries for GDB, the GNU Debugger.
-   Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1998, 1999, 2000,
-   2001
-   Free Software Foundation, Inc.
+
+   Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1998, 1999,
+   2000, 2001, 2003 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -37,6 +37,9 @@
 #include "solist.h"
 #include "solib-svr4.h"
 
+#include "bfd-target.h"
+#include "exec.h"
+
 #ifndef SVR4_FETCH_LINK_MAP_OFFSETS
 #define SVR4_FETCH_LINK_MAP_OFFSETS() svr4_fetch_link_map_offsets ()
 #endif
@@ -911,6 +914,24 @@
 	  || in_plt_section (pc, NULL));
 }
 
+/* Given an executable's ABFD and target, compute the entry-point
+   address.  */
+
+static CORE_ADDR
+exec_entry_point (struct bfd *abfd, struct target_ops *targ)
+{
+  /* KevinB wrote ... for most targets, the address returned by
+     bfd_get_start_address() is the entry point for the start
+     function.  But, for some targets, bfd_get_start_address() returns
+     the address of a function descriptor from which the entry point
+     address may be extracted.  This address is extracted by
+     gdbarch_convert_from_func_ptr_addr().  The method
+     gdbarch_convert_from_func_ptr_addr() is the merely the identify
+     function for targets which don't use function descriptors.  */
+  return gdbarch_convert_from_func_ptr_addr (current_gdbarch,
+					     bfd_get_start_address (abfd),
+					     targ);
+}
 
 /*
 
@@ -984,6 +1005,7 @@
       int load_addr_found = 0;
       struct so_list *inferior_sos;
       bfd *tmp_bfd = NULL;
+      struct target_ops *tmp_bfd_target;
       int tmp_fd = -1;
       char *tmp_pathname = NULL;
       CORE_ADDR sym_addr = 0;
@@ -1019,6 +1041,11 @@
 	  goto bkpt_at_symbol;
 	}
 
+      /* Now convert the TMP_BFD into a target.  That way target, as
+         well as BFD operations can be used.  Note that closing the
+         target will also close the underlying bfd.  */
+      tmp_bfd_target = target_bfd_reopen (tmp_bfd);
+
       /* If the entry in _DYNAMIC for the dynamic linker has already
          been filled in, we can read its base address from there. */
       inferior_sos = svr4_current_sos ();
@@ -1042,7 +1069,8 @@
 	 the current pc (which should point at the entry point for the
 	 dynamic linker) and subtracting the offset of the entry point.  */
       if (!load_addr_found)
-	load_addr = read_pc () - tmp_bfd->start_address;
+	load_addr = (read_pc ()
+		     - exec_entry_point (tmp_bfd, tmp_bfd_target));
 
       /* Record the relocated start and end address of the dynamic linker
          text and plt section for svr4_in_dynsym_resolve_code.  */
@@ -1080,8 +1108,9 @@
 	    break;
 	}
 
-      /* We're done with the temporary bfd.  */
-      bfd_close (tmp_bfd);
+      /* We're done with both the temporary bfd and target.  Remember,
+         closing the target closes the underlying bfd.  */
+      target_close (tmp_bfd_target, 0);
 
       if (sym_addr != 0)
 	{
@@ -1200,7 +1229,7 @@
   interp_sect = bfd_get_section_by_name (exec_bfd, ".interp");
   if (interp_sect == NULL 
       && (bfd_get_file_flags (exec_bfd) & DYNAMIC) != 0
-      && bfd_get_start_address (exec_bfd) != pc)
+      && (exec_entry_point (exec_bfd, &exec_ops) != pc))
     {
       struct cleanup *old_chain;
       struct section_offsets *new_offsets;
@@ -1232,7 +1261,7 @@
 	 The same language also appears in Edition 4.0 of the System V
 	 ABI and is left unspecified in some of the earlier editions.  */
 
-      displacement = pc - bfd_get_start_address (exec_bfd);
+      displacement = pc - exec_entry_point (exec_bfd, &exec_ops);
       changed = 0;
 
       new_offsets = xcalloc (symfile_objfile->num_sections,