RE: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

RE: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Jitendra Pawar]

Hi,

>On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
>> The strtoul change in my patch was already present. Sorry about that.
>> 
>> You have changed the data type of thread_resume::thread as well as cont_thread 
>> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
>> place. How does this work?
>
>Hmm, looks like you found a real problem here; would you like to fix
>it?  Modern GDBs don't heavily use this code, because of the vCont
>packet.  Otherwise I'll fix it, but I may not find time for a bit.
>

I have understood the problem and I'll like to fix it. shall I do it?

- Jitendra

>> Can we use a thread value of "0" to indicate all threads or no-thread instead 
>> of "-1"? The condition "cont_thread > 0" becomes valid if we do that.
>
>This value can be set from the remote protocol, so we would have to
>transform it in Hc/Hg support also.

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Wed, Mar 16, 2005 at 10:19:18PM +0530, Jitendra Pawar wrote:
> Hi,
> 
> >On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
> >> The strtoul change in my patch was already present. Sorry about that.
> >> 
> >> You have changed the data type of thread_resume::thread as well as cont_thread 
> >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> >> place. How does this work?
> >
> >Hmm, looks like you found a real problem here; would you like to fix
> >it?  Modern GDBs don't heavily use this code, because of the vCont
> >packet.  Otherwise I'll fix it, but I may not find time for a bit.
> >
> 
> I have understood the problem and I'll like to fix it. shall I do it?

Have you got an FSF copyright assignment on file?  It doesn't look like
it.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Jitendra Pawar]

 
> > >On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
> > >> The strtoul change in my patch was already present. Sorry about that.
> > >> 
> > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > >> place. How does this work?

There are about 15 files in gdb source have statement pid_to_ptid (-1);
which finally returns -1 to server. Is it OK to replace  -1 with 0 ? I
would like to know significance of returning pid -1, 0 and positive
integer.

- Jitendra

> > >
> > >Hmm, looks like you found a real problem here; would you like to fix
> > >it?  Modern GDBs don't heavily use this code, because of the vCont
> > >packet.  Otherwise I'll fix it, but I may not find time for a bit.
> > >

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Tue, Mar 22, 2005 at 07:35:24PM +0530, Jitendra Pawar wrote:
>  
> > > >On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
> > > >> The strtoul change in my patch was already present. Sorry about that.
> > > >> 
> > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > > >> place. How does this work?
> 
> There are about 15 files in gdb source have statement pid_to_ptid (-1);
> which finally returns -1 to server. Is it OK to replace  -1 with 0 ? I
> would like to know significance of returning pid -1, 0 and positive
> integer.

Amit's original issue is in gdbserver; how do any of the invalid GDB
pids get sent over the wire to gdbserver?  That shouldn't happen.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Jitendra Pawar]

On Tue, 2005-03-22 at 09:23 -0500, Daniel Jacobowitz wrote:
> On Tue, Mar 22, 2005 at 07:35:24PM +0530, Jitendra Pawar wrote:
> >  
> > > > >On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
> > > > >> The strtoul change in my patch was already present. Sorry about that.
> > > > >> 
> > > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > > > >> place. How does this work?
> > 
> > There are about 15 files in gdb source have statement pid_to_ptid (-1);
> > which finally returns -1 to server. Is it OK to replace  -1 with 0 ? I
> > would like to know significance of returning pid -1, 0 and positive
> > integer.
> 
> Amit's original issue is in gdbserver; how do any of the invalid GDB
> pids get sent over the wire to gdbserver?  That shouldn't happen.
> 

So can I say that GDB never written pid with -1 value to gdbserver?  OR
do I need to find only those statements form GDB that returns -1 to
gdbserver? If this is the case then whether replacing those '-1' with
'0' will be the solution for this?  
 

- Jitendra

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Tue, Mar 22, 2005 at 08:14:09PM +0530, Jitendra Pawar wrote:
> On Tue, 2005-03-22 at 09:23 -0500, Daniel Jacobowitz wrote:
> > On Tue, Mar 22, 2005 at 07:35:24PM +0530, Jitendra Pawar wrote:
> > >  
> > > > > >On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
> > > > > >> The strtoul change in my patch was already present. Sorry about that.
> > > > > >> 
> > > > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > > > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > > > > >> place. How does this work?
> > > 
> > > There are about 15 files in gdb source have statement pid_to_ptid (-1);
> > > which finally returns -1 to server. Is it OK to replace  -1 with 0 ? I
> > > would like to know significance of returning pid -1, 0 and positive
> > > integer.
> > 
> > Amit's original issue is in gdbserver; how do any of the invalid GDB
> > pids get sent over the wire to gdbserver?  That shouldn't happen.
> > 
> 
> So can I say that GDB never written pid with -1 value to gdbserver?  OR
> do I need to find only those statements form GDB that returns -1 to
> gdbserver? If this is the case then whether replacing those '-1' with
> '0' will be the solution for this?  

I don't know.  But you're barking up the wrong tree; whatever you need
to change, you should be doing it only within gdbserver.  If you change
GDB to fix a problem in gdbserver, you're changing the remote protocol.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Jitendra Pawar]

> > > > > > >> The strtoul change in my patch was already present. Sorry about that.
> > > > > > >> 
> > > > > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > > > > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > > > > > >> place. How does this work?
> > > > 
> > > > There are about 15 files in gdb source have statement pid_to_ptid (-1);
> > > > which finally returns -1 to server. Is it OK to replace  -1 with 0 ? I
> > > > would like to know significance of returning pid -1, 0 and positive
> > > > integer.
> > > 
> whatever you need
> to change, you should be doing it only within gdbserver.  If you change
> GDB to fix a problem in gdbserver, you're changing the remote protocol.

Understood thanks. I believe that changing type of thread IDs to 'long
long' in gdbserver code will work. In that case I need to update all
places as you did it for changing type of thread IDs to 'unsigned long'
few days back. Is it the fix for this?

- Jitendra 

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Wed, Mar 23, 2005 at 03:27:09PM +0530, Jitendra Pawar wrote:
> > > > > > > >> The strtoul change in my patch was already present. Sorry about that.
> > > > > > > >> 
> > > > > > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > > > > > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > > > > > > >> place. How does this work?
> > > > > 
> > > > > There are about 15 files in gdb source have statement pid_to_ptid (-1);
> > > > > which finally returns -1 to server. Is it OK to replace  -1 with 0 ? I
> > > > > would like to know significance of returning pid -1, 0 and positive
> > > > > integer.
> > > > 
> > whatever you need
> > to change, you should be doing it only within gdbserver.  If you change
> > GDB to fix a problem in gdbserver, you're changing the remote protocol.
> 
> Understood thanks. I believe that changing type of thread IDs to 'long
> long' in gdbserver code will work. In that case I need to update all
> places as you did it for changing type of thread IDs to 'unsigned long'
> few days back. Is it the fix for this?

Why don't you explain exactly what you think is the problem, and
exactly why you need to double the size of thread IDs to fix it?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Jitendra Pawar]

> 
> > Understood thanks. I believe that changing type of thread IDs to 'long
> > long' in gdbserver code will work. In that case I need to update all
> > places as you did it for changing type of thread IDs to 'unsigned long'
> > few days back. Is it the fix for this?
> 
> Why don't you explain exactly what you think is the problem, and
> exactly why you need to double the size of thread IDs to fix it?
> 

Sorry It was my mistake, I wanted to say 'long int' instead of 'long
long '.  and size of long int and unsigned int is same i.e. 64-bit.

You have commited the patch named 'Support large thread IDs in
gdbserver' at the beginning of this month. In this commit you changed
the type of thread ID to unsigned int. One of the use of This commit is
to support for 64-bit hosts and maintain consistency. The variable
cont_thread is also declared as 'unsigned int' and 'cont_thread = -1'
statement is still in the place. 

Now please see the Amit's comments:
> > > > > > > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > > > > > > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > > > > > > > >> place. How does this work?

So believe that changing data type of thread IDs to 'long int' will be
the fix for this.

regards
-Jitendra 


On Wed, 2005-03-23 at 10:43 -0500, Daniel Jacobowitz wrote:
> On Wed, Mar 23, 2005 at 03:27:09PM +0530, Jitendra Pawar wrote:
> > > > > > > > >> The strtoul change in my patch was already present. Sorry about that.
> > > > > > > > >> 
> > > > > > > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > > > > > > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > > > > > > > >> place. How does this work?
> > > > > > 
> > > > > > There are about 15 files in gdb source have statement pid_to_ptid (-1);
> > > > > > which finally returns -1 to server. Is it OK to replace  -1 with 0 ? I
> > > > > > would like to know significance of returning pid -1, 0 and positive
> > > > > > integer.
> > > > > 
> > > whatever you need
> > > to change, you should be doing it only within gdbserver.  If you change
> > > GDB to fix a problem in gdbserver, you're changing the remote protocol.
> > 
> > Understood thanks. I believe that changing type of thread IDs to 'long
> > long' in gdbserver code will work. In that case I need to update all
> > places as you did it for changing type of thread IDs to 'unsigned long'
> > few days back. Is it the fix for this?
> 
> Why don't you explain exactly what you think is the problem, and
> exactly why you need to double the size of thread IDs to fix it?
> 

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Thu, Mar 24, 2005 at 07:06:02PM +0530, Jitendra Pawar wrote:
> > 
> > > Understood thanks. I believe that changing type of thread IDs to 'long
> > > long' in gdbserver code will work. In that case I need to update all
> > > places as you did it for changing type of thread IDs to 'unsigned long'
> > > few days back. Is it the fix for this?
> > 
> > Why don't you explain exactly what you think is the problem, and
> > exactly why you need to double the size of thread IDs to fix it?
> > 
> 
> Sorry It was my mistake, I wanted to say 'long int' instead of 'long
> long '.  and size of long int and unsigned int is same i.e. 64-bit.
> 
> You have commited the patch named 'Support large thread IDs in
> gdbserver' at the beginning of this month. In this commit you changed
> the type of thread ID to unsigned int. One of the use of This commit is
> to support for 64-bit hosts and maintain consistency. The variable
> cont_thread is also declared as 'unsigned int' and 'cont_thread = -1'
> statement is still in the place. 
> 
> Now please see the Amit's comments:
> > > > > > > > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
> > > > > > > > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > > > > > > > > >> place. How does this work?
> 
> So believe that changing data type of thread IDs to 'long int' will be
> the fix for this.

The patch fixed problems on 32-bit systems, where int and long are the
same size, by changing int to unsigned long.  It should be clear from
that explanation that changing to signed long won't fix the problem;
it will re-break NPTL.

I'll look at it soon.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Mark Kettenis]

   From: Jitendra Pawar <jitendra@linsyssoft.com>
   Date: Tue, 22 Mar 2005 19:35:24 +0530

   > > >On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
   > > >> The strtoul change in my patch was already present. Sorry about that.
   > > >> 
   > > >> You have changed the data type of thread_resume::thread as well as cont_thread 
   > > >> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
   > > >> place. How does this work?

   There are about 15 files in gdb source have statement pid_to_ptid (-1);
   which finally returns -1 to server. Is it OK to replace  -1 with 0 ?

Certainly not.  pid_to_ptid(-1), a.k.a. minus_one_ptid has a very
special meaning within gdb.  See the comment in inferior.h.

Mark

[patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Amit S. Kale]

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

Hi,

I found that gdbserver fails on a 32-bit ppc rfs running in a 64-bit 2.6 linux 
kernel. The thread id's fetched from thread-db library are above 0x7fffffff. 
gdbserver has implicit assumptions that thread id's are "signed integers". 
This is true when running under a 32-bit kernel, but not true here. Attached 
patch fixes this problem by changing the "strtol" that scans the thread-id to 
"strtoul" and by changing the "cont_thread > 0" comparison to "cont_thread != 
-1".

Is there any likelyhood of this assumption being made in some other places in 
gdbserver or gdb?

Thanks.
-Amit

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

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2005-03-10 19:31:24.000000000 +0530
+++ src/gdb/gdbserver/linux-low.c	2005-03-14 20:49:11.589413080 +0530
@@ -667,7 +667,7 @@
      then we need to make sure we restart the other threads.  We could
      pick a thread at random or restart all; restarting all is less
      arbitrary.  */
-  if (cont_thread > 0)
+  if (cont_thread != -1)
     {
       child = (struct thread_info *) find_inferior_id (&all_threads,
 						       cont_thread);
@@ -1430,7 +1430,7 @@
 {
   extern unsigned long signal_pid;
 
-  if (cont_thread > 0)
+  if (cont_thread != -1)
     {
       struct process_info *process;
 
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2005-03-10 19:31:24.000000000 +0530
+++ src/gdb/gdbserver/server.c	2005-03-14 21:05:22.774770464 +0530
@@ -193,7 +193,7 @@
       if (p[0] == 'S' || p[0] == 'C')
 	{
 	  int sig;
-	  sig = strtol (p + 1, &q, 16);
+	  sig = strtoul (p + 1, &q, 16);
 	  if (p == q)
 	    goto err;
 	  p = q;
@@ -281,7 +281,7 @@
   struct thread_resume resume_info[2];
   int n = 0;
 
-  if (step || sig || cont_thread > 0)
+  if (step || sig || cont_thread != -1)
     {
       resume_info[0].thread
 	= ((struct inferior_list_entry *) current_inferior)->id;
@@ -293,7 +293,7 @@
   resume_info[n].thread = -1;
   resume_info[n].step = 0;
   resume_info[n].sig = 0;
-  resume_info[n].leave_stopped = (cont_thread > 0);
+  resume_info[n].leave_stopped = (cont_thread != -1);
 
   (*the_target->resume) (resume_info);
 }

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Mon, Mar 14, 2005 at 09:37:35PM +0530, Amit S. Kale wrote:
> Hi,
> 
> I found that gdbserver fails on a 32-bit ppc rfs running in a 64-bit 2.6 linux 

"rfs"?  It'd be nice to explain one's acronyms...

> kernel. The thread id's fetched from thread-db library are above 0x7fffffff. 
> gdbserver has implicit assumptions that thread id's are "signed integers". 
> This is true when running under a 32-bit kernel, but not true here. Attached 
> patch fixes this problem by changing the "strtol" that scans the thread-id to 
> "strtoul" and by changing the "cont_thread > 0" comparison to "cont_thread != 
> -1".
> 
> Is there any likelyhood of this assumption being made in some other places in 
> gdbserver or gdb?

Please see:

2005-03-03  Daniel Jacobowitz  <dan@codesourcery.com>

        * inferiors.c (change_inferior_id, add_thread, find_inferior_id):
        Take unsigned long arguments for PIDs.
        * linux-low.c (add_process, linux_attach_lwp, linux_attach)
        (linux_thread_alive, linux_wait_for_event, kill_lwp, send_sigstop)
        (wait_for_sigstop, linux_resume_one_process)
        (regsets_fetch_inferior_registers, linux_send_signal)
        (linux_read_auxv): Likewise.  Update the types of variables holding
        PIDs.  Update format string specifiers.
        * linux-low.h (struct process_info, linux_attach_lwp): Likewise.
        * remote-utils.c (prepare_resume_reply): Likewise.
        * server.c (cont_thread, general_thread, step_thread)
        (thread_from_wait, old_thread_from_wait, signal_pid): Change type to
        unsigned long.
        (handle_query): Update format specifiers.
        (handle_v_cont, main): Use strtoul for thread IDs.
        * server.h (struct inferior_list_entry): Use unsigned long for ID.
        (add_thread, find_inferior_id, change_inferior_id, cont_thread)
        (general_thread, step_thread, thread_from_wait)
        (old_thread_from_wait): Update.
        * target.h (struct thread_resume): Use unsigned long for THREAD.
        (struct target_ops): Use unsigned long for arguments to attach and
        thread_alive.

Your sources are dated after this but don't seem to include this fix.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Christopher Faylor]

[reply-to set]
On Mon, Mar 14, 2005 at 11:22:52AM -0500, Daniel Jacobowitz wrote:
>On Mon, Mar 14, 2005 at 09:37:35PM +0530, Amit S. Kale wrote:
>>I found that gdbserver fails on a 32-bit ppc rfs running in a 64-bit
>>2.6 linux
>
>"rfs"?  It'd be nice to explain one's acronyms...

"R"oot "F"ile "S"system.

cgf

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Mon, Mar 14, 2005 at 11:57:08AM -0500, Christopher Faylor wrote:
> [reply-to set]
> On Mon, Mar 14, 2005 at 11:22:52AM -0500, Daniel Jacobowitz wrote:
> >On Mon, Mar 14, 2005 at 09:37:35PM +0530, Amit S. Kale wrote:
> >>I found that gdbserver fails on a 32-bit ppc rfs running in a 64-bit
> >>2.6 linux
> >
> >"rfs"?  It'd be nice to explain one's acronyms...
> 
> "R"oot "F"ile "S"system.

Ah!  Thank you...

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Amit S. Kale]

The strtoul change in my patch was already present. Sorry about that.

You have changed the data type of thread_resume::thread as well as cont_thread 
to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
place. How does this work?

Can we use a thread value of "0" to indicate all threads or no-thread instead 
of "-1"? The condition "cont_thread > 0" becomes valid if we do that.

-Amit

On Monday 14 Mar 2005 9:52 pm, Daniel Jacobowitz wrote:
> On Mon, Mar 14, 2005 at 09:37:35PM +0530, Amit S. Kale wrote:
> > Hi,
> >
> > I found that gdbserver fails on a 32-bit ppc rfs running in a 64-bit 2.6
> > linux
>
> "rfs"?  It'd be nice to explain one's acronyms...
>
> > kernel. The thread id's fetched from thread-db library are above
> > 0x7fffffff. gdbserver has implicit assumptions that thread id's are
> > "signed integers". This is true when running under a 32-bit kernel, but
> > not true here. Attached patch fixes this problem by changing the "strtol"
> > that scans the thread-id to "strtoul" and by changing the "cont_thread >
> > 0" comparison to "cont_thread != -1".
> >
> > Is there any likelyhood of this assumption being made in some other
> > places in gdbserver or gdb?
>
> Please see:
>
> 2005-03-03  Daniel Jacobowitz  <dan@codesourcery.com>
>
>         * inferiors.c (change_inferior_id, add_thread, find_inferior_id):
>         Take unsigned long arguments for PIDs.
>         * linux-low.c (add_process, linux_attach_lwp, linux_attach)
>         (linux_thread_alive, linux_wait_for_event, kill_lwp, send_sigstop)
>         (wait_for_sigstop, linux_resume_one_process)
>         (regsets_fetch_inferior_registers, linux_send_signal)
>         (linux_read_auxv): Likewise.  Update the types of variables holding
>         PIDs.  Update format string specifiers.
>         * linux-low.h (struct process_info, linux_attach_lwp): Likewise.
>         * remote-utils.c (prepare_resume_reply): Likewise.
>         * server.c (cont_thread, general_thread, step_thread)
>         (thread_from_wait, old_thread_from_wait, signal_pid): Change type
> to unsigned long.
>         (handle_query): Update format specifiers.
>         (handle_v_cont, main): Use strtoul for thread IDs.
>         * server.h (struct inferior_list_entry): Use unsigned long for ID.
>         (add_thread, find_inferior_id, change_inferior_id, cont_thread)
>         (general_thread, step_thread, thread_from_wait)
>         (old_thread_from_wait): Update.
>         * target.h (struct thread_resume): Use unsigned long for THREAD.
>         (struct target_ops): Use unsigned long for arguments to attach and
>         thread_alive.
>
> Your sources are dated after this but don't seem to include this fix.

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
> The strtoul change in my patch was already present. Sorry about that.
> 
> You have changed the data type of thread_resume::thread as well as cont_thread 
> to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> place. How does this work?

Hmm, looks like you found a real problem here; would you like to fix
it?  Modern GDBs don't heavily use this code, because of the vCont
packet.  Otherwise I'll fix it, but I may not find time for a bit.

> Can we use a thread value of "0" to indicate all threads or no-thread instead 
> of "-1"? The condition "cont_thread > 0" becomes valid if we do that.

This value can be set from the remote protocol, so we would have to
transform it in Hc/Hg support also.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [patch] gdbserver fails on 32-bit ppc rfs running in a-64 bit 2.6 linux kernel

[Daniel Jacobowitz]

On Tue, Mar 15, 2005 at 10:16:09PM -0500, Daniel Jacobowitz wrote:
> On Tue, Mar 15, 2005 at 06:36:22PM +0530, Amit S. Kale wrote:
> > The strtoul change in my patch was already present. Sorry about that.
> > 
> > You have changed the data type of thread_resume::thread as well as cont_thread 
> > to unsigned long. "cont_thread = -1" and "(cont_thread > 0)" are still in 
> > place. How does this work?
> 
> Hmm, looks like you found a real problem here; would you like to fix
> it?  Modern GDBs don't heavily use this code, because of the vCont
> packet.  Otherwise I'll fix it, but I may not find time for a bit.
> 
> > Can we use a thread value of "0" to indicate all threads or no-thread instead 
> > of "-1"? The condition "cont_thread > 0" becomes valid if we do that.
> 
> This value can be set from the remote protocol, so we would have to
> transform it in Hc/Hg support also.

Better late than never: this patch eliminates the bogus -1 checks. 
Tested i686-linux, committed.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-06-16  Daniel Jacobowitz  <dan@codesourcery.com>

	* linux-low.c (linux_wait, linux_send_signal): Don't test
	an unsigned long variable for > 0 if it could be MAX_ULONG.
	* server.c (myresume): Likewise.
	* target.c (set_desired_inferior): Likewise.

Index: linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.37
diff -u -p -r1.37 linux-low.c
--- linux-low.c	13 Jun 2005 01:59:22 -0000	1.37
+++ linux-low.c	17 Jun 2005 03:51:05 -0000
@@ -667,7 +667,7 @@ retry:
      then we need to make sure we restart the other threads.  We could
      pick a thread at random or restart all; restarting all is less
      arbitrary.  */
-  if (cont_thread > 0)
+  if (cont_thread != 0 && cont_thread != -1)
     {
       child = (struct thread_info *) find_inferior_id (&all_threads,
 						       cont_thread);
@@ -1435,7 +1435,7 @@ linux_send_signal (int signum)
 {
   extern unsigned long signal_pid;
 
-  if (cont_thread > 0)
+  if (cont_thread != 0 && cont_thread != -1)
     {
       struct process_info *process;
 
Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.26
diff -u -p -r1.26 server.c
--- server.c	13 Jun 2005 01:59:22 -0000	1.26
+++ server.c	17 Jun 2005 03:51:05 -0000
@@ -281,7 +281,7 @@ myresume (int step, int sig)
   struct thread_resume resume_info[2];
   int n = 0;
 
-  if (step || sig || cont_thread > 0)
+  if (step || sig || (cont_thread != 0 && cont_thread != -1))
     {
       resume_info[0].thread
 	= ((struct inferior_list_entry *) current_inferior)->id;
@@ -293,7 +293,7 @@ myresume (int step, int sig)
   resume_info[n].thread = -1;
   resume_info[n].step = 0;
   resume_info[n].sig = 0;
-  resume_info[n].leave_stopped = (cont_thread > 0);
+  resume_info[n].leave_stopped = (cont_thread != 0 && cont_thread != -1);
 
   (*the_target->resume) (resume_info);
 }
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/target.c,v
retrieving revision 1.5
diff -u -p -r1.5 target.c
--- target.c	13 Jun 2005 01:59:22 -0000	1.5
+++ target.c	17 Jun 2005 03:51:05 -0000
@@ -42,7 +42,8 @@ set_desired_inferior (int use_general)
       /* If we are continuing any (all) thread(s), use step_thread
 	 to decide which thread to step and/or send the specified
 	 signal to.  */
-      if (step_thread > 0 && (cont_thread == 0 || cont_thread == -1))
+      if ((step_thread != 0 && step_thread != -1)
+	  && (cont_thread == 0 || cont_thread == -1))
 	found = (struct thread_info *) find_inferior_id (&all_threads,
 							 step_thread);