Re: [patch] Fixes problem setting breakpoint in dynamic loader

[PAUL GILLIAM <pgilliam at us dot ibm dot com>]

I have removed the previous attempts from the bug tracker and am
starting over with this one.

:ADDPATCH PowerPC-64:


The problem:

On PowerPC-64, with 64-bit executables, GDB has been giving this message
for a while:

        warning: Unable to find dynamic linker breakpoint function.
        GDB will be unable to debug shared library initializers
        and track explicitly loaded dynamic code.

This is because "enable_break()" in solib-svr4.c was looking for the
symbol "._dl_debug_state" in the 64-bit dynamic loader and not finding
it.  This should not be a surprise because these 'dot' symbols have not
been generated by the linker for a while.

The reason that the non-'dot' symbol was also passed by is that it
points into a data section and the existing code only checked code
sections.

The fix:

I changed the local routine bfd_lookup_symbol() and the call to
it so that the symbol is looked for in both code and data sections.

If we find a symbol in a code section, we're done: we have an
address where we can set the breakpoint.

If we find a symbol in a data section, then we must be on a system
that uses function descriptors.  Again we're done: we just use the
address of the function descriptor and depend on adjust_breakpoint()
to do the right thing.

Doing things this way means we can forget that there ever where 'dot'
symbols (at least in this case).

Of course for this to work, adjust_breakpoint must do the right thing.
I believe it does for architectures that use function descriptors, except
for ppc-32.  The reason ppc-32 was being ignored was that its function
descriptors where executable and we could pretend that the function address
pointed to it's entry point.  But now, with the new "secure PLT's" feature,
ppc-32 function descriptors can be in a data section.  With this new type of
ppc-32 function descriptor, adjust_breakpoint must get involved and
know how to "dereference" them.

So I changed ppc64_linux_convert_from_func_ptr_addr() to just 
ppc_linux_convert_from_func_ptr_addr() and taught it how to
deal with ppc-32 function descriptors, using the word size to
know which it's dealing with: ppc-32 or ppc-64.  I also needed
to change ppc_linux_init_abi so that the newly updated
ppc_linux_convert_from_func_ptr_addr() would be plugged into
"gdbarch_convert_from_func_ptr_addr" for both ppc-64 and ppc-32.

I have attached the patch:  OK to commit?

-=# Paul #=-

PS: In a previous reincarnation of this patch, rs6000-tdep.c was modified,
but that is not needed with this version.

2006-07-19 Paul Gilliam  <pgilliam@us.ibm.com>
	* ppc-linux-tdep.c (ppc64_linux_convert_from_func_ptr_addr):
	Change name and update to work for both ppc-64 and ppc-32.
	(ppc_linux_init_abi): Arange for "gdbarch_convert_from_func_ptr_addr"
	to be set for both ppc-64 and ppc-32.
	* solib-svr4.c (solib_break_names): Remove "._dl_debug_state" and
	the comment explaining it.
	(bfd_lookup_symbol): Change the meaning of the parameter "sect_flags"
	so that any bit can be on instead of all bits.
	(enable_break): Change the call to bfd_lookup_symbol() to search
	both code and data sections and change the comment to explain why,
	getting rid of any mention of 'dot' symbols.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.78
diff -a -u -r1.78 ppc-linux-tdep.c
--- ppc-linux-tdep.c	18 Apr 2006 19:20:06 -0000	1.78
+++ ppc-linux-tdep.c	19 Jul 2006 21:10:41 -0000
@@ -725,17 +725,25 @@
 }
 
 
-/* Support for CONVERT_FROM_FUNC_PTR_ADDR (ARCH, ADDR, TARG) on PPC64
+/* Support for CONVERT_FROM_FUNC_PTR_ADDR (ARCH, ADDR, TARG) on PPC
    GNU/Linux.
 
    Usually a function pointer's representation is simply the address
-   of the function. On GNU/Linux on the 64-bit PowerPC however, a
-   function pointer is represented by a pointer to a TOC entry. This
-   TOC entry contains three words, the first word is the address of
-   the function, the second word is the TOC pointer (r2), and the
-   third word is the static chain value.  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
+   of the function.  On GNU/Linux on the PowerPC however, a function
+   pointer is, in fact, a pointer to a function descriptor.
+
+   For PPC64, a function descriptor is a TOC entry, in a data section,
+   which contains three words: the first word is the address of the
+   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 descriptors: non-secure
+   and secure.  Non-secure function descriptors are in a code section,
+   are executable and thus need no translation.  Secure ones 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
    require allocation of a TOC entry in the inferior's memory space,
    with all its drawbacks.  To be able to call C++ virtual methods in
@@ -744,20 +752,44 @@
    from a function pointer.  */
 
 /* If ADDR points at what is clearly a function descriptor, transform
-   it into the address of the corresponding function.  Be
+   it into the address of the corresponding function, if needed.  Be
    conservative, otherwize GDB will do the transformation on any
    random addresses such as occures when there is no symbol table.  */
 
 static CORE_ADDR
-ppc64_linux_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
-					CORE_ADDR addr,
-					struct target_ops *targ)
+ppc_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 = 0;
+
+  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 consitency check"));
+    }
 
   /* Check if ADDR points to a function descriptor.  */
-  if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
-    return get_target_memory_unsigned (targ, addr, 8);
+
+  /* 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);
 
   return addr;
 }
@@ -1034,6 +1066,11 @@
   /* NOTE: cagney/2005-01-25: True for both 32- and 64-bit.  */
   set_gdbarch_long_double_bit (gdbarch, 8 * TARGET_CHAR_BIT);
 
+  /* Handle PPC GNU/Linux function pointers (which are really function
+     descriptors).  */
+  set_gdbarch_convert_from_func_ptr_addr
+        (gdbarch, ppc_linux_convert_from_func_ptr_addr);
+    
   if (tdep->wordsize == 4)
     {
       /* Until November 2001, gcc did not comply with the 32 bit SysV
@@ -1055,27 +1092,27 @@
         (gdbarch, svr4_ilp32_fetch_link_map_offsets);
 
       /* Trampolines.  */
-      tramp_frame_prepend_unwinder (gdbarch, &ppc32_linux_sigaction_tramp_frame);
-      tramp_frame_prepend_unwinder (gdbarch, &ppc32_linux_sighandler_tramp_frame);
+      tramp_frame_prepend_unwinder (gdbarch,
+				    &ppc32_linux_sigaction_tramp_frame);
+      tramp_frame_prepend_unwinder (gdbarch,
+				    &ppc32_linux_sighandler_tramp_frame);
     }
   
   if (tdep->wordsize == 8)
     {
-      /* Handle PPC64 GNU/Linux function pointers (which are really
-         function descriptors).  */
-      set_gdbarch_convert_from_func_ptr_addr
-        (gdbarch, ppc64_linux_convert_from_func_ptr_addr);
-      set_gdbarch_skip_trampoline_code (gdbarch, ppc64_skip_trampoline_code);
-
       /* Shared library handling.  */
+      set_gdbarch_skip_trampoline_code (gdbarch, ppc64_skip_trampoline_code);
       set_solib_svr4_fetch_link_map_offsets
         (gdbarch, svr4_lp64_fetch_link_map_offsets);
 
       /* Trampolines.  */
-      tramp_frame_prepend_unwinder (gdbarch, &ppc64_linux_sigaction_tramp_frame);
-      tramp_frame_prepend_unwinder (gdbarch, &ppc64_linux_sighandler_tramp_frame);
+      tramp_frame_prepend_unwinder (gdbarch,
+				    &ppc64_linux_sigaction_tramp_frame);
+      tramp_frame_prepend_unwinder (gdbarch,
+				    &ppc64_linux_sighandler_tramp_frame);
     }
-  set_gdbarch_regset_from_core_section (gdbarch, ppc_linux_regset_from_core_section);
+  set_gdbarch_regset_from_core_section (gdbarch,
+					ppc_linux_regset_from_core_section);
 
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.58
diff -a -u -r1.58 solib-svr4.c
--- solib-svr4.c	18 May 2006 20:38:56 -0000	1.58
+++ solib-svr4.c	19 Jul 2006 21:10:41 -0000
@@ -85,16 +85,6 @@
   "rtld_db_dlactivity",
   "_rtld_debug_state",
 
-  /* On the 64-bit PowerPC, the linker symbol with the same name as
-     the C function points to a function descriptor, not to the entry
-     point.  The linker symbol whose name is the C function name
-     prefixed with a '.' points to the function's entry point.  So
-     when we look through this table, we ignore symbols that point
-     into the data section (thus skipping the descriptor's symbol),
-     and eventually try this one, giving us the real entry point
-     address.  */
-  "._dl_debug_state",
-
   NULL
 };
 
@@ -287,7 +277,7 @@
    interface structures in the shared library.
 
    If SECT_FLAGS is non-zero, only match symbols in sections whose
-   flags include all those in SECT_FLAGS.
+   flags include any of those in SECT_FLAGS.
 
    Note that 0 is specifically allowed as an error return (no
    such symbol).
@@ -316,7 +306,7 @@
 	{
 	  sym = *symbol_table++;
 	  if (strcmp (sym->name, symname) == 0
-              && (sym->section->flags & sect_flags) == sect_flags)
+              && (sym->section->flags & sect_flags))
 	    {
 	      /* Bfd symbols are section relative. */
 	      symaddr = sym->value + sym->section->vma;
@@ -345,7 +335,7 @@
 	  sym = *symbol_table++;
 
 	  if (strcmp (sym->name, symname) == 0
-              && (sym->section->flags & sect_flags) == sect_flags)
+              && (sym->section->flags & sect_flags))
 	    {
 	      /* Bfd symbols are section relative. */
 	      symaddr = sym->value + sym->section->vma;
@@ -1043,16 +1033,19 @@
       /* Now try to set a breakpoint in the dynamic linker.  */
       for (bkpt_namep = solib_break_names; *bkpt_namep != NULL; bkpt_namep++)
 	{
-          /* On ABI's that use function descriptors, there are usually
-             two linker symbols associated with each C function: one
-             pointing at the actual entry point of the machine code,
-             and one pointing at the function's descriptor.  The
-             latter symbol has the same name as the C function.
-
-             What we're looking for here is the machine code entry
-             point, so we are only interested in symbols in code
-             sections.  */
-	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
+          /* On ABI's that use function descriptors, the linker symbol
+	     with the same name as a C function points to a function
+	     descriptor that can be in either a code or data section. 
+             In either case, pointers to a function desciptors are
+	     converted to "breakpointable" addresses in set_raw_breakpoint()
+	     using gdbarch_adjust_breakpoint_address(), if needed.
+
+	     On ABI's that don't use function descriptors, there is no
+	     need for set_raw_breakpoint() to adjust the address.  Also,
+	     we assume that none of the symbols in the solib_break_names
+	     table are in a data section.  */
+	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep,
+					SEC_CODE | SEC_DATA);
 	  if (sym_addr != 0)
 	    break;
 	}