Re: [RFA] add remote.c gdbarch thread-handling hooks

Re: [RFA] add remote.c gdbarch thread-handling hooks

[Nick Duffek]

On 16-Jul-2001, Andrew Cagney wrote:

>Just FYI, I backed this out.

You beat me to it.

>Just that it included a little bit more than I suspect you intended.

Yup.  I'll re-commit just the part that I posted, okay?

Nick

Re: [RFA] add remote.c gdbarch thread-handling hooks

[Nick Duffek]

On 16-Jul-2001, Andrew Cagney wrote:

>Nick, could you please post a patch of what you actually committed.

Are you saying that you weren't approving the obvious part of the patch
when you wrote the following?

>I'd assume this bit is pretty obvious and can go in as a separate 
>independant patch.

Here it is again.

Nick

Index: gdb/remote.c
===================================================================
diff -up gdb/remote.c gdb/remote.c
--- gdb/remote.c	Mon Jul 16 19:31:11 2001
+++ gdb/remote.c	Mon Jul 16 19:30:22 2001
@@ -5235,8 +5235,8 @@ Specify the serial device it is connecte
   remote_ops.to_mourn_inferior = remote_mourn;
   remote_ops.to_thread_alive = remote_thread_alive;
   remote_ops.to_find_new_threads = remote_threads_info;
-  remote_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_ops.to_pid_to_str = remote_pid_to_str;
+  remote_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_ops.to_stop = remote_stop;
   remote_ops.to_query = remote_query;
   remote_ops.to_rcmd = remote_rcmd;
@@ -5644,7 +5644,8 @@ Specify the serial device it is connecte
   remote_cisco_ops.to_mourn_inferior = remote_cisco_mourn;
   remote_cisco_ops.to_thread_alive = remote_thread_alive;
   remote_cisco_ops.to_find_new_threads = remote_threads_info;
-  remote_ops.to_extra_thread_info = remote_threads_extra_info;
+  remote_cisco_ops.to_pid_to_str = remote_pid_to_str;
+  remote_cisco_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_cisco_ops.to_stratum = process_stratum;
   remote_cisco_ops.to_has_all_memory = 1;
   remote_cisco_ops.to_has_memory = 1;
@@ -5734,7 +5735,8 @@ Specify the serial device it is connecte
   remote_async_ops.to_mourn_inferior = remote_async_mourn;
   remote_async_ops.to_thread_alive = remote_thread_alive;
   remote_async_ops.to_find_new_threads = remote_threads_info;
-  remote_ops.to_extra_thread_info = remote_threads_extra_info;
+  remote_async_ops.to_pid_to_str = remote_pid_to_str;
+  remote_async_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_async_ops.to_stop = remote_stop;
   remote_async_ops.to_query = remote_query;
   remote_async_ops.to_rcmd = remote_rcmd;

Re: [RFA] add remote.c gdbarch thread-handling hooks

[Andrew Cagney]

> On 16-Jul-2001, Andrew Cagney wrote in direct e-mail:
> 
> 
>>Nick, could you please post a patch of what you actually committed.
> 
> 
> Are you saying that you weren't approving the obvious part of the patch
> when you wrote the following?
> 
>>I'd assume this bit is pretty obvious and can go in as a separate 
>>independant patch.


No.  I'm just asking you post a patch of what you separated out and 
committed as obvious.

>>Just that it included a little bit more than I suspect you intended.
> 
> 
> Yup.  I'll re-commit just the part that I posted, okay?


Yes, as I wrote (now that I've spent 10 minutes grubbing around in CVS 
finding out what happened :-):

 > Nothing wrong with your intended changes.

	Andrew

[RFA] add remote.c gdbarch thread-handling hooks

[Nick Duffek]

Hi,

I've been working on an architecture with a primitive threading mechanism
that GDB queries and manipulates by directly setting registers in the
target.

This patch facilitates that with hooks for target-specific code to
override remote.c thread-handling functions.

It also fixes a few typos in init_remote_cisco_ops() and
init_remote_async_ops() and moves assignment of a few target_ops fields to
be consistent with ordering in target.h.

ChangeLog:

	* gdbarch.sh (REMOTE_SET_THREAD, REMOTE_THREAD_ALIVE,
	REMOTE_CURRENT_THREAD, REMOTE_THREADS_INFO,
	REMOTE_THREADS_EXTRA_INFO, REMOTE_PID_TO_STR): New functions.
	(struct thread_info): New forward declaration.
	* remote.c (set_thread): Call REMOTE_SET_THREAD if defined.
	(remote_thread_alive): Call REMOTE_THREAD_ALIVE if defined.
	(remote_current_thread, remote_wait, remote_async_wait): Call
	REMOTE_CURRENT_THREAD if defined.
	(remote_threads_info): Call REMOTE_THREADS_INFO if defined.
	(remote_threads_extra_info): Call REMOTE_THREADS_EXTRA_INFO if
	defined.
	(remote_pid_to_str): Call REMOTE_PID_TO_STR if defined.
	(init_remote_ops, init_remote_cisco_ops, init_remote_async_ops):
	Set to_pid_to_str and to_extra_thread_info correctly and in the
	same order as declared in target.h.

Okay to apply?

Nick Duffek
<nsd@redhat.com>

[patch follows]

Index: gdb/gdbarch.sh
===================================================================
diff -up gdb/gdbarch.sh gdb/gdbarch.sh
--- gdb/gdbarch.sh	Mon Jul 16 13:19:25 2001
+++ gdb/gdbarch.sh	Mon Jul 16 13:19:19 2001
@@ -495,6 +495,20 @@ f::PREPARE_TO_PROCEED:int:prepare_to_pro
 v:2:FUNCTION_START_OFFSET:CORE_ADDR:function_start_offset::::0:-1
 #
 f:2:REMOTE_TRANSLATE_XFER_ADDRESS:void:remote_translate_xfer_address:CORE_ADDR gdb_addr, int gdb_len, CORE_ADDR *rem_addr, int *rem_len:gdb_addr, gdb_len, rem_addr, rem_len:::generic_remote_translate_xfer_address::0
+# Set the remote system's current thread to TID, in preparation for general
+# queries (e.g. register fetch) if GEN and for continuing execution
+# otherwise.  TID 0 means all threads, -1 means any thread.
+F:2:REMOTE_SET_THREAD:void:remote_set_thread:int tid, int gen:tid, gen::0
+# Return whether thread TID is still alive on the remote system.
+F:2:REMOTE_THREAD_ALIVE:int:remote_thread_alive:int tid:tid::0
+# Return the remote system's current thread id if known, else return PREVTID.
+F:2:REMOTE_CURRENT_THREAD:int:remote_current_thread:int prevtid:prevtid::0
+# Query the remote system for new threads and add them to the thread list.
+F:2:REMOTE_THREADS_INFO:void:remote_threads_info:void:::0
+# Return a static string describing the state of thread TP->pid.
+F:2:REMOTE_THREADS_EXTRA_INFO:char *:remote_threads_extra_info:struct thread_info *tp:tp::0
+# Return a static string representation of internal gdb process id PID.
+F:2:REMOTE_PID_TO_STR:char *:remote_pid_to_str:ptid_t ptid:ptid::0
 #
 v:2:FRAME_ARGS_SKIP:CORE_ADDR:frame_args_skip::::0:-1
 f:2:FRAMELESS_FUNCTION_INVOCATION:int:frameless_function_invocation:struct frame_info *fi:fi:::generic_frameless_function_invocation_not::0
@@ -625,6 +639,7 @@ cat <<EOF
 #define GDBARCH_H
 
 struct frame_info;
+struct thread_info;
 struct value;
 
 
Index: gdb/remote.c
===================================================================
diff -up gdb/remote.c gdb/remote.c
--- gdb/remote.c	Mon Jul 16 13:19:31 2001
+++ gdb/remote.c	Mon Jul 16 13:19:19 2001
@@ -937,6 +937,12 @@ set_thread (int th, int gen)
   char *buf = alloca (PBUFSIZ);
   int state = gen ? general_thread : continue_thread;
 
+  if (REMOTE_SET_THREAD_P ())
+    {
+      REMOTE_SET_THREAD (th, gen);
+      return;
+    }
+
   if (state == th)
     return;
 
@@ -967,6 +973,9 @@ remote_thread_alive (ptid_t ptid)
   int tid = PIDGET (ptid);
   char buf[16];
 
+  if (REMOTE_THREAD_ALIVE_P ())
+    return REMOTE_THREAD_ALIVE (tid);
+
   if (tid < 0)
     sprintf (buf, "T-%08x", -tid);
   else
@@ -1654,6 +1663,9 @@ remote_current_thread (ptid_t oldpid)
 {
   char *buf = alloca (PBUFSIZ);
 
+  if (REMOTE_CURRENT_THREAD_P ())
+    return pid_to_ptid (REMOTE_CURRENT_THREAD (PIDGET (oldpid)));
+
   putpkt ("qC");
   getpkt (buf, PBUFSIZ, 0);
   if (buf[0] == 'Q' && buf[1] == 'C')
@@ -1692,6 +1704,12 @@ remote_threads_info (void)
   if (remote_desc == 0)		/* paranoia */
     error ("Command can only be used when connected to the remote target.");
 
+  if (REMOTE_THREADS_INFO_P ())
+    {
+      REMOTE_THREADS_INFO ();
+      return;
+    }
+
   if (use_threadinfo_query)
     {
       putpkt ("qfThreadInfo");
@@ -1746,6 +1764,9 @@ remote_threads_extra_info (struct thread
     internal_error (__FILE__, __LINE__,
 		    "remote_threads_extra_info");
 
+  if (REMOTE_THREADS_EXTRA_INFO_P ())
+    return REMOTE_THREADS_EXTRA_INFO (tp);
+
   if (use_threadextra_query)
     {
       sprintf (bufp, "qThreadExtraInfo,%x", PIDGET (tp->ptid));
@@ -3062,6 +3083,13 @@ Packet Dropped");
 	}
     }
 got_status:
+
+  if (REMOTE_CURRENT_THREAD_P () && status->kind == TARGET_WAITKIND_STOPPED)
+    {
+      thread_num = REMOTE_CURRENT_THREAD (thread_num);
+      if (thread_num != -1)
+	record_currthread (thread_num);
+    }
   if (thread_num != -1)
     {
       return pid_to_ptid (thread_num);
@@ -3284,6 +3312,13 @@ Packet Dropped");
 	}
     }
 got_status:
+
+  if (REMOTE_CURRENT_THREAD_P () && status->kind == TARGET_WAITKIND_STOPPED)
+    {
+      thread_num = REMOTE_CURRENT_THREAD (thread_num);
+      if (thread_num != -1)
+	record_currthread (thread_num);
+    }
   if (thread_num != -1)
     {
       return pid_to_ptid (thread_num);
@@ -5197,6 +5232,9 @@ remote_pid_to_str (ptid_t ptid)
 {
   static char buf[30];
 
+  if (REMOTE_PID_TO_STR_P ())
+    return REMOTE_PID_TO_STR (ptid);
+
   sprintf (buf, "Thread %d", PIDGET (ptid));
   return buf;
 }
@@ -5227,8 +5265,8 @@ Specify the serial device it is connecte
   remote_ops.to_mourn_inferior = remote_mourn;
   remote_ops.to_thread_alive = remote_thread_alive;
   remote_ops.to_find_new_threads = remote_threads_info;
-  remote_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_ops.to_pid_to_str = remote_pid_to_str;
+  remote_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_ops.to_stop = remote_stop;
   remote_ops.to_query = remote_query;
   remote_ops.to_rcmd = remote_rcmd;
@@ -5636,7 +5674,8 @@ Specify the serial device it is connecte
   remote_cisco_ops.to_mourn_inferior = remote_cisco_mourn;
   remote_cisco_ops.to_thread_alive = remote_thread_alive;
   remote_cisco_ops.to_find_new_threads = remote_threads_info;
-  remote_ops.to_extra_thread_info = remote_threads_extra_info;
+  remote_cisco_ops.to_pid_to_str = remote_pid_to_str;
+  remote_cisco_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_cisco_ops.to_stratum = process_stratum;
   remote_cisco_ops.to_has_all_memory = 1;
   remote_cisco_ops.to_has_memory = 1;
@@ -5726,7 +5765,8 @@ Specify the serial device it is connecte
   remote_async_ops.to_mourn_inferior = remote_async_mourn;
   remote_async_ops.to_thread_alive = remote_thread_alive;
   remote_async_ops.to_find_new_threads = remote_threads_info;
-  remote_ops.to_extra_thread_info = remote_threads_extra_info;
+  remote_async_ops.to_pid_to_str = remote_pid_to_str;
+  remote_async_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_async_ops.to_stop = remote_stop;
   remote_async_ops.to_query = remote_query;
   remote_async_ops.to_rcmd = remote_rcmd;

Re: [RFA] add remote.c gdbarch thread-handling hooks

[Andrew Cagney]

> I've been working on an architecture with a primitive threading mechanism
> that GDB queries and manipulates by directly setting registers in the
> target.
> 
> This patch facilitates that with hooks for target-specific code to
> override remote.c thread-handling functions.


Nick, while creative, this looks wrong.  You're using remote.c to half 
implement some new custom protocol.


> It also fixes a few typos in init_remote_cisco_ops() and
> init_remote_async_ops() and moves assignment of a few target_ops fields to
> be consistent with ordering in target.h.


I'd assume this bit is pretty obvious and can go in as a separate 
independant patch.

	Andrew

Re: [RFA] add remote.c gdbarch thread-handling hooks

[Nick Duffek]

On 16-Jul-2001, Andrew Cagney wrote:

>> It also fixes a few typos in init_remote_cisco_ops() and
>> init_remote_async_ops() and moves assignment of a few target_ops fields to
>> be consistent with ordering in target.h.

>I'd assume this bit is pretty obvious and can go in as a separate 
>independant patch.

Committed.

Nick

Re: [RFA] add remote.c gdbarch thread-handling hooks

[J.T. Conklin]

>>>>> "Nick" == Nick Duffek <nsd@redhat.com> writes:
Nick> I've been working on an architecture with a primitive threading
Nick> mechanism that GDB queries and manipulates by directly setting
Nick> registers in the target.
Nick>
Nick> This patch facilitates that with hooks for target-specific code
Nick> to override remote.c thread-handling functions.

Until I'm convinced otherwise, I'm against adding this extension to
remote.c.

This change essentially creates a easy to use mechanism for creating
yet another remote protocol varient.  It's actually worse than that, 
since it overrides existing protocol.  While I think we support too
many protocol varients already, I'd be more sympathetic to a change
adding another varient than hijacking the existing protocol definitions.

Also, the remote protocol is intendent to be target neutral.  Why
can't the existing thread primitives be changed into the register
manipulation in the target's debug agent?  Although I don't have any
information on the architecture in question, this proposed change 
appears to tie GDB to one possible thread implementation.  

However, if it is true that this is the only thread model used under
this architecture, I the change is at the wrong level.  Binding it
into remote.c means that it won't be available to other back ends.  To
use this with an ICE or a ROM monitor, a user would have to write a
gdbserver-like program to attach to from GDB which would in turn talk
to the target.

        --jtc

-- 
J.T. Conklin
RedBack Networks