[patch] Fixes problem setting breakpoint in dynamic loader

[patch] Fixes problem setting breakpoint in dynamic loader

[PAUL GILLIAM]

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

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 used 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.

If none of the symbols on the list was found in a code section, the
attached patch looks in data sections.  When it finds one, and the data
section is either '.plt' or '.opd', then the address points to a
function descriptor which is then used to find the corresponding code
address where the breakpoint can be set.

OK to commit?

-=# Paul #=-

[-- Attachment #2: loader_break.diff --]
[-- Type: text/x-patch, Size: 4204 bytes --]

2006-05-24  Paul Gilliam  <pgilliam@us.ibm.com

	* solib-svr4.c (enable_break): Resolve break address when the
	symbol is found in the data section.

diff -Naur old/solib-svr4.c new/solib-svr4.c
--- old/solib-svr4.c	2006-05-24 15:50:42.000000000 -0700
+++ new/solib-svr4.c	2006-05-24 15:51:36.000000000 -0700
@@ -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
 };
 
@@ -1043,20 +1033,75 @@
       /* 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.  */
+	  /* What we're looking for here is the machine code entry point,
+	     so we are only interested in symbols in code sections.
+
+	     On ABI's that use function descriptors, the linker symbol with
+	     the same name as a C funtion points to that functions descriptor.
+	     when those function descriptors are in the code section, they
+	     contain executable code and we can set a breakpoint there. */
 	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
 	  if (sym_addr != 0)
 	    break;
 	}
 
+      if (sym_addr == 0)
+        {
+	  CORE_ADDR sect_offset;
+	  
+	  /* No symbol was found in a code section, so look in the data
+             sections.  This will only happen when the linker symbol points
+	     to a function descriptor that is in a data section. */
+	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
+	    {
+	      /* On ABI's that use function descriptors that are in the data
+	         section,
+	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
+	      if (sym_addr != 0)
+		break;
+	    }
+	  if (sym_addr == 0)
+	    {
+	      target_close (tmp_bfd_target, 0);
+	      goto bkpt_at_symbol;
+	    }
+
+	  /* On some ABI's, the function descriptor we need will be in the
+	     ".plt" section.  In others, it will be in the ".opd" section. */
+	  if (sym_addr + load_addr >= interp_plt_sect_low
+              && sym_addr + load_addr < interp_plt_sect_high)
+	    {
+	      interp_sect = bfd_get_section_by_name (tmp_bfd, ".plt");
+	      sect_offset = interp_plt_sect_low - load_addr;
+	    }
+	  else
+	    {
+	      interp_sect = bfd_get_section_by_name (tmp_bfd, ".opd");
+	      if (interp_sect != 0)
+		{
+		  sect_offset = bfd_section_vma (tmp_bfd, interp_sect);
+		  if (sym_addr < sect_offset)
+		    interp_sect == 0;
+		  else if (sym_addr - sect_offset >=
+				       bfd_section_size (tmp_bfd, interp_sect))
+		    interp_sect == 0;
+		}
+	    }
+	  if (interp_sect != 0)
+	    {
+	      /* Try to convert the function descriptor we found above, into
+		 the address we need.  It will be relocated below by adding
+		 "load_addr" to it. */
+	      char *buf = alloca (sizeof (LONGEST));
+	      if (bfd_get_section_contents (tmp_bfd, interp_sect, buf,
+					    sym_addr - sect_offset,
+					    sizeof (LONGEST)))
+		sym_addr = extract_unsigned_integer (buf, sizeof (LONGEST));
+	      else
+		sym_addr = 0;
+	    }
+        }
+
       /* We're done with both the temporary bfd and target.  Remember,
          closing the target closes the underlying bfd.  */
       target_close (tmp_bfd_target, 0);

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

[Daniel Jacobowitz]

On Wed, May 24, 2006 at 04:26:11PM -0700, PAUL GILLIAM wrote:
> +	      interp_sect = bfd_get_section_by_name (tmp_bfd, ".plt");

> +	      interp_sect = bfd_get_section_by_name (tmp_bfd, ".opd");

Magic names...

> +	  if (interp_sect != 0)
> +	    {
> +	      /* Try to convert the function descriptor we found above, into
> +		 the address we need.  It will be relocated below by adding
> +		 "load_addr" to it. */
> +	      char *buf = alloca (sizeof (LONGEST));
> +	      if (bfd_get_section_contents (tmp_bfd, interp_sect, buf,
> +					    sym_addr - sect_offset,
> +					    sizeof (LONGEST)))
> +		sym_addr = extract_unsigned_integer (buf, sizeof (LONGEST));
> +	      else
> +		sym_addr = 0;
> +	    }
> +        }
> +

... and a magic load; you have no idea what the format of a function
descriptor is, at this point.

Can you make convert_from_func_ptr_addr do what you need?  It needs a
target_ops; conveniently you've got one (tmp_bfd_target).  Some targets
use a memory read function which honors the supplied target_ops, others
don't.  rs6000's doesn't so you'd need to fix that.

-- 
Daniel Jacobowitz
CodeSourcery

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

[PAUL GILLIAM]

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

On Wed, 2006-05-24 at 22:26 -0400, Daniel Jacobowitz wrote:
> On Wed, May 24, 2006 at 04:26:11PM -0700, PAUL GILLIAM wrote:
> > +	      interp_sect = bfd_get_section_by_name (tmp_bfd, ".plt");
> 
> > +	      interp_sect = bfd_get_section_by_name (tmp_bfd, ".opd");
> 
> Magic names...

These aren't magic: they are common to all 64-bit ELF ABIs.  Well, ok:
maybe they're enchanted.

> 
> > +	  if (interp_sect != 0)
> > +	    {
> > +	      /* Try to convert the function descriptor we found above, into
> > +		 the address we need.  It will be relocated below by adding
> > +		 "load_addr" to it. */
> > +	      char *buf = alloca (sizeof (LONGEST));
> > +	      if (bfd_get_section_contents (tmp_bfd, interp_sect, buf,
> > +					    sym_addr - sect_offset,
> > +					    sizeof (LONGEST)))
> > +		sym_addr = extract_unsigned_integer (buf, sizeof (LONGEST));
> > +	      else
> > +		sym_addr = 0;
> > +	    }
> > +        }
> > +
> 
> ... and a magic load; you have no idea what the format of a function
> descriptor is, at this point.
> 
uffda!  I'm afraid I was just a little bit ppc64-centric.

> Can you make convert_from_func_ptr_addr do what you need?  It needs a
> target_ops; conveniently you've got one (tmp_bfd_target).  Some targets
> use a memory read function which honors the supplied target_ops, others
> don't.  rs6000's doesn't so you'd need to fix that.
> 

Yes! Thank you: "gdbarch_convert_from_func_ptr_addr()" does exactly what
I want.  And while it's true that "rs6000_convert_from_func_ptr_addr()"
does not use the target_ops, "ppc64_linux_convert_from_func_ptr_addr()"
does and that is the version that is invoked on the configuration
reporting the error I am trying to fix.

I went ahead and converted "rs6000_convert_from_func_ptr_addr()" to use
target_ops, but because I have no way to test the change, I thought it
best to make it a separate patch.

So is it OK to commit the solib-svr4.c patch?

Does anyone want to test the rs6000-tdep.c patch and commit it?

-=# Paul #=-

PS:  Uffda is the word a Swed would exclaim when examining the bottom of
their shoe after stepping in a fresh cow-pie or after a similarly
annoying event.

[-- Attachment #2: loader_break.diff --]
[-- Type: text/x-patch, Size: 3280 bytes --]

2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com

	* solib-svr4.c (enable_break): Resolve break address when the
	symbol is found in the data section.

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	25 May 2006 22:12:55 -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
 };
 
@@ -1043,20 +1033,45 @@
       /* 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.  */
+	  /* What we're looking for here is the machine code entry point,
+	     so we are only interested in symbols in code sections.
+
+	     On ABI's that use function descriptors, the linker symbol with
+	     the same name as a C funtion points to that functions descriptor.
+	     when those function descriptors are in the code section, they
+	     contain executable code and we can set a breakpoint there. */
 	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
 	  if (sym_addr != 0)
 	    break;
 	}
 
+      if (sym_addr == 0)
+        {
+	  CORE_ADDR sect_offset;
+	  
+	  /* No symbol was found in a code section, so look in the data
+             sections.  This will only happen when the linker symbol points
+	     to a function descriptor that is in a data section. */
+	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
+	    {
+	      /* On ABI's that use function descriptors that are in the data
+	         section, */
+	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
+	      if (sym_addr != 0)
+		break;
+	    }
+	  if (sym_addr == 0)
+	    {
+	      target_close (tmp_bfd_target, 0);
+	      goto bkpt_at_symbol;
+	    }
+
+	  /* Convert 'sym_addr' from a function pointer to an address. */
+	  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);

[-- Attachment #3: rs6000.diff --]
[-- Type: text/x-patch, Size: 1286 bytes --]

2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com

        *rs6000-tdep.c (rs6000_convert_from_func_ptr_addr): Use target_ops.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.258
diff -a -u -r1.258 rs6000-tdep.c
--- rs6000-tdep.c	23 Apr 2006 14:15:01 -0000	1.258
+++ rs6000-tdep.c	25 May 2006 22:44:07 -0000
@@ -2338,7 +2338,8 @@
    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.  */
+   function address from a function pointer.  It is also used by
+   enable_break from svr4_solib_create_inferior_hook.  */
 
 /* Return real function address if ADDR (a function pointer) is in the data
    space and is therefore a special function pointer.  */
@@ -2355,7 +2356,9 @@
     return addr;
 
   /* ADDR is in the data space, so it's a special function pointer. */
-  return read_memory_addr (addr, gdbarch_tdep (current_gdbarch)->wordsize);
+
+  return get_target_memory_unsigned (targ, addr,
+                                     gdbarch_tdep (current_gdbarch)->wordsize);
 }
 
 

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

[Daniel Jacobowitz]

On Thu, May 25, 2006 at 02:52:50PM -0700, PAUL GILLIAM wrote:
> 2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com
> 
> 	* solib-svr4.c (enable_break): Resolve break address when the
> 	symbol is found in the data section.

Need a changelog for the removal of "._dl_debug_state", too.

I don't feel that I can really review this; I'm going to wait a while
and hope someone more familiar with a function descriptor platform
responds.

-- 
Daniel Jacobowitz
CodeSourcery

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

[PAUL GILLIAM]

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

Boy! That was fast!  Thanks!

On Thu, 2006-05-25 at 18:58 -0400, Daniel Jacobowitz wrote:
> On Thu, May 25, 2006 at 02:52:50PM -0700, PAUL GILLIAM wrote:
> > 2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com
> > 
> > 	* solib-svr4.c (enable_break): Resolve break address when the
> > 	symbol is found in the data section.
> 
> Need a changelog for the removal of "._dl_debug_state", too.
> 
OK.

> I don't feel that I can really review this; I'm going to wait a while
> and hope someone more familiar with a function descriptor platform
> responds.
> 
I suppose that goes for the rs6000-tdep.c patch as well.

-=# Paul #=-

PS: Any chance of getting these into 6.5?

[-- Attachment #2: loader_break.diff --]
[-- Type: text/x-patch, Size: 3392 bytes --]

2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com

	* solib-svr4.c: Remove "._dl_debug_state" from the list of symbols at
	which to try setting a breakpoint for tracking solibs.
	(enable_break): Resolve break address when the symbol is found in
	the data section.

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	25 May 2006 22:12:55 -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
 };
 
@@ -1043,20 +1033,45 @@
       /* 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.  */
+	  /* What we're looking for here is the machine code entry point,
+	     so we are only interested in symbols in code sections.
+
+	     On ABI's that use function descriptors, the linker symbol with
+	     the same name as a C funtion points to that functions descriptor.
+	     when those function descriptors are in the code section, they
+	     contain executable code and we can set a breakpoint there. */
 	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
 	  if (sym_addr != 0)
 	    break;
 	}
 
+      if (sym_addr == 0)
+        {
+	  CORE_ADDR sect_offset;
+	  
+	  /* No symbol was found in a code section, so look in the data
+             sections.  This will only happen when the linker symbol points
+	     to a function descriptor that is in a data section. */
+	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
+	    {
+	      /* On ABI's that use function descriptors that are in the data
+	         section, */
+	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
+	      if (sym_addr != 0)
+		break;
+	    }
+	  if (sym_addr == 0)
+	    {
+	      target_close (tmp_bfd_target, 0);
+	      goto bkpt_at_symbol;
+	    }
+
+	  /* Convert 'sym_addr' from a function pointer to an address. */
+	  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);

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

[PAUL GILLIAM]

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

It's been just about a month, so I thought I try again (and use the new
patch tracker!)

:ADDPATCH PowerPC-64:

> On Thu, 2006-05-25 at 18:58 -0400, Daniel Jacobowitz wrote:

> > I don't feel that I can really review this; I'm going to wait a while
> > and hope someone more familiar with a function descriptor platform
> > responds.

I have attached the patches again.  'loader_break.diff' applied cleanly
to cvs-head, but I had to do a trivial tweak to 'rs6000.diff' so that it
would.

OK to commit?

-=# Paul #=-



[-- Attachment #2: loader_break.diff --]
[-- Type: text/x-patch, Size: 3392 bytes --]

2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com

	* solib-svr4.c: Remove "._dl_debug_state" from the list of symbols at
	which to try setting a breakpoint for tracking solibs.
	(enable_break): Resolve break address when the symbol is found in
	the data section.

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	25 May 2006 22:12:55 -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
 };
 
@@ -1043,20 +1033,45 @@
       /* 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.  */
+	  /* What we're looking for here is the machine code entry point,
+	     so we are only interested in symbols in code sections.
+
+	     On ABI's that use function descriptors, the linker symbol with
+	     the same name as a C funtion points to that functions descriptor.
+	     when those function descriptors are in the code section, they
+	     contain executable code and we can set a breakpoint there. */
 	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
 	  if (sym_addr != 0)
 	    break;
 	}
 
+      if (sym_addr == 0)
+        {
+	  CORE_ADDR sect_offset;
+	  
+	  /* No symbol was found in a code section, so look in the data
+             sections.  This will only happen when the linker symbol points
+	     to a function descriptor that is in a data section. */
+	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
+	    {
+	      /* On ABI's that use function descriptors that are in the data
+	         section, */
+	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
+	      if (sym_addr != 0)
+		break;
+	    }
+	  if (sym_addr == 0)
+	    {
+	      target_close (tmp_bfd_target, 0);
+	      goto bkpt_at_symbol;
+	    }
+
+	  /* Convert 'sym_addr' from a function pointer to an address. */
+	  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);

[-- Attachment #3: rs6000.diff --]
[-- Type: text/x-patch, Size: 1287 bytes --]

2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com

        *rs6000-tdep.c (rs6000_convert_from_func_ptr_addr): Use target_ops.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.258
diff -a -u -r1.258 rs6000-tdep.c
--- rs6000-tdep.c	23 Apr 2006 14:15:01 -0000	1.258
+++ rs6000-tdep.c	23 Jun 2006 21:22:32 -0000
@@ -2338,7 +2338,8 @@
    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.  */
+   function address from a function pointer.  It is also used by
+   enable_break from svr4_solib_create_inferior_hook.  */
 
 /* Return real function address if ADDR (a function pointer) is in the data
    space and is therefore a special function pointer.  */
@@ -2355,7 +2356,9 @@
     return addr;
 
   /* ADDR is in the data space, so it's a special function pointer. */
-  return read_memory_addr (addr, gdbarch_tdep (current_gdbarch)->wordsize);
+
+  return get_target_memory_unsigned (targ, addr,
+                                     gdbarch_tdep (current_gdbarch)->wordsize);
 }
 \f
 

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

[Mark Kettenis]

> From: PAUL GILLIAM <pgilliam@us.ibm.com>
> Date: Fri, 23 Jun 2006 13:29:13 -0700
>
> > > I don't feel that I can really review this; I'm going to wait a while
> > > and hope someone more familiar with a function descriptor platform
> > > responds.
> 
> I have attached the patches again.  'loader_break.diff' applied cleanly
> to cvs-head, but I had to do a trivial tweak to 'rs6000.diff' so that it
> would.
> 
> OK to commit?

You didn't answer the question I asked about this patch:

http://sourceware.org/ml/gdb-patches/2006-06/msg00245.html

Mark

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

[PAUL GILLIAM]

On Fri, 2006-06-23 at 23:48 +0200, Mark Kettenis wrote:
> > From: PAUL GILLIAM <pgilliam@us.ibm.com>
> > 
> > OK to commit?
> 
> You didn't answer the question I asked about this patch:
> 
> http://sourceware.org/ml/gdb-patches/2006-06/msg00245.html
> 
> Mark

Sorry.  I don't know how I missed that: good catch.

There was really no reason to delete the dot symbol.

So here is the patch with out that first part...  it does what I want
with the dot symbol still there (as long as it's last in the list).

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

[PAUL GILLIAM]

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

Here is the new patch, with out deleting the 'dot' symbol.  I included a
copy of the rs6000 patch as well, just for completeness.

OK to commit?

-=# Paul #=-


[-- Attachment #2: loader_break.diff --]
[-- Type: text/x-patch, Size: 2675 bytes --]

2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com

	* solib-svr4.c (enable_break): Resolve break address when the symbol
	is found in the data section.

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	26 Jun 2006 22:08:43 -0000
@@ -1043,20 +1043,45 @@
       /* 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.  */
+	  /* What we're looking for here is the machine code entry point,
+	     so we are only interested in symbols in code sections.
+
+	     On ABI's that use function descriptors, the linker symbol with
+	     the same name as a C funtion points to that functions descriptor.
+	     When those function descriptors are in the code section, they
+	     contain executable code and we can set a breakpoint there. */
 	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
 	  if (sym_addr != 0)
 	    break;
 	}
 
+      if (sym_addr == 0)
+        {
+	  CORE_ADDR sect_offset;
+	  
+	  /* No symbol was found in a code section, so look in the data
+             sections.  This will only happen when the linker symbol points
+	     to a function descriptor that is in a data section. */
+	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
+	    {
+	      /* On ABI's that use function descriptors that are in the data
+	         section, */
+	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
+	      if (sym_addr != 0)
+		break;
+	    }
+	  if (sym_addr == 0)
+	    {
+	      target_close (tmp_bfd_target, 0);
+	      goto bkpt_at_symbol;
+	    }
+
+	  /* Convert 'sym_addr' from a function pointer to an address. */
+	  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);

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

[Kevin Buettner]

On Mon, 26 Jun 2006 14:23:41 -0700
PAUL GILLIAM <pgilliam@us.ibm.com> wrote:

> Here is the new patch, with out deleting the 'dot' symbol.  I included a
> copy of the rs6000 patch as well, just for completeness.

I didn't see the rs6000 patch.

> OK to commit?

Not quite yet...

> 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	26 Jun 2006 22:08:43 -0000
> @@ -1043,20 +1043,45 @@
>        /* 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.  */

I have a few suggestions regarding your rewrite of the comment:

> +	  /* What we're looking for here is the machine code entry point,
> +	     so we are only interested in symbols in code sections.
> +
> +	     On ABI's that use function descriptors, the linker symbol with
                ^^^^^
		ABIs
> +	     the same name as a C funtion points to that functions descriptor.
                                  ^^^^^^^                ^^^^^^^^^
				  function               function's
> +	     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.

>  	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
>  	  if (sym_addr != 0)
>  	    break;
>  	}
>  
> +      if (sym_addr == 0)
> +        {
> +	  CORE_ADDR sect_offset;
> +	  
> +	  /* No symbol was found in a code section, so look in the data
> +             sections.  This will only happen when the linker symbol points
> +	     to a function descriptor that is in a data section. */
> +	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
> +	    {
> +	      /* On ABI's that use function descriptors that are in the data
> +	         section, */
> +	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
> +	      if (sym_addr != 0)
> +		break;
> +	    }

Starting from the line immediately below...
> +	  if (sym_addr == 0)
> +	    {
> +	      target_close (tmp_bfd_target, 0);
> +	      goto bkpt_at_symbol;
> +	    }
...through the line immediately above, could we delete those lines and
instead just say:

	  if (sym_addr != 0)

before the assignment (sym_addr = gdbarch_convert...) below?

(This gets rid of the goto and the extra call to target_close().)
> +
> +	  /* Convert 'sym_addr' from a function pointer to an address. */
> +	  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);

With my suggested changes above, I think this is okay.  I'd like to
see another patch posted to this list though prior to committing...

Thanks,

Kevin

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

[Kevin Buettner]

On Fri, 23 Jun 2006 13:29:13 -0700
PAUL GILLIAM <pgilliam@us.ibm.com> wrote:

> 	* solib-svr4.c: Remove "._dl_debug_state" from the list of symbols at
> 	which to try setting a breakpoint for tracking solibs.

I too would like to know whether or not it is still possible to debug
binaries created on older versions of the OS after removing
._dl_debug_state.  (Mark asked this question first, and thus far, I've
not seen an answer.)

What happens if this portion of the patch is omitted?  Does your patch
still work for the case that you're interesting in?

Kevin

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

[Mark Kettenis]

> From: PAUL GILLIAM <pgilliam@us.ibm.com>
> Date: Wed, 24 May 2006 16:26:11 -0700
> 
> --=-z0a1QHXPsj5sKoA5562L
> Content-Type: text/plain
> Content-Transfer-Encoding: 7bit
> 
> 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 used for a while.

Your patch removes "._dl_debug_state" from the list of symbols.
Doesn't this break debugging old binaries that still have the 'dot'
symbols?

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

[PAUL GILLIAM]

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

On Sat, 2006-06-17 at 22:21 +0200, Mark Kettenis wrote:
> > From: PAUL GILLIAM <pgilliam@us.ibm.com>
> > Date: Wed, 24 May 2006 16:26:11 -0700
> > 
> > --=-z0a1QHXPsj5sKoA5562L
> > Content-Type: text/plain
> > Content-Transfer-Encoding: 7bit
> > 
> > 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 used for a while.
> 
> Your patch removes "._dl_debug_state" from the list of symbols.
> Doesn't this break debugging old binaries that still have the 'dot'
> symbols?

No. The non-'dot' symbol "_dl_debug_state" will either be found in a
code segment or in a data segment.  If found in a code segment, we can
set the breakpoint at where ever the symbol points.  If it points to a
function descriptor that's OK because if it's in the code segment, a
function descriptor consists of executable code and a breakpoint can be
set there.  If found in the data segment, then it must be a function
descriptor and it just gets "dereferenced".

So I am attaching a new patch that addresses most of your comments in
http://sourceware.org/ml/gdb-patches/2006-06/msg00382.html
and deletes the "._dl_debug_state" entry in the table.

One thing not addressed is this:

> 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 two linker symbols issue is no longer relevant for this piece of
code: the "._dl_debug_state" symbol was just a crutch used to avoid
having to "dereference" the function descriptor pointed to by
"_dl_debug_state", should it happen to be in a data section.  Now there
is code to do just that.


OK to commit?
 


[-- Attachment #2: loader_break.diff --]
[-- Type: text/x-patch, Size: 3061 bytes --]

2006-07-04  Paul Gilliam  <pgilliam@us.ibm.com

	* solib-svr4.c (enable_break): Resolve break address when the symbol
	is found in the data section.

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	6 Jul 2006 00:50:51 -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
 };
 
@@ -1043,20 +1033,38 @@
       /* 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.  */
+	  /* What we're looking for here is the machine code entry point,
+	     so we are only interested in symbols in code sections.
+
+	     On ABI's that use function descriptors, the linker symbol with
+	     the same name as a C funtion points to that functions descriptor.
+	     When those function descriptors are in the code section, they
+	     contain executable code and we can set a breakpoint there. */
 	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
 	  if (sym_addr != 0)
 	    break;
 	}
 
+      if (sym_addr == 0)
+        {
+	  CORE_ADDR sect_offset;
+	  
+	  /* No symbol was found in a code section, so look elsewhere. */
+	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
+	    {
+	      /* On ABI's that use function descriptors that are in the data
+	         section, */
+	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
+	      if (sym_addr != 0)
+		break;
+	    }
+	  if (sym_addr != 0)
+	    /* Convert 'sym_addr' from a function pointer to an address. */
+	    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);

[-- Attachment #3: rs6000.diff --]
[-- Type: text/x-patch, Size: 1287 bytes --]

2006-05-25  Paul Gilliam  <pgilliam@us.ibm.com

        *rs6000-tdep.c (rs6000_convert_from_func_ptr_addr): Use target_ops.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.258
diff -a -u -r1.258 rs6000-tdep.c
--- rs6000-tdep.c	23 Apr 2006 14:15:01 -0000	1.258
+++ rs6000-tdep.c	23 Jun 2006 21:22:32 -0000
@@ -2338,7 +2338,8 @@
    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.  */
+   function address from a function pointer.  It is also used by
+   enable_break from svr4_solib_create_inferior_hook.  */
 
 /* Return real function address if ADDR (a function pointer) is in the data
    space and is therefore a special function pointer.  */
@@ -2355,7 +2356,9 @@
     return addr;
 
   /* ADDR is in the data space, so it's a special function pointer. */
-  return read_memory_addr (addr, gdbarch_tdep (current_gdbarch)->wordsize);
+
+  return get_target_memory_unsigned (targ, addr,
+                                     gdbarch_tdep (current_gdbarch)->wordsize);
 }
 \f
 

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

[Daniel Jacobowitz]

Formatting notes.

On Wed, Jul 05, 2006 at 04:57:23PM -0700, PAUL GILLIAM wrote:
> +	     contain executable code and we can set a breakpoint there. */

Two spaces after period, here and elsewhere.

> +	  /* No symbol was found in a code section, so look elsewhere. */
> +	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)

Line is too long; also related, spaces around operators.

> +	      /* On ABI's that use function descriptors that are in the data
> +	         section, */

Lost a bit of this comment?


-- 
Daniel Jacobowitz
CodeSourcery

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

[Kevin Buettner]

On Wed, 05 Jul 2006 16:57:23 -0700
PAUL GILLIAM <pgilliam@us.ibm.com> wrote:

> +	  /* What we're looking for here is the machine code entry point,
> +	     so we are only interested in symbols in code sections.
> +
> +	     On ABI's that use function descriptors, the linker symbol with
> +	     the same name as a C funtion points to that functions descriptor.
> +	     When those function descriptors are in the code section, they
> +	     contain executable code and we can set a breakpoint there. */

See my earlier post regarding this comment:

  http://sources.redhat.com/ml/gdb-patches/2006-06/msg00382.html

In addition to the problems that Daniel has found, there are three
other punctuation / spelling problems that I had brought to your
attention.

Kevin

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

[Andreas Schwab]

PAUL GILLIAM <pgilliam@us.ibm.com> writes:

> +	  if (sym_addr != 0)
> +	    /* Convert 'sym_addr' from a function pointer to an address. */
> +	    sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch,
> +							   sym_addr,
> +							   tmp_bfd_target);

Why do you call gdbarch_convert_from_func_ptr_addr here?  This is already
called by adjust_breakpoint_address via
ppc64_sysv_abi_adjust_breakpoint_address when setting the breakpoint.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[PAUL GILLIAM]

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

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.


[-- Attachment #2: dynamic-loader-breakpoint.diff --]
[-- Type: text/x-patch, Size: 9868 bytes --]

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;
 	}

[PATCH] Fixes problem setting breakpoint in dynamic loader

[Thiago Jung Bauermann]

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

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 #2: comments.txt --]
[-- Type: text/plain, Size: 3997 bytes --]

My comments in [].

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

> +	  if (sym_addr != 0)
> +	    /* Convert 'sym_addr' from a function pointer to an address. */
> +	    sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch,
> +							   sym_addr,
> +							   tmp_bfd_target);

Why do you call gdbarch_convert_from_func_ptr_addr here?  This is already
called by adjust_breakpoint_address via
ppc64_sysv_abi_adjust_breakpoint_address when setting the breakpoint.

[ that code is gone now ]

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

Formatting notes.

On Wed, Jul 05, 2006 at 04:57:23PM -0700, PAUL GILLIAM wrote:
> +	     contain executable code and we can set a breakpoint there. */

Two spaces after period, here and elsewhere.

[ that code is gone now ]

> +	  /* No symbol was found in a code section, so look elsewhere. */
> +	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)

Line is too long; also related, spaces around operators.

[ that code is gone now ]

> +	      /* On ABI's that use function descriptors that are in the data
> +	         section, */

Lost a bit of this comment?

[ that code is gone now ]
-- 
Daniel Jacobowitz
CodeSourcery
--------------------------------------------------------------------------------
http://sources.redhat.com/ml/gdb-patches/2006-06/msg00382.html

> +	  /* What we're looking for here is the machine code entry point,
> +	     so we are only interested in symbols in code sections.
> +
> +	     On ABI's that use function descriptors, the linker symbol with
                ^^^^^
		ABIs
[ fixed ]

> +	     the same name as a C funtion points to that functions descriptor.
                                  ^^^^^^^                ^^^^^^^^^
				  function               function's

[ that code is gone now ]

> +	     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.

[ see message ]

>  	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
>  	  if (sym_addr != 0)
>  	    break;
>  	}
>  
> +      if (sym_addr == 0)
> +        {
> +	  CORE_ADDR sect_offset;
> +	  
> +	  /* No symbol was found in a code section, so look in the data
> +             sections.  This will only happen when the linker symbol points
> +	     to a function descriptor that is in a data section. */
> +	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
> +	    {
> +	      /* On ABI's that use function descriptors that are in the data
> +	         section, */
> +	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
> +	      if (sym_addr != 0)
> +		break;
> +	    }

Starting from the line immediately below...
> +	  if (sym_addr == 0)
> +	    {
> +	      target_close (tmp_bfd_target, 0);
> +	      goto bkpt_at_symbol;
> +	    }
...through the line immediately above, could we delete those lines and
instead just say:

	  if (sym_addr != 0)

before the assignment (sym_addr = gdbarch_convert...) below?

(This gets rid of the goto and the extra call to target_close().)

[ that code is gone now ]

> +
> +	  /* Convert 'sym_addr' from a function pointer to an address. */
> +	  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);

With my suggested changes above, I think this is okay.  I'd like to
see another patch posted to this list though prior to committing...

Thanks,

Kevin

[-- Attachment #3: loader-break-2007-04-27.diff --]
[-- Type: text/x-patch, Size: 10096 bytes --]

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;
 	}

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

[Joseph S. Myers]

On Fri, 27 Apr 2007, Thiago Jung Bauermann wrote:

> Hi folks,
> 
> This is a re-submission of:
> 
> http://sourceware.org/ml/gdb-patches/2006-07/msg00264.html

This doesn't work properly for me.  There seem to be at least two problems 
with relying on the implicit conversion by 
ppc_linux_convert_from_func_ptr_addr done when setting a breakpoint:

* ppc_linux_convert_from_func_ptr_addr relies on being able to find the 
function descriptor address in a section table, but doesn't have current 
information about the load address of ld.so which has only just been 
determined in enable_break.

* The function descriptor in ld.so has not yet been relocated (as part of 
ld.so applying relocations to itself) at the time this breakpoint is set.  
So after the address is extracted from the function descriptor, the load 
address of ld.so must be added to it again.  (I think this provides an 
answer to Andreas's question in 
<http://sourceware.org/ml/gdb-patches/2006-07/msg00183.html>: the explicit 
conversion is needed so that this relocation applies when load_addr gets 
added to the address later before the breakpoint is set.)

I have a patch version partly based on the earlier revision 
<http://sourceware.org/ml/gdb-patches/2006-07/msg00019.html> which does 
set the breakpoint correctly, but I still have other GDB problems I'm 
investigating with that patch applied.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Daniel Jacobowitz]

On Thu, Jun 14, 2007 at 11:38:33PM +0000, Joseph S. Myers wrote:
> I have a patch version partly based on the earlier revision 
> <http://sourceware.org/ml/gdb-patches/2006-07/msg00019.html> which does 
> set the breakpoint correctly, but I still have other GDB problems I'm 
> investigating with that patch applied.

Here's an extended version of Joseph's revision of Thiago's revision
of Paul's patch (begat by... who begat...).  I tested it with
gdbserver on powerpc-linux -m64; it fixes the function calling test in
nodebug.exp and all the threading tests.  The branch I was testing on
is a little bit older than HEAD, and I had an unstripped libc; but I
believe that on HEAD and with a stripped libc, it will also fix a
number of shared library load/unload tests and all of the calls to
malloc to create strings.  I'm going to test it natively now.

The threading tests are a remote-specific problem.  gdbserver asks
"where's __nptl_create_event?" and gdb was responding with the .opd
location.  We need to give the function instead, since gdbserver is
going to put a breakpoint there.  This requires handling memory reads
(from the .opd entry) during qSymbol conversations.

Any comments on this patch?

-- 
Daniel Jacobowitz
CodeSourcery

2007-06-20  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: remote.c
===================================================================
--- remote.c	(revision 174465)
+++ remote.c	(working copy)
@@ -2187,9 +2187,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: infcall.c
===================================================================
--- infcall.c	(revision 174465)
+++ infcall.c	(working copy)
@@ -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: ppc-linux-tdep.c
===================================================================
--- ppc-linux-tdep.c	(revision 174465)
+++ ppc-linux-tdep.c	(working copy)
@@ -725,39 +725,73 @@ 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 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;
 }
@@ -1038,6 +1072,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
@@ -1065,13 +1104,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: 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
   if (len < 0)
     return -1;
 
-  if (strncmp (own_buf, "qSymbol:", strlen ("qSymbol:")) != 0)
+  /* 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
+     packet.  */
+  while (own_buf[0] == 'm')
     {
-      /* Malformed response.  */
-      if (remote_debug)
-	{
-	  fprintf (stderr, "Malformed response to qSymbol, ignoring.\n");
-	  fflush (stderr);
-	}
+      CORE_ADDR mem_addr;
+      unsigned char *mem_buf;
 
+      decode_m_packet (&own_buf[1], &mem_addr, &len);
+      mem_buf = malloc (len);
+      if (read_inferior_memory (mem_addr, mem_buf, len) == 0)
+	convert_int_to_ascii (mem_buf, own_buf, 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)
+    {
+      warning ("Malformed response to qSymbol, ignoring: %s\n", own_buf);
       return -1;
     }
 
Index: solib-svr4.c
===================================================================
--- solib-svr4.c	(revision 174465)
+++ solib-svr4.c	(working copy)
@@ -85,16 +85,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
 };
 
@@ -317,7 +307,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 *);
 
 /*
 
@@ -327,24 +317,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;
@@ -366,9 +357,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;
 	    }
@@ -395,9 +386,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;
 	    }
@@ -1105,7 +1096,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)
 	{
@@ -1128,7 +1119,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
@@ -1153,20 +1144,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);

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

[Daniel Jacobowitz]

On Wed, Jun 20, 2007 at 09:15:24AM -0400, Daniel Jacobowitz wrote:
> Here's an extended version of Joseph's revision of Thiago's revision
> of Paul's patch (begat by... who begat...).  I tested it with
> gdbserver on powerpc-linux -m64; it fixes the function calling test in
> nodebug.exp and all the threading tests.  The branch I was testing on
> is a little bit older than HEAD, and I had an unstripped libc; but I
> believe that on HEAD and with a stripped libc, it will also fix a
> number of shared library load/unload tests and all of the calls to
> malloc to create strings.  I'm going to test it natively now.

The shared library tests seem generally OK, and it fixes all sorts of
tests that call malloc or fork.

Unless someone sees a problem with this patch, I would like to include
it in GDB 6.7.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Thiago Jung Bauermann]

On Wed, 2007-06-20 at 10:21 -0400, Daniel Jacobowitz wrote:
> On Wed, Jun 20, 2007 at 09:15:24AM -0400, Daniel Jacobowitz wrote:
> > Here's an extended version of Joseph's revision of Thiago's revision
> > of Paul's patch (begat by... who begat...).  I tested it with
> > gdbserver on powerpc-linux -m64; it fixes the function calling test in
> > nodebug.exp and all the threading tests.  The branch I was testing on
> > is a little bit older than HEAD, and I had an unstripped libc; but I
> > believe that on HEAD and with a stripped libc, it will also fix a
> > number of shared library load/unload tests and all of the calls to
> > malloc to create strings.  I'm going to test it natively now.
> 
> The shared library tests seem generally OK, and it fixes all sorts of
> tests that call malloc or fork.

This patch fixes my problem as well. Thanks for helping with this.

> Unless someone sees a problem with this patch, I would like to include
> it in GDB 6.7.

Me too.
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

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

[Ulrich Weigand]

Dan Jacobowitz wrote:
> The threading tests are a remote-specific problem.  gdbserver asks
> "where's __nptl_create_event?" and gdb was responding with the .opd
> location.  We need to give the function instead, since gdbserver is
> going to put a breakpoint there.  This requires handling memory reads
> (from the .opd entry) during qSymbol conversations.

Nice!  I've had a local hack in our tree to fix this, but your solution
is a lot cleaner.  Thanks for taking care of this.

> Any comments on this patch?

Looks OK to me.

Bye,
Ulrich

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

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

[Ulrich Weigand]

Daniel Jacobowitz wrote:

One minor issue I just noticed:

> 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.

Bye,
Ulrich

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

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

[Daniel Jacobowitz]

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;
     }
 

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

[Joel Brobecker]

Daniel,

You said you wanted this to be part of 6.7. Ulrich said it looked OK,
so can we commit this patch sometime soon?

> 2007-06-20  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.

Thanks,
-- 
Joel

Re: [PATCH] -var-update

[Nick Roberts]

> > Anyway here's a patch that actually seems to work.  I've taken code from
> > c_val_print.  Maybe there should be more checks and I've not tested
> > varobj_set_value (-var-assign) yet, but I thought I'd sound out the
> > genaral approcah first.

> I think that in any case, you should add a function and call it, instead of
> adding three identical code fragments.

It's just for discussion, not a finalised patch for approval.

>                                         BTW, why coerce_ref is not suitable
> here?

coerce_ref ensures that the address is placed in the value's contents, not
the actual value which is being referred to.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH] -var-update

[Daniel Jacobowitz]

On Sat, May 20, 2006 at 04:23:04PM +1200, Nick Roberts wrote:
> >                                         BTW, why coerce_ref is not suitable
> > here?
> 
> coerce_ref ensures that the address is placed in the value's contents, not
> the actual value which is being referred to.

Did you try that?  That certainly shouldn't be true!

struct value *
coerce_ref (struct value *arg)
{
  struct type *value_type_arg_tmp = check_typedef (value_type (arg));
  if (TYPE_CODE (value_type_arg_tmp) == TYPE_CODE_REF)
    arg = value_at_lazy (TYPE_TARGET_TYPE (value_type_arg_tmp),
                         unpack_pointer (value_type (arg),              
                                         value_contents (arg)));
  return arg;
}


-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] -var-update

[Nick Roberts]

 > > >                                         BTW, why coerce_ref is not
 > > > suitable here?
 > > 
 > > coerce_ref ensures that the address is placed in the value's contents, not
 > > the actual value which is being referred to.
 > 
 > Did you try that?  That certainly shouldn't be true!

Yes.  Perhaps my summary isn't precise, but it didn't work.  My patch uses
value_at while coerce_ref uses value_at_lazy.  The comment says:

   Call value_at only if the data needs to be fetched immediately;
   if we can be 'lazy' and defer the fetch, perhaps indefinately, call
                                                    ^^^^^^^^^^^^
   value_at_lazy instead.  value_at_lazy simply records the address of
   the data and sets the lazy-evaluation-required flag.  The lazy flag
   is tested in the value_contents macro, which is used if and when
   the contents are actually required.

value_contents is not a macro (VALUE_CONTENTS used to be one) but a function,
and doesn't test the lazy flag.  So the comment is out of date, but perhaps
the `data' (I don't know what that means exactly) is never fetched when
-var-update is issued.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH] -var-update

[Daniel Jacobowitz]

On Sun, May 21, 2006 at 10:25:24AM +1200, Nick Roberts wrote:
> Yes.  Perhaps my summary isn't precise, but it didn't work.  My patch uses
> value_at while coerce_ref uses value_at_lazy.  The comment says:
> 
>    Call value_at only if the data needs to be fetched immediately;
>    if we can be 'lazy' and defer the fetch, perhaps indefinately, call
>                                                     ^^^^^^^^^^^^
>    value_at_lazy instead.  value_at_lazy simply records the address of
>    the data and sets the lazy-evaluation-required flag.  The lazy flag
>    is tested in the value_contents macro, which is used if and when
>    the contents are actually required.
> 
> value_contents is not a macro (VALUE_CONTENTS used to be one) but a function,
> and doesn't test the lazy flag.

No:

const gdb_byte *
value_contents (struct value *value)
{ 
  return value_contents_writeable (value);
}

gdb_byte *
value_contents_writeable (struct value *value)
{ 
  if (value->lazy)
    value_fetch_lazy (value);
  return value_contents_raw (value);
}

If you take a look at the code you're patching, there should be a
nearby call to value_fetch_lazy or gdb_value_fetch_lazy in each case.
You want to be calling coerce_ref before you do that.  Also, see the
existing calls to release_value?  If you change var->value after that,
you're going to leak memory.

Try calling coerce_ref in here:

      if (gdb_evaluate_expression (var->root->exp, &var->value))
        {
          /* no error */

/* HERE */

          release_value (var->value);
          if (value_lazy (var->value))
            gdb_value_fetch_lazy (var->value);
        }
      else
        var->value = evaluate_type (var->root->exp);


-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] -var-update

[Nick Roberts]

 > If you take a look at the code you're patching, there should be a
 > nearby call to value_fetch_lazy or gdb_value_fetch_lazy in each case.
 > You want to be calling coerce_ref before you do that.

OK.  Perhaps I called it after value_fetch_lazy.  

 > Try calling coerce_ref in here:
 > 
 >       if (gdb_evaluate_expression (var->root->exp, &var->value))
 >         {
 >           /* no error */
 > 
 > /* HERE */
 > 
 >           release_value (var->value);
 >           if (value_lazy (var->value))
 >             gdb_value_fetch_lazy (var->value);
 >         }
 >       else
 >         var->value = evaluate_type (var->root->exp);

I've tried to follow this suggestion in the patch below, which acts slightly
differently to the previous patch.  Itjust returns the actual value in the
value field instead of the address and value (which is what -stack-list-locals
does).  This is because the previously execution went through case
TYPE_CODE_REF in c-val-print, while now it goes through TYPE_CODE_INT say (for
a variable referencing an integer) because of the call to coerce_ref.  I'm
not sure which is preferable.

Now the change occurs earlier in varobj_create, I'm not sure what happens when
gdb_evaluate_expression returns 0 (or why it would return 0).

-- 
Nick                                           http://www.inet.net.nz/~nickrob


*** varobj.c	19 May 2006 11:37:28 +1200	1.60
--- varobj.c	21 May 2006 16:06:23 +1200	
*************** varobj_create (char *objname,
*** 502,513 ****
  	  select_frame (fi);
  	}
  
!       /* We definitively need to catch errors here.
           If evaluate_expression succeeds we got the value we wanted.
           But if it fails, we still go on with a call to evaluate_type()  */
        if (gdb_evaluate_expression (var->root->exp, &var->value))
  	{
  	  /* no error */
  	  release_value (var->value);
  	  if (value_lazy (var->value))
  	    gdb_value_fetch_lazy (var->value);
--- 502,514 ----
  	  select_frame (fi);
  	}
  
!       /* We definitely need to catch errors here.
           If evaluate_expression succeeds we got the value we wanted.
           But if it fails, we still go on with a call to evaluate_type()  */
        if (gdb_evaluate_expression (var->root->exp, &var->value))
  	{
  	  /* no error */
+ 	  var->value = coerce_ref (var->value);
  	  release_value (var->value);
  	  if (value_lazy (var->value))
  	    gdb_value_fetch_lazy (var->value);
*************** varobj_set_value (struct varobj *var, ch
*** 820,825 ****
--- 821,830 ----
        exp = parse_exp_1 (&s, 0, 0);
        if (!gdb_evaluate_expression (exp, &value))
  	{
+ 	  value = coerce_ref (value);
+ 	  if (value_lazy (value))
+ 	    gdb_value_fetch_lazy (value);
+ 
  	  /* We cannot proceed without a valid expression. */
  	  xfree (exp);
  	  return 0;
*************** c_value_of_root (struct varobj **var_han
*** 1898,1903 ****
--- 1903,1909 ----
           go on */
        if (gdb_evaluate_expression (var->root->exp, &new_val))
  	{
+ 	  new_val = coerce_ref (new_val);
  	  if (value_lazy (new_val))
  	    {
  	      /* We need to catch errors because if

Re: [PATCH] -var-update

[Nick Roberts]

 >  > Try calling coerce_ref in here:
 >  > 
 >  >       if (gdb_evaluate_expression (var->root->exp, &var->value))
 >  >         {
 >  >           /* no error */
 >  > 
 >  > /* HERE */
 >  > 
 >  >           release_value (var->value);
 >  >           if (value_lazy (var->value))
 >  >             gdb_value_fetch_lazy (var->value);
 >  >         }
 >  >       else
 >  >         var->value = evaluate_type (var->root->exp);
 > 
 > I've tried to follow this suggestion in the patch below,...

Actually I don't think this can be quite right because if I have:

int i;
int& ri = i;

and I do:
-var-create - * ri

I get:
^done,name="var1",numchild="0",type="int"

when I really should have:
^done,name="var1",numchild="0",type="int &"

-- 
Nick                                           http://www.inet.net.nz/~nickrob

[patch] Fixes problem setting breakpoint in dynamic loader

[PAUL GILLIAM]

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

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 used 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.

If none of the symbols on the list was found in a code section, the
attached patch looks in data sections.  When it finds one, and the data
section is either '.plt' or '.opd', then the address points to a
function descriptor which is then used to find the corresponding code
address where the breakpoint can be set.

OK to commit?

-=# Paul #=-



[-- Attachment #2: loader_break.diff --]
[-- Type: text/x-patch, Size: 4204 bytes --]

2006-05-24  Paul Gilliam  <pgilliam@us.ibm.com

	* solib-svr4.c (enable_break): Resolve break address when the
	symbol is found in the data section.

diff -Naur old/solib-svr4.c new/solib-svr4.c
--- old/solib-svr4.c	2006-05-24 15:50:42.000000000 -0700
+++ new/solib-svr4.c	2006-05-24 15:51:36.000000000 -0700
@@ -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
 };
 
@@ -1043,20 +1033,75 @@
       /* 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.  */
+	  /* What we're looking for here is the machine code entry point,
+	     so we are only interested in symbols in code sections.
+
+	     On ABI's that use function descriptors, the linker symbol with
+	     the same name as a C funtion points to that functions descriptor.
+	     when those function descriptors are in the code section, they
+	     contain executable code and we can set a breakpoint there. */
 	  sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE);
 	  if (sym_addr != 0)
 	    break;
 	}
 
+      if (sym_addr == 0)
+        {
+	  CORE_ADDR sect_offset;
+	  
+	  /* No symbol was found in a code section, so look in the data
+             sections.  This will only happen when the linker symbol points
+	     to a function descriptor that is in a data section. */
+	  for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++)
+	    {
+	      /* On ABI's that use function descriptors that are in the data
+	         section,
+	      sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA);
+	      if (sym_addr != 0)
+		break;
+	    }
+	  if (sym_addr == 0)
+	    {
+	      target_close (tmp_bfd_target, 0);
+	      goto bkpt_at_symbol;
+	    }
+
+	  /* On some ABI's, the function descriptor we need will be in the
+	     ".plt" section.  In others, it will be in the ".opd" section. */
+	  if (sym_addr + load_addr >= interp_plt_sect_low
+              && sym_addr + load_addr < interp_plt_sect_high)
+	    {
+	      interp_sect = bfd_get_section_by_name (tmp_bfd, ".plt");
+	      sect_offset = interp_plt_sect_low - load_addr;
+	    }
+	  else
+	    {
+	      interp_sect = bfd_get_section_by_name (tmp_bfd, ".opd");
+	      if (interp_sect != 0)
+		{
+		  sect_offset = bfd_section_vma (tmp_bfd, interp_sect);
+		  if (sym_addr < sect_offset)
+		    interp_sect == 0;
+		  else if (sym_addr - sect_offset >=
+				       bfd_section_size (tmp_bfd, interp_sect))
+		    interp_sect == 0;
+		}
+	    }
+	  if (interp_sect != 0)
+	    {
+	      /* Try to convert the function descriptor we found above, into
+		 the address we need.  It will be relocated below by adding
+		 "load_addr" to it. */
+	      char *buf = alloca (sizeof (LONGEST));
+	      if (bfd_get_section_contents (tmp_bfd, interp_sect, buf,
+					    sym_addr - sect_offset,
+					    sizeof (LONGEST)))
+		sym_addr = extract_unsigned_integer (buf, sizeof (LONGEST));
+	      else
+		sym_addr = 0;
+	    }
+        }
+
       /* We're done with both the temporary bfd and target.  Remember,
          closing the target closes the underlying bfd.  */
       target_close (tmp_bfd_target, 0);

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

[PAUL GILLIAM]

Sorry, this got put in the wrong mail list thread.  Let me try again.
-=# Paul #=-