[RFA] fix psaddr_t fall-back definition for gdbserver

[RFA] fix psaddr_t fall-back definition for gdbserver

[Doug Evans]

On Fri, Dec 18, 2009 at 11:22 AM, Daniel Jacobowitz <drow@false.org> wrote:
> On Fri, Dec 18, 2009 at 10:39:45AM -0800, Doug Evans wrote:
>> Hi.
>>
>> While porting gdb 7.0 to android I needed to add this patch.
>>
>> Ok to check in?
>>
>> 2009-12-18  Doug Evans  <dje@google.com>
>>
>>       * gdb_proc_service.h (psaddr_t): Fix definition.
>
> Not quite.  This would diverge this copy of the header from the one in
> gdb/; which version is right?  Also, why did you need it?  If system
> headers don't declare psaddr_t it should not matter.

Blech, righto.
The duplication is just so wrong (YMMV :-)) that my mind pushes it out of
cache and I always forget to check the next time.

It turns out gdb hardcodes void* for some parameters to td_thr_tls_get_addr,
and it was causing compilation warnings (passing an int for a pointer, etc.)

From glibc/nptl_db/td_thr_tls_get_addr.c:

td_err_e
td_thr_tls_get_addr (const td_thrhandle_t *th,
		     psaddr_t map_address, size_t offset, psaddr_t *address)

From linux-thread-db.c:

   td_err_e (*td_thr_tls_get_addr_p) (const td_thrhandle_t *th,
				     void *map_address,
				     size_t offset, void **address);

It would be rather odd if psaddr_t were not a pointer in a different
linux system.
[I haven't been on a Solaris system in ages, and I gather from
sol-thread.c that psaddr_t is some int there, but gdb_proc_service
isn't used there.]

How about this?

2009-12-18  Doug Evans  <dje@google.com>

	gdb/
	* gdb_proc_service.h (psaddr_t): Fix type.
	* linux-thread-db.c (thread_db_info.td_thr_tls_get_addr_p): Fix
	signature to match glibc.
	(thread_db_get_thread_local_address): Use psaddr_t for type of
	address to match parameter of td_thr_tls_get_addr_p.
	Cast through psaddr_t instead of (void*) to match parameter of
	td_thr_tls_get_addr_p.

	gdbserver/
	* gdb_proc_service.h (psaddr_t): Fix type.
	* thread-db.c (thread_db_info.td_thr_tls_get_addr_p): Fix
	signature to match glibc.

Index: gdb_proc_service.h
===================================================================
RCS file: /cvs/src/src/gdb/gdb_proc_service.h,v
retrieving revision 1.10
diff -u -p -r1.10 gdb_proc_service.h
--- gdb_proc_service.h	27 Feb 2009 20:34:41 -0000	1.10
+++ gdb_proc_service.h	18 Dec 2009 21:30:44 -0000
@@ -47,7 +47,7 @@ typedef unsigned int lwpid_t;
 #endif
 
 #ifndef HAVE_PSADDR_T
-typedef unsigned long psaddr_t;
+typedef void *psaddr_t;
 #endif
 
 #ifndef HAVE_PRGREGSET_T
Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.69
diff -u -p -r1.69 linux-thread-db.c
--- linux-thread-db.c	3 Dec 2009 17:59:02 -0000	1.69
+++ linux-thread-db.c	18 Dec 2009 21:30:44 -0000
@@ -153,8 +153,8 @@ struct thread_db_info
 				     int event);
 
   td_err_e (*td_thr_tls_get_addr_p) (const td_thrhandle_t *th,
-				     void *map_address,
-				     size_t offset, void **address);
+				     psaddr_t map_address,
+				     size_t offset, psaddr_t *address);
 };
 
 /* List of known processes using thread_db, and the required
@@ -1530,7 +1530,7 @@ thread_db_get_thread_local_address (stru
   if (thread_info != NULL && thread_info->private != NULL)
     {
       td_err_e err;
-      void *address;
+      psaddr_t address;
       struct thread_db_info *info;
 
       info = get_thread_db_info (GET_PID (ptid));
@@ -1544,8 +1544,11 @@ thread_db_get_thread_local_address (stru
       gdb_assert (lm != 0);
 
       /* Finally, get the address of the variable.  */
+      /* Note the cast through uintptr_t: this interface only works if
+	 a target address fits in a psaddr_t, which is a host pointer.
+	 So a 32-bit debugger can not access 64-bit TLS through this.  */
       err = info->td_thr_tls_get_addr_p (&thread_info->private->th,
-					 (void *)(size_t) lm,
+					 (psaddr_t)(uintptr_t) lm,
 					 offset, &address);
 
 #ifdef THREAD_DB_HAS_TD_NOTALLOC
Index: gdbserver/gdb_proc_service.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/gdb_proc_service.h,v
retrieving revision 1.6
diff -u -p -r1.6 gdb_proc_service.h
--- gdbserver/gdb_proc_service.h	1 Apr 2009 22:50:24 -0000	1.6
+++ gdbserver/gdb_proc_service.h	18 Dec 2009 21:30:44 -0000
@@ -54,7 +54,7 @@ typedef unsigned int lwpid_t;
 #endif
 
 #ifndef HAVE_PSADDR_T
-typedef unsigned long psaddr_t;
+typedef void *psaddr_t;
 #endif
 
 #ifndef HAVE_PRGREGSET_T
Index: gdbserver/thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/thread-db.c,v
retrieving revision 1.29
diff -u -p -r1.29 thread-db.c
--- gdbserver/thread-db.c	16 Nov 2009 18:15:05 -0000	1.29
+++ gdbserver/thread-db.c	18 Dec 2009 21:30:44 -0000
@@ -71,8 +71,8 @@ struct thread_db
 				sigset_t *ti_sigmask_p,
 				unsigned int ti_user_flags);
   td_err_e (*td_thr_tls_get_addr_p) (const td_thrhandle_t *th,
-				     void *map_address,
-				     size_t offset, void **address);
+				     psaddr_t map_address,
+				     size_t offset, psaddr_t *address);
   const char ** (*td_symbol_list_p) (void);
 };
 

Re: [RFA] fix psaddr_t fall-back definition for gdbserver

[Daniel Jacobowitz]

On Fri, Dec 18, 2009 at 02:04:36PM -0800, Doug Evans wrote:
> How about this?
> 
> 2009-12-18  Doug Evans  <dje@google.com>
> 
> 	gdb/
> 	* gdb_proc_service.h (psaddr_t): Fix type.
> 	* linux-thread-db.c (thread_db_info.td_thr_tls_get_addr_p): Fix
> 	signature to match glibc.
> 	(thread_db_get_thread_local_address): Use psaddr_t for type of
> 	address to match parameter of td_thr_tls_get_addr_p.
> 	Cast through psaddr_t instead of (void*) to match parameter of
> 	td_thr_tls_get_addr_p.
> 
> 	gdbserver/
> 	* gdb_proc_service.h (psaddr_t): Fix type.
> 	* thread-db.c (thread_db_info.td_thr_tls_get_addr_p): Fix
> 	signature to match glibc.

Looks good.

-- 
Daniel Jacobowitz
CodeSourcery

[RFA] fix psaddr_t fall-back definition for gdbserver

[Doug Evans]

Hi.

While porting gdb 7.0 to android I needed to add this patch.

Ok to check in?

2009-12-18  Doug Evans  <dje@google.com>

	* gdb_proc_service.h (psaddr_t): Fix definition.

Index: gdb_proc_service.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/gdb_proc_service.h,v
retrieving revision 1.6
diff -u -p -r1.6 gdb_proc_service.h
--- gdb_proc_service.h	1 Apr 2009 22:50:24 -0000	1.6
+++ gdb_proc_service.h	18 Dec 2009 18:34:13 -0000
@@ -54,7 +54,7 @@ typedef unsigned int lwpid_t;
 #endif
 
 #ifndef HAVE_PSADDR_T
-typedef unsigned long psaddr_t;
+typedef void *psaddr_t;
 #endif
 
 #ifndef HAVE_PRGREGSET_T

Re: [RFA] fix psaddr_t fall-back definition for gdbserver

[Daniel Jacobowitz]

On Fri, Dec 18, 2009 at 10:39:45AM -0800, Doug Evans wrote:
> Hi.
> 
> While porting gdb 7.0 to android I needed to add this patch.
> 
> Ok to check in?
> 
> 2009-12-18  Doug Evans  <dje@google.com>
> 
> 	* gdb_proc_service.h (psaddr_t): Fix definition.

Not quite.  This would diverge this copy of the header from the one in
gdb/; which version is right?  Also, why did you need it?  If system
headers don't declare psaddr_t it should not matter.

-- 
Daniel Jacobowitz
CodeSourcery