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

[Daniel Jacobowitz <drow at false dot org>]

On Mon, Jun 25, 2007 at 08:17:43PM +0200, Ulrich Weigand wrote:
> > Index: gdbserver/remote-utils.c
> > ===================================================================
> > --- gdbserver/remote-utils.c	(revision 174465)
> > +++ gdbserver/remote-utils.c	(working copy)
> > @@ -1063,15 +1063,33 @@ look_up_one_symbol (const char *name, CO
> 
> > +      decode_m_packet (&own_buf[1], &mem_addr, &len);
> 
> This results in a warning as the last parameter to decode_m_packet
> is supposed to be "unsigned int *", but "len" is signed int.

Whoops, and the reuse of len is confusing anyway.  I have committed
the version attached.

-- 
Daniel Jacobowitz
CodeSourcery

2007-07-03  Paul Gilliam  <pgilliam@us.ibm.com>
	    Thiago Bauermann  <bauerman@br.ibm.com>
	    Joseph S. Myers  <joseph@codesourcery.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	gdb/
	* remote.c (remote_check_symbols): Use
	gdbarch_convert_from_func_ptr_addr.
	* infcall.c (find_function_addr): Handle function descriptors
	without debugging information.
	* ppc-linux-tdep.c (ppc_linux_convert_from_func_ptr_addr): Renamed
	from ppc64_linux_convert_from_func_ptr_addr.  Handle -msecure-plt.
	(ppc_linux_init_abi): Always set convert_from_func_ptr_addr.
	* solib-svr4.c (solib_break_names): Remove "._dl_debug_state".
	(bfd_lookup_symbol): Do not take a SECT_FLAGS argument.  Always
	allow SEC_CODE and SEC_DATA.
	(enable_break): Update calls.  Pass current_target to solib_add.
	Use gdbarch_convert_from_func_ptr_addr.

	gdb/gdbserver/
	* remote-utils.c (look_up_one_symbol): Handle 'm' packets.

Index: gdb/infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.85
diff -u -p -r1.85 infcall.c
--- gdb/infcall.c	18 Jun 2007 18:23:08 -0000	1.85
+++ gdb/infcall.c	3 Jul 2007 15:56:37 -0000
@@ -222,8 +222,24 @@ find_function_addr (struct value *functi
       if (TYPE_LENGTH (ftype) == 1)
 	funaddr = value_as_address (value_addr (function));
       else
-	/* Handle integer used as address of a function.  */
-	funaddr = (CORE_ADDR) value_as_long (function);
+	{
+	  /* Handle function descriptors lacking debug info.  */
+	  int found_descriptor = 0;
+	  if (VALUE_LVAL (function) == lval_memory)
+	    {
+	      CORE_ADDR nfunaddr;
+	      funaddr = value_as_address (value_addr (function));
+	      nfunaddr = funaddr;
+	      funaddr = gdbarch_convert_from_func_ptr_addr (current_gdbarch,
+							    funaddr,
+							    &current_target);
+	      if (funaddr != nfunaddr)
+		found_descriptor = 1;
+	    }
+	  if (!found_descriptor)
+	    /* Handle integer used as address of a function.  */
+	    funaddr = (CORE_ADDR) value_as_long (function);
+	}
 
       value_type = builtin_type_int;
     }
Index: gdb/ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.85
diff -u -p -r1.85 ppc-linux-tdep.c
--- gdb/ppc-linux-tdep.c	18 Jun 2007 17:45:26 -0000	1.85
+++ gdb/ppc-linux-tdep.c	3 Jul 2007 15:56:38 -0000
@@ -591,39 +591,73 @@ ppc64_skip_trampoline_code (struct frame
 }
 
 
-/* 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 may be 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 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
    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
    the inferior (which are called via function pointers),
    find_function_addr uses this function to get the function address
-   from a function pointer.  */
+   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
-   random addresses such as occures when there is no symbol table.  */
+   If ADDR points at what is clearly a function descriptor, transform
+   it into the address of the corresponding function, if needed.  Be
+   conservative, otherwise GDB will do the transformation on any
+   random addresses such as occur 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 consistency 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;
 }
@@ -907,6 +941,11 @@ ppc_linux_init_abi (struct gdbarch_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 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);
+
   if (tdep->wordsize == 4)
     {
       /* Until November 2001, gcc did not comply with the 32 bit SysV
@@ -934,13 +973,8 @@ ppc_linux_init_abi (struct gdbarch_info 
   
   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);
 
Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.264
diff -u -p -r1.264 remote.c
--- gdb/remote.c	2 Jul 2007 22:01:09 -0000	1.264
+++ gdb/remote.c	3 Jul 2007 15:56:38 -0000
@@ -2259,9 +2259,19 @@ remote_check_symbols (struct objfile *ob
       if (sym == NULL)
 	xsnprintf (msg, get_remote_packet_size (), "qSymbol::%s", &reply[8]);
       else
-	xsnprintf (msg, get_remote_packet_size (), "qSymbol:%s:%s",
-		   paddr_nz (SYMBOL_VALUE_ADDRESS (sym)),
-		   &reply[8]);
+	{
+	  CORE_ADDR sym_addr = SYMBOL_VALUE_ADDRESS (sym);
+
+	  /* If this is a function address, return the start of code
+	     instead of any data function descriptor.  */
+	  sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch,
+							 sym_addr,
+							 &current_target);
+
+	  xsnprintf (msg, get_remote_packet_size (), "qSymbol:%s:%s",
+		     paddr_nz (sym_addr), &reply[8]);
+	}
+  
       putpkt (msg);
       getpkt (&rs->buf, &rs->buf_size, 0);
       reply = rs->buf;
Index: gdb/solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.68
diff -u -p -r1.68 solib-svr4.c
--- gdb/solib-svr4.c	3 Jul 2007 12:14:43 -0000	1.68
+++ gdb/solib-svr4.c	3 Jul 2007 15:56:38 -0000
@@ -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
 };
 
@@ -263,7 +253,7 @@ static char *debug_loader_name;
 
 static int match_main (char *);
 
-static CORE_ADDR bfd_lookup_symbol (bfd *, char *, flagword);
+static CORE_ADDR bfd_lookup_symbol (bfd *, char *);
 
 /*
 
@@ -273,24 +263,25 @@ static CORE_ADDR bfd_lookup_symbol (bfd 
 
    SYNOPSIS
 
-   CORE_ADDR bfd_lookup_symbol (bfd *abfd, char *symname, flagword sect_flags)
+   CORE_ADDR bfd_lookup_symbol (bfd *abfd, char *symname)
 
    DESCRIPTION
 
    An expensive way to lookup the value of a single symbol for
    bfd's that are only temporary anyway.  This is used by the
    shared library support to find the address of the debugger
-   interface structures in the shared library.
+   notification routine in the shared library.
 
-   If SECT_FLAGS is non-zero, only match symbols in sections whose
-   flags include all those in SECT_FLAGS.
+   The returned symbol may be in a code or data section; functions
+   will normally be in a code section, but may be in a data section
+   if this architecture uses function descriptors.
 
    Note that 0 is specifically allowed as an error return (no
    such symbol).
  */
 
 static CORE_ADDR
-bfd_lookup_symbol (bfd *abfd, char *symname, flagword sect_flags)
+bfd_lookup_symbol (bfd *abfd, char *symname)
 {
   long storage_needed;
   asymbol *sym;
@@ -312,9 +303,9 @@ bfd_lookup_symbol (bfd *abfd, char *symn
 	{
 	  sym = *symbol_table++;
 	  if (strcmp (sym->name, symname) == 0
-              && (sym->section->flags & sect_flags) == sect_flags)
+              && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0)
 	    {
-	      /* Bfd symbols are section relative. */
+	      /* BFD symbols are section relative.  */
 	      symaddr = sym->value + sym->section->vma;
 	      break;
 	    }
@@ -341,9 +332,9 @@ bfd_lookup_symbol (bfd *abfd, char *symn
 	  sym = *symbol_table++;
 
 	  if (strcmp (sym->name, symname) == 0
-              && (sym->section->flags & sect_flags) == sect_flags)
+              && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0)
 	    {
-	      /* Bfd symbols are section relative. */
+	      /* BFD symbols are section relative.  */
 	      symaddr = sym->value + sym->section->vma;
 	      break;
 	    }
@@ -1033,7 +1024,7 @@ enable_break (void)
 
       /* On a running target, we can get the dynamic linker's base
          address from the shared library table.  */
-      solib_add (NULL, 0, NULL, auto_solib_add);
+      solib_add (NULL, 0, &current_target, auto_solib_add);
       so = master_so_list ();
       while (so)
 	{
@@ -1056,7 +1047,7 @@ enable_break (void)
 	  debug_loader_name = xstrdup (buf);
 	  debug_loader_offset_p = 1;
 	  debug_loader_offset = load_addr;
-	  solib_add (NULL, 0, NULL, auto_solib_add);
+	  solib_add (NULL, 0, &current_target, auto_solib_add);
 	}
 
       /* Record the relocated start and end address of the dynamic linker
@@ -1081,20 +1072,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);
+	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep);
 	  if (sym_addr != 0)
 	    break;
 	}
 
+      if (sym_addr != 0)
+	/* Convert 'sym_addr' from a function pointer to an address.
+	   Because we pass tmp_bfd_target instead of the current
+	   target, this will always produce an unrelocated value.  */
+	sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch,
+						       sym_addr,
+						       tmp_bfd_target);
+
       /* We're done with both the temporary bfd and target.  Remember,
          closing the target closes the underlying bfd.  */
       target_close (tmp_bfd_target, 0);
Index: gdb/gdbserver/remote-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/remote-utils.c,v
retrieving revision 1.45
diff -u -p -r1.45 remote-utils.c
--- gdb/gdbserver/remote-utils.c	12 Jun 2007 14:38:32 -0000	1.45
+++ gdb/gdbserver/remote-utils.c	3 Jul 2007 15:56:38 -0000
@@ -1109,15 +1109,34 @@ look_up_one_symbol (const char *name, CO
   if (len < 0)
     return -1;
 
+  /* We ought to handle pretty much any packet at this point while we
+     wait for the qSymbol "response".  That requires re-entering the
+     main loop.  For now, this is an adequate approximation; allow
+     GDB to read from memory while it figures out the address of the
+     symbol.  */
+  while (own_buf[0] == 'm')
+    {
+      CORE_ADDR mem_addr;
+      unsigned char *mem_buf;
+      unsigned int mem_len;
+
+      decode_m_packet (&own_buf[1], &mem_addr, &mem_len);
+      mem_buf = malloc (mem_len);
+      if (read_inferior_memory (mem_addr, mem_buf, mem_len) == 0)
+	convert_int_to_ascii (mem_buf, own_buf, mem_len);
+      else
+	write_enn (own_buf);
+      free (mem_buf);
+      if (putpkt (own_buf) < 0)
+	return -1;
+      len = getpkt (own_buf);
+      if (len < 0)
+	return -1;
+    }
+  
   if (strncmp (own_buf, "qSymbol:", strlen ("qSymbol:")) != 0)
     {
-      /* Malformed response.  */
-      if (remote_debug)
-	{
-	  fprintf (stderr, "Malformed response to qSymbol, ignoring.\n");
-	  fflush (stderr);
-	}
-
+      warning ("Malformed response to qSymbol, ignoring: %s\n", own_buf);
       return -1;
     }