Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag
@ 2012-04-03 20:48 Maciej W. Rozycki
  2012-04-03 21:27 ` Pedro Alves
  2012-04-04 19:16 ` Jan Kratochvil
  0 siblings, 2 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2012-04-03 20:48 UTC (permalink / raw)
  To: gdb-patches

Hi,

 While working on a test case for an unrelated issue I have noticed 
single-stepping over system library calls is broken on the MIPS/Linux 
target when `gdbserver' is used.  On closer inspection I have observed 
`gdbserver' reports no shared libraries loaded.  Further investigation has 
revealed that the newly-added support for the `qXfer:libraries-svr4:read' 
packet does not handle the DT_MIPS_RLD_MAP dynamic tag and therefore 
cannot locate the link map on the MIPS target.

 Such support has been long present in gdb/solib-svr4.c, hence a fix 
turned out rather mechanical; here's a piece of code ported from there 
over to linux-low.c, likewise making no special exception just for the 
MIPS platform and applying to generic code instead.  This change has fixed 
the single-stepping problem observed for me.

 Just to be safe I have regression-tested this change for the 
mips-linux-gnu remote target, o32/big-endian multilib (using an x86 Linux 
host) with 139 failures removed starting from:

(gdb) PASS: gdb.base/break.exp: backtrace from factorial(5.1)
break exit
Function "exit" not defined.
(gdb) FAIL: gdb.base/break.exp: setting breakpoint at exit

and no new ones.  I didn't test the change for any other target, but then 
DT_MIPS_RLD_MAP tags are not expected to appear on non-MIPS binaries, 
hence the "dyn->d_tag == DT_MIPS_RLD_MAP" conditional is not expected to 
trigger there.

 OK to apply?

2012-04-03  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/gdbserver/
	* linux-low.c (get_r_debug): Handle DT_MIPS_RLD_MAP.

  Maciej

gdb-gdbserver-rld-map.diff
Index: gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/gdbserver/linux-low.c	2012-04-03 13:38:58.575561428 +0100
+++ gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c	2012-04-03 13:39:17.315624148 +0100
@@ -5419,7 +5419,9 @@ get_dynamic (const int pid, const int is
 }
 
 /* Return &_r_debug in the inferior, or -1 if not present.  Return value
-   can be 0 if the inferior does not yet have the library list initialized.  */
+   can be 0 if the inferior does not yet have the library list initialized.
+   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
+   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
 
 static CORE_ADDR
 get_r_debug (const int pid, const int is_elf64)
@@ -5437,6 +5439,21 @@ get_r_debug (const int pid, const int is
       if (is_elf64)
 	{
 	  Elf64_Dyn *const dyn = (Elf64_Dyn *) buf;
+	  union
+	    {
+	      Elf64_Xword map;
+	      unsigned char buf[sizeof (Elf64_Xword)];
+	    }
+	  rld_map;
+
+	  if (dyn->d_tag == DT_MIPS_RLD_MAP)
+	    {
+	      if (linux_read_memory (dyn->d_un.d_val,
+				     rld_map.buf, sizeof (rld_map.buf)) == 0)
+		return rld_map.map;
+	      else
+		break;
+	    }
 
 	  if (dyn->d_tag == DT_DEBUG)
 	    return dyn->d_un.d_val;
@@ -5447,6 +5464,21 @@ get_r_debug (const int pid, const int is
       else
 	{
 	  Elf32_Dyn *const dyn = (Elf32_Dyn *) buf;
+	  union
+	    {
+	      Elf32_Word map;
+	      unsigned char buf[sizeof (Elf32_Word)];
+	    }
+	  rld_map;
+
+	  if (dyn->d_tag == DT_MIPS_RLD_MAP)
+	    {
+	      if (linux_read_memory (dyn->d_un.d_val,
+				     rld_map.buf, sizeof (rld_map.buf)) == 0)
+		return rld_map.map;
+	      else
+		break;
+	    }
 
 	  if (dyn->d_tag == DT_DEBUG)
 	    return dyn->d_un.d_val;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag
  2012-04-03 20:48 [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag Maciej W. Rozycki
@ 2012-04-03 21:27 ` Pedro Alves
  2012-04-04 11:37   ` Maciej W. Rozycki
  2012-04-04 19:16 ` Jan Kratochvil
  1 sibling, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-04-03 21:27 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches

Hi Maciej,

On 04/03/2012 09:47 PM, Maciej W. Rozycki wrote:

>  While working on a test case for an unrelated issue I have noticed 
> single-stepping over system library calls is broken on the MIPS/Linux 
> target when `gdbserver' is used.  On closer inspection I have observed 
> `gdbserver' reports no shared libraries loaded.  Further investigation has 
> revealed that the newly-added support for the `qXfer:libraries-svr4:read' 
> packet does not handle the DT_MIPS_RLD_MAP dynamic tag and therefore 
> cannot locate the link map on the MIPS target.


Yeah, I had noticed that on
<http://sourceware.org/ml/gdb-patches/2012-03/msg00142.html>, but never got
to propose a fix.  Sorry about that.


> gdb-gdbserver-rld-map.diff

> Index: gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c
> ===================================================================
> --- gdb-fsf-trunk-quilt.orig/gdb/gdbserver/linux-low.c	2012-04-03 13:38:58.575561428 +0100
> +++ gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c	2012-04-03 13:39:17.315624148 +0100
> @@ -5419,7 +5419,9 @@ get_dynamic (const int pid, const int is
>  }
>  
>  /* Return &_r_debug in the inferior, or -1 if not present.  Return value
> -   can be 0 if the inferior does not yet have the library list initialized.  */
> +   can be 0 if the inferior does not yet have the library list initialized.
> +   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
> +   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */

>

Can that "unused DT_DEBUG" entry ever appear before DT_MIPS_RLD_MAP in the tags
list?  With this patch, we'll stop looking if we see DT_DEBUG first, while
solib-svr4.c in GDB first looks for DT_MIPS_RLD_MAP in all tags, and only
if not found looks for DT_DEBUG (presumably, if there's no DT_MIPS_RLD_MAP,
then there's no DT_DEBUG either; only if DT_MIPS_RLD_MAP exist might there be
a DT_DEBUG).  Do we need to worry about this?

>  static CORE_ADDR
>  get_r_debug (const int pid, const int is_elf64)
> @@ -5437,6 +5439,21 @@ get_r_debug (const int pid, const int is
>        if (is_elf64)
>  	{
>  	  Elf64_Dyn *const dyn = (Elf64_Dyn *) buf;
> +	  union
> +	    {
> +	      Elf64_Xword map;
> +	      unsigned char buf[sizeof (Elf64_Xword)];
> +	    }
> +	  rld_map;
> +
> +	  if (dyn->d_tag == DT_MIPS_RLD_MAP)
> +	    {
> +	      if (linux_read_memory (dyn->d_un.d_val,
> +				     rld_map.buf, sizeof (rld_map.buf)) == 0)
> +		return rld_map.map;
> +	      else
> +		break;
> +	    }
>  
>  	  if (dyn->d_tag == DT_DEBUG)
>  	    return dyn->d_un.d_val;
> @@ -5447,6 +5464,21 @@ get_r_debug (const int pid, const int is
>        else
>  	{
>  	  Elf32_Dyn *const dyn = (Elf32_Dyn *) buf;
> +	  union
> +	    {
> +	      Elf32_Word map;
> +	      unsigned char buf[sizeof (Elf32_Word)];
> +	    }
> +	  rld_map;
> +
> +	  if (dyn->d_tag == DT_MIPS_RLD_MAP)
> +	    {
> +	      if (linux_read_memory (dyn->d_un.d_val,
> +				     rld_map.buf, sizeof (rld_map.buf)) == 0)
> +		return rld_map.map;
> +	      else
> +		break;
> +	    }
>  
>  	  if (dyn->d_tag == DT_DEBUG)
>  	    return dyn->d_un.d_val;



-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag
  2012-04-03 21:27 ` Pedro Alves
@ 2012-04-04 11:37   ` Maciej W. Rozycki
  2012-04-04 11:55     ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2012-04-04 11:37 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Hi Pedro,

> Yeah, I had noticed that on
> <http://sourceware.org/ml/gdb-patches/2012-03/msg00142.html>, but never got
> to propose a fix.  Sorry about that.

 Well, I'm supposed to catch this kind of breakage myself these days. ;)

> > Index: gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c
> > ===================================================================
> > --- gdb-fsf-trunk-quilt.orig/gdb/gdbserver/linux-low.c	2012-04-03 13:38:58.575561428 +0100
> > +++ gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c	2012-04-03 13:39:17.315624148 +0100
> > @@ -5419,7 +5419,9 @@ get_dynamic (const int pid, const int is
> >  }
> >  
> >  /* Return &_r_debug in the inferior, or -1 if not present.  Return value
> > -   can be 0 if the inferior does not yet have the library list initialized.  */
> > +   can be 0 if the inferior does not yet have the library list initialized.
> > +   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
> > +   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
> 
> Can that "unused DT_DEBUG" entry ever appear before DT_MIPS_RLD_MAP in the tags
> list?  With this patch, we'll stop looking if we see DT_DEBUG first, while
> solib-svr4.c in GDB first looks for DT_MIPS_RLD_MAP in all tags, and only
> if not found looks for DT_DEBUG (presumably, if there's no DT_MIPS_RLD_MAP,
> then there's no DT_DEBUG either; only if DT_MIPS_RLD_MAP exist might there be
> a DT_DEBUG).  Do we need to worry about this?

 Hmm, I've double-checked the sources and as you say solib-svr4.c does not 
rely on the order of the tags indeed.  I got mislead by the comment in 
MIPS BFD that says:

      /* SGI object has the equivalence of DT_DEBUG in the
         DT_MIPS_RLD_MAP entry.  This must come first because glibc
         only fills in DT_MIPS_RLD_MAP (not DT_DEBUG) and GDB only
         looks at the first one it sees.  */

and which obviously does not stand (anymore?) as far as GDB is concerned.  
I'll post a proposal to update it to match reality.  It is worth noting 
that current MIPS BFD always adds DT_DEBUG too (and always after 
DT_MIPS_RLD_MAP, the comment gets this part right).

 Anyway, I think `gdbserver' should be as liberal as to what it accepts as 
GDB itself is, so I have updated my change as below.  No changes in 
regression testing.

 Thanks for your input.  Any further comments?

2012-04-03  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/gdbserver/
	* linux-low.c (get_r_debug): Handle DT_MIPS_RLD_MAP.

  Maciej

gdb-gdbserver-rld-map.diff
Index: gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/gdbserver/linux-low.c	2012-04-03 16:49:32.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c	2012-04-04 11:56:37.405566578 +0100
@@ -5419,7 +5419,9 @@ get_dynamic (const int pid, const int is
 }
 
 /* Return &_r_debug in the inferior, or -1 if not present.  Return value
-   can be 0 if the inferior does not yet have the library list initialized.  */
+   can be 0 if the inferior does not yet have the library list initialized.
+   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
+   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
 
 static CORE_ADDR
 get_r_debug (const int pid, const int is_elf64)
@@ -5427,19 +5429,35 @@ get_r_debug (const int pid, const int is
   CORE_ADDR dynamic_memaddr;
   const int dyn_size = is_elf64 ? sizeof (Elf64_Dyn) : sizeof (Elf32_Dyn);
   unsigned char buf[sizeof (Elf64_Dyn)];  /* The larger of the two.  */
+  CORE_ADDR map = -1;
 
   dynamic_memaddr = get_dynamic (pid, is_elf64);
   if (dynamic_memaddr == 0)
-    return (CORE_ADDR) -1;
+    return map;
 
   while (linux_read_memory (dynamic_memaddr, buf, dyn_size) == 0)
     {
       if (is_elf64)
 	{
 	  Elf64_Dyn *const dyn = (Elf64_Dyn *) buf;
+	  union
+	    {
+	      Elf64_Xword map;
+	      unsigned char buf[sizeof (Elf64_Xword)];
+	    }
+	  rld_map;
 
-	  if (dyn->d_tag == DT_DEBUG)
-	    return dyn->d_un.d_val;
+	  if (dyn->d_tag == DT_MIPS_RLD_MAP)
+	    {
+	      if (linux_read_memory (dyn->d_un.d_val,
+				     rld_map.buf, sizeof (rld_map.buf)) == 0)
+		return rld_map.map;
+	      else
+		break;
+	    }
+
+	  if (dyn->d_tag == DT_DEBUG && map == -1)
+	    map = dyn->d_un.d_val;
 
 	  if (dyn->d_tag == DT_NULL)
 	    break;
@@ -5447,9 +5465,24 @@ get_r_debug (const int pid, const int is
       else
 	{
 	  Elf32_Dyn *const dyn = (Elf32_Dyn *) buf;
+	  union
+	    {
+	      Elf32_Word map;
+	      unsigned char buf[sizeof (Elf32_Word)];
+	    }
+	  rld_map;
 
-	  if (dyn->d_tag == DT_DEBUG)
-	    return dyn->d_un.d_val;
+	  if (dyn->d_tag == DT_MIPS_RLD_MAP)
+	    {
+	      if (linux_read_memory (dyn->d_un.d_val,
+				     rld_map.buf, sizeof (rld_map.buf)) == 0)
+		return rld_map.map;
+	      else
+		break;
+	    }
+
+	  if (dyn->d_tag == DT_DEBUG && map == -1)
+	    map = dyn->d_un.d_val;
 
 	  if (dyn->d_tag == DT_NULL)
 	    break;
@@ -5458,7 +5491,7 @@ get_r_debug (const int pid, const int is
       dynamic_memaddr += dyn_size;
     }
 
-  return (CORE_ADDR) -1;
+  return map;
 }
 
 /* Read one pointer from MEMADDR in the inferior.  */


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag
  2012-04-04 11:37   ` Maciej W. Rozycki
@ 2012-04-04 11:55     ` Pedro Alves
  2012-04-10 22:54       ` Maciej W. Rozycki
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-04-04 11:55 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches

On 04/04/2012 12:36 PM, Maciej W. Rozycki wrote:

>  Anyway, I think `gdbserver' should be as liberal as to what it accepts as
> GDB itself is, so I have updated my change as below.  No changes in
> regression testing.

> 
>  Thanks for your input.  Any further comments?


Nope.  Looks good to me.

Thanks,
-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag
  2012-04-03 20:48 [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag Maciej W. Rozycki
  2012-04-03 21:27 ` Pedro Alves
@ 2012-04-04 19:16 ` Jan Kratochvil
  1 sibling, 0 replies; 6+ messages in thread
From: Jan Kratochvil @ 2012-04-04 19:16 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches

On Tue, 03 Apr 2012 22:47:28 +0200, Maciej W. Rozycki wrote:
> 	gdb/gdbserver/
> 	* linux-low.c (get_r_debug): Handle DT_MIPS_RLD_MAP.

Thanks for catching it, I only recently got MIPS access for it but did not yet
get to it.


Jan


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag
  2012-04-04 11:55     ` Pedro Alves
@ 2012-04-10 22:54       ` Maciej W. Rozycki
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2012-04-10 22:54 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Wed, 4 Apr 2012, Pedro Alves wrote:

> Nope.  Looks good to me.

 Applied now, thanks for your review.

  Maciej


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-04-10 22:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-03 20:48 [PATCH] gdbserver: Handle DT_MIPS_RLD_MAP dynamic tag Maciej W. Rozycki
2012-04-03 21:27 ` Pedro Alves
2012-04-04 11:37   ` Maciej W. Rozycki
2012-04-04 11:55     ` Pedro Alves
2012-04-10 22:54       ` Maciej W. Rozycki
2012-04-04 19:16 ` Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox