[patch] Fix 64x32 handling in gdbserver/linux-x86-low.c:ps_get_thread_area

[patch] Fix 64x32 handling in gdbserver/linux-x86-low.c:ps_get_thread_area

[Doug Evans]

Hi.

I found this while testing gdbserver with 64x32
(64-bit gdb/gdbserver with the testcase compiled for 32-bit).
tls.exp has a number of bizarre failures.

E.g.,
Cannot find thread-local storage for Thread 16598, executable file /g5/local/ctools/gdb/trunk/build/obj64/gdb/testsuite/gdb.threads/tls:
and
gdbserver: PID mismatch!  Expected 16598, got 16596

I found the cause to be the storing of a 32 bit address in a 64 bit value,
leaving the high bits uninitialized.
One can see the bug with this patch.

diff thread-db.c
--- thread-db.c~ 2013-02-19 14:45:14.000000000 -0800
+++ thread-db.c 2013-05-24 17:20:14.450500768 -0700
@@ -40,6 +40,8 @@
 #include <dlfcn.h>
 #endif
 
+#include <stdio.h>
+#include <stdarg.h>
 #include <stdint.h>
 #include <limits.h>
 #include <ctype.h>
@@ -267,6 +269,22 @@
   return 1;
 }
 
+void
+plogf (const char *msg, ...)
+{
+  static FILE *f;
+  va_list args;
+  if (f == NULL)
+    {
+      f = fopen ("/tmp/gdbserver.log", "a");
+      fprintf (f, "... new run ...\n");
+    }
+  va_start (args, msg);
+  vfprintf (f, msg, args);
+  va_end (args);
+  fflush (f);
+}
+
 static int
 find_one_thread (ptid_t ptid)
 {
@@ -283,12 +301,18 @@
   if (lwp->thread_known)
     return 1;
 
+  memset (&th, -1, sizeof (th));
+
   /* Get information about this thread.  */
   err = thread_db->td_ta_map_lwp2thr_p (thread_db->thread_agent, lwpid, &th);
   if (err != TD_OK)
     error ("Cannot get thread handle for LWP %d: %s",
 	   lwpid, thread_db_err_str (err));
 
+  plogf ("lwpid %d\n", lwpid);
+  plogf ("th_ta_p %p\n", th.th_ta_p);
+  plogf ("th_unique %p\n", th.th_unique);
+
   err = thread_db->td_thr_get_info_p (&th, &ti);
   if (err != TD_OK)
     error ("Cannot get thread info for LWP %d: %s",
---

Testing tls.exp in 32-bit mode with 64-bit gdb/gdbserver, one sees this
in /tmp/gdbserver.log:

... new run ...
lwpid 16596
th_ta_p 0x17ce020
th_unique (nil)
lwpid 16598
th_ta_p 0x17ce020
th_unique 0xfffffffff7cb9b40
lwpid 16598
th_ta_p 0x17ce020
th_unique 0xfffffffff7cb9b40
...

I need to do a bit more testing, but this seems pretty obvious.
The bug doesn't exist in gdb, it already does the needed extension.

2013-05-24  Doug Evans  <dje@google.com>

	* linux-x86-low.c (ps_get_thread_area): Properly extend address to
	64 bits in 64-cross-32 environment.

Index: gdbserver/linux-x86-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-x86-low.c,v
retrieving revision 1.45
diff -u -p -r1.45 linux-x86-low.c
--- gdbserver/linux-x86-low.c	23 May 2013 17:17:50 -0000	1.45
+++ gdbserver/linux-x86-low.c	25 May 2013 01:06:49 -0000
@@ -196,7 +196,8 @@ ps_get_thread_area (const struct ps_proc
 		(void *) (intptr_t) idx, (unsigned long) &desc) < 0)
       return PS_ERR;
 
-    *(int *)base = desc[1];
+    /* Ensure we properly extend the value to 64-bits for x86_64.  */
+    *base = (void*) (uintptr_t) desc[1];
     return PS_OK;
   }
 }

Re: [patch] Fix 64x32 handling in gdbserver/linux-x86-low.c:ps_get_thread_area

[Pedro Alves]

Fun.  Thanks for the detailed analysis.

On 05/25/2013 02:25 AM, Doug Evans wrote:

> -    *(int *)base = desc[1];
> +    /* Ensure we properly extend the value to 64-bits for x86_64.  */
> +    *base = (void*) (uintptr_t) desc[1];

missing space: 'void *'.

Otherwise OK.

Thanks,
-- 
Pedro Alves

Re: [patch] Fix 64x32 handling in gdbserver/linux-x86-low.c:ps_get_thread_area

[Doug Evans]

On Sat, May 25, 2013 at 8:04 AM, Pedro Alves <palves@redhat.com> wrote:
> Fun.  Thanks for the detailed analysis.
>
> On 05/25/2013 02:25 AM, Doug Evans wrote:
>
>> -    *(int *)base = desc[1];
>> +    /* Ensure we properly extend the value to 64-bits for x86_64.  */
>> +    *base = (void*) (uintptr_t) desc[1];
>
> missing space: 'void *'.
>
> Otherwise OK.
>
> Thanks,
> --
> Pedro Alves

Patch committed to 7.6 branch as well (and 7.6 wiki updated).

Re: [patch] Fix 64x32 handling in gdbserver/linux-x86-low.c:ps_get_thread_area

[Joel Brobecker]

> Patch committed to 7.6 branch as well (and 7.6 wiki updated).

Thanks Doug. Is there a PR for this? I think it's useful to
have a PR, because I will provide that information in the release
announcement, and that gives people somewhere to go in order to
know more about any specific item.

-- 
Joel

Re: [patch] Fix 64x32 handling in gdbserver/linux-x86-low.c:ps_get_thread_area

[Doug Evans]

On Wed, Jun 5, 2013 at 10:20 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Patch committed to 7.6 branch as well (and 7.6 wiki updated).
>
> Thanks Doug. Is there a PR for this? I think it's useful to
> have a PR, because I will provide that information in the release
> announcement, and that gives people somewhere to go in order to
> know more about any specific item.

As you wish.