Re: [rfc] Remove "function descriptor" handling on ppc32

Re: [rfc] Remove "function descriptor" handling on ppc32

[Daniel Jacobowitz]

On Wed, May 14, 2008 at 07:31:12PM +0200, Ulrich Weigand wrote:
> Dan,
> 
> a while back you installed a patch to make ...convert_from_func_ptr_addr
> handle "secure PLTs" on ppc32; this was apparently based on earlier 
> patches by Thiago and Paul:
> http://sourceware.org/ml/gdb-patches/2007-07/msg00068.html

The history of that patch is, as you can see, long and tortuous.  I
think I just inherited that part; I can't remember its history.

> The only reason I could imagine where this convert_from_func_ptr_addr 
> version would ever apply is that the "...@plt" synthetic symbols used
> to point to those PLT data entries.  After the patch Alan just committed,
> this is no longer the case, however --  @plt symbols now point to the
> plt call stubs as well.

This sounds like a plausible explanation to me.

-- 
Daniel Jacobowitz
CodeSourcery

[rfc] Remove "function descriptor" handling on ppc32

[Ulrich Weigand]

Dan,

a while back you installed a patch to make ...convert_from_func_ptr_addr
handle "secure PLTs" on ppc32; this was apparently based on earlier 
patches by Thiago and Paul:
http://sourceware.org/ml/gdb-patches/2007-07/msg00068.html

I'm not sure why this is required.  The secure PLT change did not change
the format of function pointers on ppc32, they still always are simple
code addresses, not pointers to function descriptors.  While it is true
that the PLT now holds a data element, not code, there should never be
a function pointer that points to that PLT -- function pointers would
now point to the PLT stub.

The only reason I could imagine where this convert_from_func_ptr_addr 
version would ever apply is that the "...@plt" synthetic symbols used
to point to those PLT data entries.  After the patch Alan just committed,
this is no longer the case, however --  @plt symbols now point to the
plt call stubs as well.

Thus, this all appears to be dead code, and I'd prefer to remove it.
I've tested the patch below with no change in behaviour on powerpc-linux
and powerpc64-linux.

Am I overlooking anything here?

Bye,
Ulrich


ChangeLog:

	* ppc-linux-tdep.c (ppc_linux_convert_from_func_ptr_addr): Rename ...
	(ppc64_linux_convert_from_func_ptr_addr): ... to this.  No longer try
	to handle ppc32 PLT entries.
	(ppc_linux_init_abi): Install ppc64_linux_convert_from_func_ptr_addr
	only on ppc64.


diff -urNp gdb-orig/gdb/ppc-linux-tdep.c gdb-head/gdb/ppc-linux-tdep.c
--- gdb-orig/gdb/ppc-linux-tdep.c	2008-05-14 19:21:17.501263514 +0200
+++ gdb-head/gdb/ppc-linux-tdep.c	2008-05-14 18:25:49.957428608 +0200
@@ -556,7 +556,7 @@ ppc64_skip_trampoline_code (struct frame
 }
 
 
-/* Support for convert_from_func_ptr_addr (ARCH, ADDR, TARG) on PPC
+/* Support for convert_from_func_ptr_addr (ARCH, ADDR, TARG) on PPC64
    GNU/Linux.
 
    Usually a function pointer's representation is simply the address
@@ -568,12 +568,6 @@ ppc64_skip_trampoline_code (struct frame
    function, the second word is the TOC pointer (r2), and the third word
    is the static chain value.
 
-   For PPC32, there are two kinds of function pointers: non-secure and
-   secure.  Non-secure function pointers point directly to the
-   function in a code section and thus need no translation.  Secure
-   ones (from GCC's -msecure-plt option) are in a data section and
-   contain one word: the address of the function.
-
    Throughout GDB it is currently assumed that a function pointer contains
    the address of the function, which is not easy to fix.  In addition, the
    conversion of a function address to a function pointer would
@@ -589,40 +583,15 @@ ppc64_skip_trampoline_code (struct frame
    random addresses such as occur when there is no symbol table.  */
 
 static CORE_ADDR
-ppc_linux_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
-				      CORE_ADDR addr,
-				      struct target_ops *targ)
+ppc64_linux_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
+					CORE_ADDR addr,
+					struct target_ops *targ)
 {
-  struct gdbarch_tdep *tdep;
   struct section_table *s = target_section_by_addr (targ, addr);
-  char *sect_name = NULL;
-
-  if (!s)
-    return addr;
-
-  tdep = gdbarch_tdep (gdbarch);
-
-  switch (tdep->wordsize)
-    {
-      case 4:
-	sect_name = ".plt";
-	break;
-      case 8:
-	sect_name = ".opd";
-	break;
-      default:
-	internal_error (__FILE__, __LINE__,
-			_("failed internal consistency check"));
-    }
 
   /* Check if ADDR points to a function descriptor.  */
-
-  /* NOTE: this depends on the coincidence that the address of a functions
-     entry point is contained in the first word of its function descriptor
-     for both PPC-64 and for PPC-32 with secure PLTs.  */
-  if ((strcmp (s->the_bfd_section->name, sect_name) == 0)
-      && s->the_bfd_section->flags & SEC_DATA)
-    return get_target_memory_unsigned (targ, addr, tdep->wordsize);
+  if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
+    return get_target_memory_unsigned (targ, addr, 8);
 
   return addr;
 }
@@ -1025,11 +994,6 @@ ppc_linux_init_abi (struct gdbarch_info 
   set_gdbarch_long_double_bit (gdbarch, 16 * TARGET_CHAR_BIT);
   set_gdbarch_long_double_format (gdbarch, floatformats_ibm_long_double);
 
-  /* Handle PPC GNU/Linux 64-bit function pointers (which are really
-     function descriptors) and 32-bit secure PLT entries.  */
-  set_gdbarch_convert_from_func_ptr_addr
-    (gdbarch, ppc_linux_convert_from_func_ptr_addr);
-
   /* Handle inferior calls during interrupted system calls.  */
   set_gdbarch_write_pc (gdbarch, ppc_linux_write_pc);
 
@@ -1069,6 +1033,11 @@ ppc_linux_init_abi (struct gdbarch_info 
       set_gdbarch_adjust_breakpoint_address
 	(gdbarch, ppc64_sysv_abi_adjust_breakpoint_address);
 
+      /* Handle PPC GNU/Linux 64-bit function pointers (which are really
+	 function descriptors).  */
+      set_gdbarch_convert_from_func_ptr_addr
+	(gdbarch, ppc64_linux_convert_from_func_ptr_addr);
+
       /* Shared library handling.  */
       set_gdbarch_skip_trampoline_code (gdbarch, ppc64_skip_trampoline_code);
       set_solib_svr4_fetch_link_map_offsets
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Remove "function descriptor" handling on ppc32

[Thiago Jung Bauermann]

On Wed, 2008-05-14 at 19:31 +0200, Ulrich Weigand wrote:
> a while back you installed a patch to make ...convert_from_func_ptr_addr
> handle "secure PLTs" on ppc32; this was apparently based on earlier 
> patches by Thiago and Paul:
> http://sourceware.org/ml/gdb-patches/2007-07/msg00068.html

I vaguely remember that... :-)

> The only reason I could imagine where this convert_from_func_ptr_addr 
> version would ever apply is that the "...@plt" synthetic symbols used
> to point to those PLT data entries.

I think that was was the problem. GDB wanted to set a breakpoint at
_dl_debug_state, but since it pointed to a data section (the plt entry)
it just flew by it.

>   After the patch Alan just committed,
> this is no longer the case, however --  @plt symbols now point to the
> plt call stubs as well.

These are synthetic symbols right, so no backward compatibility issues
there?
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [rfc] Remove "function descriptor" handling on ppc32

[Ulrich Weigand]

Thiago Jung Bauermann wrote:
> > The only reason I could imagine where this convert_from_func_ptr_addr 
> > version would ever apply is that the "...@plt" synthetic symbols used
> > to point to those PLT data entries.
> 
> I think that was was the problem. GDB wanted to set a breakpoint at
> _dl_debug_state, but since it pointed to a data section (the plt entry)
> it just flew by it.

Ah, I see.  That would explain it, and confirm that this code is indeed
no longer needed now.

> >   After the patch Alan just committed,
> > this is no longer the case, however --  @plt symbols now point to the
> > plt call stubs as well.
> 
> These are synthetic symbols right, so no backward compatibility issues
> there?

Correct.  We're now always linked against the new bfd, so we'll always
see the @plt symbols at their new location.

I'll go ahead and install the patch ...

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Remove "function descriptor" handling on ppc32

[Thiago Jung Bauermann]

On Wed, 2008-05-14 at 19:31 +0200, Ulrich Weigand wrote:
> Thus, this all appears to be dead code, and I'd prefer to remove it.
> I've tested the patch below with no change in behaviour on powerpc-linux
> and powerpc64-linux.
> 
> Am I overlooking anything here?

By the way, the patch also made bfd_lookup_symbol in solib-svr4.c look
for symbols in data sections, and added a call to
gdbarch_convert_from_func_ptr_addr. Is that dead code as well?
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [rfc] Remove "function descriptor" handling on ppc32

[Daniel Jacobowitz]

On Wed, May 14, 2008 at 04:01:26PM -0300, Thiago Jung Bauermann wrote:
> By the way, the patch also made bfd_lookup_symbol in solib-svr4.c look
> for symbols in data sections, and added a call to
> gdbarch_convert_from_func_ptr_addr. Is that dead code as well?

No, I don't think so.  It may be used on targets which really do use
descriptors.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Remove "function descriptor" handling on ppc32

[Ulrich Weigand]

Thiago Jung Bauermann wrote:
> On Wed, 2008-05-14 at 19:31 +0200, Ulrich Weigand wrote:
> > Thus, this all appears to be dead code, and I'd prefer to remove it.
> > I've tested the patch below with no change in behaviour on powerpc-linux
> > and powerpc64-linux.
> > 
> > Am I overlooking anything here?
> 
> By the way, the patch also made bfd_lookup_symbol in solib-svr4.c look
> for symbols in data sections, and added a call to
> gdbarch_convert_from_func_ptr_addr. Is that dead code as well?

No, this is still needed on powerpc64, where the symbols point to
the .opd section ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com