[PATCH] Fixes problem setting breakpoint in dynamic loader

[Thiago Jung Bauermann <bauerman at br dot ibm dot com>]

Hi folks,

This is a re-submission of:

http://sourceware.org/ml/gdb-patches/2006-07/msg00264.html

The patch above resolves all review comments which were made on previous
versions (I collected them in the attached .txt, if you wish to have a
look). The problem is that it is malformed (maybe it was hand edited, I
don't know) and doesn't fix a couple of typos in comments.

The attached patch fixes the typos and applies cleanly on current CVS
HEAD.

Regarding this comment from Kevin Buettner (found in
http://sources.redhat.com/ml/gdb-patches/2006-06/msg00382.html):

> > +	     When those function descriptors are in the code section, they
> > +	     contain executable code and we can set a breakpoint there. >
*/
> 
> Also, I don't mind that the comment was rearranged, but I would like
> to see information regarding the two linker symbols retained in some
> fashion.

The information regarding the two linker symbols is outdated. The "dot"
symbols pointing to the function entrypoint are no longer generated by
the toolchain (this is true at least for ppc, I don't know about other
architectures). Tools are expected to automatically generate those
symbols when needed.

Is this ok for inclusion?
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Attachment: comments.txt
Description: Text document

2007-04-27  Paul Gilliam  <pgilliam@us.ibm.com>
	    Thiago Bauermann  <bauerman@br.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): Arrange 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.

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index cf09a17..8c4bcb9 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -725,17 +725,25 @@ ppc64_skip_trampoline_code (CORE_ADDR pc)
 }
 
 
-/* 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,45 @@ ppc64_skip_trampoline_code (CORE_ADDR pc)
    from a function pointer.  */
 
 /* If ADDR points at what is clearly a function descriptor, transform
-   it into the address of the corresponding function.  Be
-   conservative, otherwize GDB will do the transformation on any
+   it into the address of the corresponding function, if needed.  Be
+   conservative, otherwise 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 = 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 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;
 }
@@ -1038,6 +1071,11 @@ ppc_linux_init_abi (struct gdbarch_info info,
   /* 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
@@ -1059,27 +1097,27 @@ ppc_linux_init_abi (struct gdbarch_info info,
         (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,
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 105ff33..678f90e 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -84,16 +84,6 @@ static char *solib_break_names[] =
   "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
 };
 
@@ -283,7 +273,7 @@ static CORE_ADDR bfd_lookup_symbol (bfd *, char *, flagword);
    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).
@@ -312,7 +302,7 @@ bfd_lookup_symbol (bfd *abfd, char *symname, flagword sect_flags)
 	{
 	  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;
@@ -341,7 +331,7 @@ bfd_lookup_symbol (bfd *abfd, char *symname, flagword sect_flags)
 	  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;
@@ -1094,16 +1084,19 @@ enable_break (void)
       /* 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 ABIs 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 descriptor are
+	     converted to "breakpointable" addresses in set_raw_breakpoint()
+	     using gdbarch_adjust_breakpoint_address(), if needed.
+
+	     On ABIs 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;
 	}