deal thread id as unsigned

deal thread id as unsigned

[Yoriko Komatsuzaki]

Hello.

I would like to ask you to consider this tiny patch about thread id type.
Thread id is unsigned type in Linux.

Without this patch (especially about linux-thread-db.c), thread id
display can be minus...

# Apologies in advance if this is already-known issue.

Thank you.

--- gdb-6.6/gdb/defs.org.h	2007-04-10 16:04:51.000000000 +0900
+++ gdb-6.6/gdb/defs.h	2007-04-10 16:05:06.000000000 +0900
@@ -795,7 +795,7 @@ struct ptid
     long lwp;
 
     /* Thread id */
-    long tid;
+    unsigned long tid;
   };
 
 typedef struct ptid ptid_t;
--- gdb-6.6/gdb/linux-thread-db.org.c	2007-04-10 16:55:55.000000000 +0900
+++ gdb-6.6/gdb/linux-thread-db.c	2007-04-10 16:53:30.000000000 +0900
@@ -1031,10 +1031,10 @@ thread_db_pid_to_str (ptid_t ptid)
 
       thread_info = find_thread_pid (ptid);
       if (thread_info == NULL)
-	snprintf (buf, sizeof (buf), "Thread %ld (LWP %ld) (Missing)",
+	snprintf (buf, sizeof (buf), "Thread %lu (LWP %ld) (Missing)",
 		  GET_THREAD (ptid), GET_LWP (ptid));
       else
-	snprintf (buf, sizeof (buf), "Thread %ld (LWP %ld)",
+	snprintf (buf, sizeof (buf), "Thread %lu (LWP %ld)",
 		  GET_THREAD (ptid), GET_LWP (ptid));
 
       return buf;

---
Yoriko Komatsuzaki (yoriko@sm.sony.co.jp)

Re: deal thread id as unsigned

[Mark Kettenis]

> Date: Mon, 16 Apr 2007 11:39:55 +0900
> From: Yoriko Komatsuzaki <yoriko@sm.sony.co.jp>
> 
> Hello.
> 
> I would like to ask you to consider this tiny patch about thread id type.
> Thread id is unsigned type in Linux.

I believe it used to be a signed type in Linux.  Anyway, you can't
really change it, since on other OS'es the type defenitely is signed.

You'll only really notice this when the msb is set anyway.  I think
this happens on the Linux threads implementation of today the thread
id isn't an integer at all, but a pointer instead.  This suggests that
it should really be printed as an (unsigned) hexadecimal number.
Doing so doesn't need any changes to defs.h and therefore wouldn't
affect any other platforms at all.

Mark

Re: deal thread id as unsigned

[Daniel Jacobowitz]

On Mon, Apr 16, 2007 at 04:10:27PM +0200, Mark Kettenis wrote:
> > Date: Mon, 16 Apr 2007 11:39:55 +0900
> > From: Yoriko Komatsuzaki <yoriko@sm.sony.co.jp>
> > 
> > Hello.
> > 
> > I would like to ask you to consider this tiny patch about thread id type.
> > Thread id is unsigned type in Linux.
> 
> I believe it used to be a signed type in Linux.  Anyway, you can't
> really change it, since on other OS'es the type defenitely is signed.

Well, it used to be an int, but it would never be negative (always
small and positive).

> You'll only really notice this when the msb is set anyway.  I think
> this happens on the Linux threads implementation of today the thread
> id isn't an integer at all, but a pointer instead.  This suggests that
> it should really be printed as an (unsigned) hexadecimal number.
> Doing so doesn't need any changes to defs.h and therefore wouldn't
> affect any other platforms at all.

Right.  I think printing it as hex is fine even for LinuxThreads,
where 0x4000 is not substantially less clear than "16384" (which was
always the thread ID of the first thread, no relation to PIDs).

-- 
Daniel Jacobowitz
CodeSourcery

Re: deal thread id as unsigned

[Yoriko Komatsuzaki]

Thank you for your reply and suggestion.

I get my mind around no modification of defs.h because of the care for other
os'es. 

Concerning to thread id printing, I would appreciate it if you could
consider to print it by hexadecimal notation.

Thank you.

--- 
Yoriko Komatsuzaki 
yoriko@sm.sony.co.jp

>> This suggests that
>> it should really be printed as an (unsigned) hexadecimal number.
>> Doing so doesn't need any changes to defs.h and therefore wouldn't
>> affect any other platforms at all.

> On Mon, Apr 16, 2007 at 04:10:27PM +0200, Mark Kettenis wrote:
> > > Date: Mon, 16 Apr 2007 11:39:55 +0900
> > > From: Yoriko Komatsuzaki <yoriko@sm.sony.co.jp>
> > > 
> > > Hello.
> > > 
> > > I would like to ask you to consider this tiny patch about thread id type.
> > > Thread id is unsigned type in Linux.
> > 
> > I believe it used to be a signed type in Linux.  Anyway, you can't
> > really change it, since on other OS'es the type defenitely is signed.
> 
> Well, it used to be an int, but it would never be negative (always
> small and positive).
> 
> > You'll only really notice this when the msb is set anyway.  I think
> > this happens on the Linux threads implementation of today the thread
> > id isn't an integer at all, but a pointer instead.  This suggests that
> > it should really be printed as an (unsigned) hexadecimal number.
> > Doing so doesn't need any changes to defs.h and therefore wouldn't
> > affect any other platforms at all.
> 
> Right.  I think printing it as hex is fine even for LinuxThreads,
> where 0x4000 is not substantially less clear than "16384" (which was
> always the thread ID of the first thread, no relation to PIDs).
> 
> -- 
> Daniel Jacobowitz
> CodeSourcery

Re: deal thread id as unsigned

[Daniel Jacobowitz]

On Tue, Apr 17, 2007 at 11:01:43AM +0900, Yoriko Komatsuzaki wrote:
> 
> Thank you for your reply and suggestion.
> 
> I get my mind around no modification of defs.h because of the care for other
> os'es. 
> 
> Concerning to thread id printing, I would appreciate it if you could
> consider to print it by hexadecimal notation.

Yes, this is a good idea.  Sorry for the delay; I checked in this
patch, to change the output to hexadecimal.  Tested on x86_64-linux.

I've just realized that the test change won't work on remote
debugging; I'll fix up linux-dp.exp.

-- 
Daniel Jacobowitz
CodeSourcery

2007-05-14  Daniel Jacobowitz  <dan@codesourcery.com>

	* linux-thread-db.c (thread_db_pid_to_str): Print thread IDs
	as hexadecimal.

	* gdb.threads/linux-dp.exp: Expect hexadecimal thread IDs.
	* gdb.threads/print-threads.exp (test_all_threads): Allow
	negative and hexadecimal thread IDs.

Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.29
diff -u -p -r1.29 linux-thread-db.c
--- linux-thread-db.c	11 May 2007 19:55:19 -0000	1.29
+++ linux-thread-db.c	14 May 2007 16:49:49 -0000
@@ -1013,10 +1013,10 @@ thread_db_pid_to_str (ptid_t ptid)
 
       thread_info = find_thread_pid (ptid);
       if (thread_info == NULL)
-	snprintf (buf, sizeof (buf), "Thread %ld (LWP %ld) (Missing)",
+	snprintf (buf, sizeof (buf), "Thread 0x%lx (LWP %ld) (Missing)",
 		  GET_THREAD (ptid), GET_LWP (ptid));
       else
-	snprintf (buf, sizeof (buf), "Thread %ld (LWP %ld)",
+	snprintf (buf, sizeof (buf), "Thread 0x%lx (LWP %ld)",
 		  GET_THREAD (ptid), GET_LWP (ptid));
 
       return buf;
Index: testsuite/gdb.threads/linux-dp.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/linux-dp.exp,v
retrieving revision 1.15
diff -u -p -r1.15 linux-dp.exp
--- testsuite/gdb.threads/linux-dp.exp	9 Jan 2007 17:59:14 -0000	1.15
+++ testsuite/gdb.threads/linux-dp.exp	14 May 2007 16:49:50 -0000
@@ -74,7 +74,7 @@ for {set i 0} {$i < 5} {incr i} {
 	-re "info threads\r\n" {
 	    exp_continue
 	}
-	-re "^. +(\[0-9\]+ Thread \[-0-9\]+) \[^\n\]*\n" {
+	-re "^. +(\[0-9\]+ Thread 0x\[0-9a-f\]+) \[^\n\]*\n" {
 	    verbose -log "found thread $expect_out(1,string)" 2
 	    lappend threads_before $expect_out(1,string)
 	    exp_continue
@@ -147,7 +147,7 @@ for {set i 0} {$i < 5} {incr i} {
 	-re "info threads\r\n" {
 	    exp_continue
 	}
-	-re "^. +(\[0-9\]+ Thread \[-0-9\]+) \[^\n\]*\n" {
+	-re "^. +(\[0-9\]+ Thread 0x\[0-9a-f\]+) \[^\n\]*\n" {
 	    set name $expect_out(1,string)
 	    for {set j 0} {$j != [llength $threads_before] } {incr j} {
 		if {$name == [lindex $threads_before $j]} {
Index: testsuite/gdb.threads/print-threads.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/print-threads.exp,v
retrieving revision 1.7
diff -u -p -r1.7 print-threads.exp
--- testsuite/gdb.threads/print-threads.exp	9 Jan 2007 17:59:14 -0000	1.7
+++ testsuite/gdb.threads/print-threads.exp	14 May 2007 16:49:50 -0000
@@ -94,7 +94,7 @@ proc test_all_threads { name kill } {
 		fail "all threads ran once ($name) (total $i threads ran)"
 	    }
 	}
-	-re "Program received signal SIGTRAP.*(Thread \[0-9\]* \\(zombie\\)|0x00000000 in ).*$gdb_prompt $" {
+	-re "Program received signal SIGTRAP.*(Thread \[-0-9a-fx\]* \\(zombie\\)|0x00000000 in ).*$gdb_prompt $" {
 	    if { $kill == 1 } {
 		kfail "gdb/1265" "Running threads ($name) (zombie thread)"
 	    } else {

Re: deal thread id as unsigned

[Daniel Jacobowitz]

On Mon, May 14, 2007 at 12:53:33PM -0400, Daniel Jacobowitz wrote:
> On Tue, Apr 17, 2007 at 11:01:43AM +0900, Yoriko Komatsuzaki wrote:
> > 
> > Thank you for your reply and suggestion.
> > 
> > I get my mind around no modification of defs.h because of the care for other
> > os'es. 
> > 
> > Concerning to thread id printing, I would appreciate it if you could
> > consider to print it by hexadecimal notation.
> 
> Yes, this is a good idea.  Sorry for the delay; I checked in this
> patch, to change the output to hexadecimal.  Tested on x86_64-linux.
> 
> I've just realized that the test change won't work on remote
> debugging; I'll fix up linux-dp.exp.

Like so.  gdbserver reports the LWP ID (PID) as the thread ID, so
decimal is appropriate.

-- 
Daniel Jacobowitz
CodeSourcery

2007-05-14  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.threads/linux-dp.exp: Allow non-hexadecimal IDs also.

Index: testsuite/gdb.threads/linux-dp.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/linux-dp.exp,v
retrieving revision 1.16
diff -u -p -r1.16 linux-dp.exp
--- testsuite/gdb.threads/linux-dp.exp	14 May 2007 16:52:13 -0000	1.16
+++ testsuite/gdb.threads/linux-dp.exp	14 May 2007 16:55:29 -0000
@@ -74,7 +74,7 @@ for {set i 0} {$i < 5} {incr i} {
 	-re "info threads\r\n" {
 	    exp_continue
 	}
-	-re "^. +(\[0-9\]+ Thread 0x\[0-9a-f\]+) \[^\n\]*\n" {
+	-re "^. +(\[0-9\]+ Thread \[-0-9a-fx\]+) \[^\n\]*\n" {
 	    verbose -log "found thread $expect_out(1,string)" 2
 	    lappend threads_before $expect_out(1,string)
 	    exp_continue
@@ -147,7 +147,7 @@ for {set i 0} {$i < 5} {incr i} {
 	-re "info threads\r\n" {
 	    exp_continue
 	}
-	-re "^. +(\[0-9\]+ Thread 0x\[0-9a-f\]+) \[^\n\]*\n" {
+	-re "^. +(\[0-9\]+ Thread \[-0-9a-fx\]+) \[^\n\]*\n" {
 	    set name $expect_out(1,string)
 	    for {set j 0} {$j != [llength $threads_before] } {incr j} {
 		if {$name == [lindex $threads_before $j]} {