This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/9] add target_ops fields use_agent and can_use_agent


On 02/24/2012 05:17 AM, Pedro Alves wrote:
>> > @@ -4882,6 +4883,17 @@ info_static_tracepoint_markers_command (char *arg, int from_tty)
>> >    struct ui_out *uiout = current_uiout;
>> >    int i;
>> >  
>> > +  if (target_can_use_agent () == 0)
> Hmm, I don't think doing this change now is right.  If you connect GDB to a GDBserver
> that hasn't been patched, this will fail because that GDBserver doesn't report
> the QAgent feature, so it is an incompatible change of the protocol to require QAgent
> for static tracepoint listing.
> The right thing to do seems to be to just go ahead and try the target method call,
> as before.  We could at a minimum check whether the "statictracepoints" qSupported
> feature Was reported by the target, though that doesn't tell you whether the current
> inferior has the IPA or UST loaded at all, only that the frontend stub understands
> static tracepoints.  What was the motivation for these checks here?
> 

The motivation is quite simple, `since we have some conditions, it is
better to check them first'.  However, it breaks compatibility, as you
pointed out.  In this version, I get rid of these checks, and only leave
a paragraph of comment on why we don't check these conditions there.

-- 
Yao (éå)
2012-02-17  Yao Qi  <yao@codesourcery.com>

	* target.h (struct target_ops) <to_use_agent>: New field.
	(struct target_ops) <to_can_use_agent>: New field.
	(target_use_agent, target_can_use_agent): New macro.
	* target.c (update_current_target): Update.
	* remote.c: New enum `PACKET_QAgent'.
	(remote_protocol_features): Add a new element.
	(remote_use_agent, remote_can_use_agent): New.
	(init_remote_ops): Initialize field `can_use_agent' with
	remote_can_use_agent.  Intiailize field `use_agent' with
	remote_use_agent.
	* common/agent.c (use_agent): New global.
	* common/agent.h: Declare it.
	* tracepoint.c (info_static_tracepoint_markers_command): Add
	comment.

gdb/gdbserver:

2012-02-17  Yao Qi  <yao@codesourcery.com>

	* linux-low.c (linux_supports_agent): New.
	(linux_target_ops): Initialize field `supports_agent' with
	linux_supports_agent.
	* target.h (struct target_ops) <supports_agent>: New.
	(target_supports_agent): New macro.
	* server.c (handle_general_set): Handle packet 'QAgent'.
	(handle_query): Send `QAgent+'.
	* Makefile.in (server.o): Depends on agent.h

gdb/doc:

2012-02-17  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (General Query Packets): Add packet `QAgent'.
---
 gdb/common/agent.c        |    3 +++
 gdb/common/agent.h        |    1 +
 gdb/doc/gdb.texinfo       |   13 +++++++++++++
 gdb/gdbserver/Makefile.in |    2 +-
 gdb/gdbserver/linux-low.c |    7 +++++++
 gdb/gdbserver/server.c    |   28 ++++++++++++++++++++++++++++
 gdb/gdbserver/target.h    |    7 +++++++
 gdb/remote.c              |   36 ++++++++++++++++++++++++++++++++++++
 gdb/target.c              |    8 ++++++++
 gdb/target.h              |   13 +++++++++++++
 gdb/tracepoint.c          |    6 ++++++
 11 files changed, 123 insertions(+), 1 deletions(-)

diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 8f7dded..044024f 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -41,6 +41,9 @@ int debug_agent = 0;
     fprintf_unfiltered (gdb_stdlog, fmt, ##args);
 #endif
 
+/* Global flag to determine using agent or not.  */
+int use_agent = 0;
+
 /* Addresses of in-process agent's symbols both GDB and GDBserver cares
    about.  */
 
diff --git a/gdb/common/agent.h b/gdb/common/agent.h
index 43e128c..53c6862 100644
--- a/gdb/common/agent.h
+++ b/gdb/common/agent.h
@@ -35,3 +35,4 @@ int agent_look_up_symbols (void *);
 
 extern int debug_agent;
 
+extern int use_agent;
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 02beed7..34bf77e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34512,6 +34512,11 @@ Here are the currently defined query and set packets:
 
 @table @samp
 
+@item QAgent:1
+@item QAgent:0
+Turn on or off the agent as a helper to perform some debugging operations
+delegated from @value{GDBN} (@pxref{Control Agent}).
+
 @item QAllow:@var{op}:@var{val}@dots{}
 @cindex @samp{QAllow} packet
 Specify which operations @value{GDBN} expects to request of the
@@ -35091,6 +35096,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{QAgent}
+@tab No
+@tab @samp{-}
+@tab No
+
 @item @samp{QAllow}
 @tab No
 @tab @samp{-}
@@ -35224,6 +35234,9 @@ The remote stub accepts and implements the reverse step packet
 The remote stub understands the @samp{QTDPsrc} packet that supplies
 the source form of tracepoint definitions.
 
+@item QAgent
+The remote stub understands the @samp{QAgent} packet.
+
 @item QAllow
 The remote stub understands the @samp{QAllow} packet.
 
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 17fd043..e840db2 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -395,7 +395,7 @@ mem-break.o: mem-break.c $(server_h)
 proc-service.o: proc-service.c $(server_h) $(gdb_proc_service_h)
 regcache.o: regcache.c $(server_h) $(regdef_h)
 remote-utils.o: remote-utils.c terminal.h $(server_h)
-server.o: server.c $(server_h)
+server.o: server.c $(server_h) $(agent_h)
 target.o: target.c $(server_h)
 thread-db.o: thread-db.c $(server_h) $(linux_low_h) $(gdb_proc_service_h) \
 	$(gdb_thread_db_h)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index ab34d84..c9b8a3b 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4716,6 +4716,12 @@ linux_supports_disable_randomization (void)
 #endif
 }
 
+static int
+linux_supports_agent (void)
+{
+  return 1;
+}
+
 /* Enumerate spufs IDs for process PID.  */
 static int
 spu_enumerate_spu_ids (long pid, unsigned char *buf, CORE_ADDR offset, int len)
@@ -5438,6 +5444,7 @@ static struct target_ops linux_target_ops = {
   linux_supports_disable_randomization,
   linux_get_min_fast_tracepoint_insn_len,
   linux_qxfer_libraries_svr4,
+  linux_supports_agent,
 };
 
 static void
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index c1788a9..28aef72 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "server.h"
+#include "agent.h"
 
 #if HAVE_UNISTD_H
 #include <unistd.h>
@@ -529,6 +530,30 @@ handle_general_set (char *own_buf)
       && handle_tracepoint_general_set (own_buf))
     return;
 
+  if (strncmp ("QAgent:", own_buf, strlen ("QAgent:")) == 0)
+    {
+      char *mode = own_buf + strlen ("QAgent:");
+      int req = 0;
+
+      if (strcmp (mode, "0") == 0)
+	req = 0;
+      else if (strcmp (mode, "1") == 0)
+	req = 1;
+      else
+	{
+	  /* We don't know what this value is, so complain to GDB.  */
+	  sprintf (own_buf, "E.Unknown QAgent value");
+	  return;
+	}
+
+      /* Update the flag.  */
+      use_agent = req;
+      if (remote_debug)
+	fprintf (stderr, "[%s agent]\n", req ? "Enable" : "Disable");
+      write_ok (own_buf);
+      return;
+    }
+
   /* Otherwise we didn't know what packet it was.  Say we didn't
      understand it.  */
   own_buf[0] = 0;
@@ -1621,6 +1646,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	  strcat (own_buf, ";tracenz+");
 	}
 
+      if (target_supports_agent ())
+	strcat (own_buf, ";QAgent+");
+
       return;
     }
 
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 03dff0f..256cfd9 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -394,6 +394,9 @@ struct target_ops
   int (*qxfer_libraries_svr4) (const char *annex, unsigned char *readbuf,
 			       unsigned const char *writebuf,
 			       CORE_ADDR offset, int len);
+
+  /* Return true if target supports debugging agent.  */
+  int (*supports_agent) (void);
 };
 
 extern struct target_ops *the_target;
@@ -514,6 +517,10 @@ void set_target_ops (struct target_ops *);
   (the_target->supports_disable_randomization ? \
    (*the_target->supports_disable_randomization) () : 0)
 
+#define target_supports_agent() \
+  (the_target->supports_agent ? \
+   (*the_target->supports_agent) () : 0)
+
 /* Start non-stop mode, returns 0 on success, -1 on failure.   */
 
 int start_non_stop (int nonstop);
diff --git a/gdb/remote.c b/gdb/remote.c
index 3187ac0..2977e31 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -65,6 +65,7 @@
 #include "tracepoint.h"
 #include "ax.h"
 #include "ax-gdb.h"
+#include "agent.h"
 
 /* Temp hacks for tracepoint encoding migration.  */
 static char *target_buf;
@@ -1272,6 +1273,7 @@ enum {
   PACKET_QAllow,
   PACKET_qXfer_fdpic,
   PACKET_QDisableRandomization,
+  PACKET_QAgent,
   PACKET_MAX
 };
 
@@ -3832,6 +3834,7 @@ static struct protocol_feature remote_protocol_features[] = {
     PACKET_qXfer_fdpic },
   { "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
     PACKET_QDisableRandomization },
+  { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
   { "tracenz", PACKET_DISABLE,
     remote_string_tracing_feature, -1 },
 };
@@ -10669,6 +10672,34 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
   return 1;
 }
 
+static int
+remote_use_agent (int use)
+{
+  if (remote_protocol_packets[PACKET_QAgent].support != PACKET_DISABLE)
+    {
+      struct remote_state *rs = get_remote_state ();
+
+      /* If the stub supports QAgent.  */
+      sprintf (rs->buf, "QAgent:%d", use);
+      putpkt (rs->buf);
+      getpkt (&rs->buf, &rs->buf_size, 0);
+
+      if (strcmp (rs->buf, "OK") == 0)
+	{
+	  use_agent = use;
+	  return 1;
+	}
+    }
+
+  return 0;
+}
+
+static int
+remote_can_use_agent (void)
+{
+  return (remote_protocol_packets[PACKET_QAgent].support != PACKET_DISABLE);
+}
+
 static void
 init_remote_ops (void)
 {
@@ -10778,6 +10809,8 @@ Specify the serial device it is connected to\n\
   remote_ops.to_static_tracepoint_markers_by_strid
     = remote_static_tracepoint_markers_by_strid;
   remote_ops.to_traceframe_info = remote_traceframe_info;
+  remote_ops.to_use_agent = remote_use_agent;
+  remote_ops.to_can_use_agent = remote_can_use_agent;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
@@ -11286,6 +11319,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_QDisableRandomization],
 			 "QDisableRandomization", "disable-randomization", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_QAgent],
+			 "QAgent", "agent", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.c b/gdb/target.c
index ad304bc..a7c11c3 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -698,6 +698,8 @@ update_current_target (void)
       INHERIT (to_static_tracepoint_marker_at, t);
       INHERIT (to_static_tracepoint_markers_by_strid, t);
       INHERIT (to_traceframe_info, t);
+      INHERIT (to_use_agent, t);
+      INHERIT (to_can_use_agent, t);
       INHERIT (to_magic, t);
       /* Do not inherit to_memory_map.  */
       /* Do not inherit to_flash_erase.  */
@@ -925,6 +927,12 @@ update_current_target (void)
   de_fault (to_traceframe_info,
 	    (struct traceframe_info * (*) (void))
 	    tcomplain);
+  de_fault (to_use_agent,
+	    (int (*) (int))
+	    tcomplain);
+  de_fault (to_can_use_agent,
+	    (int (*) (void))
+	    return_zero);
   de_fault (to_execution_direction, default_execution_direction);
 
 #undef de_fault
diff --git a/gdb/target.h b/gdb/target.h
index d4605ae..3bc9428 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -832,6 +832,13 @@ struct target_ops
        re-fetching when necessary.  */
     struct traceframe_info *(*to_traceframe_info) (void);
 
+    /* Ask the target to use or not to use agent according to USE.  Return 1
+       successful, 0 otherwise.  */
+    int (*to_use_agent) (int use);
+
+    /* Is the target able to use agent in current state?  */
+    int (*to_can_use_agent) (void);
+
     int to_magic;
     /* Need sub-structure for target machine related rather than comm related?
      */
@@ -1663,6 +1670,12 @@ extern char *target_fileio_read_stralloc (const char *filename);
 #define target_traceframe_info() \
   (*current_target.to_traceframe_info) ()
 
+#define target_use_agent(use) \
+  (*current_target.to_use_agent) (use)
+
+#define target_can_use_agent() \
+  (*current_target.to_can_use_agent) ()
+
 /* Command logging facility.  */
 
 #define target_log_command(p)						\
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 37e1f52..194977b 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -4882,6 +4882,12 @@ info_static_tracepoint_markers_command (char *arg, int from_tty)
   struct ui_out *uiout = current_uiout;
   int i;
 
+  /* We don't have to check target_can_use_agent and agent's capability on
+     static tracepoint here, in order to be compatible with older GDBserver.
+     We don't check USE_AGENT is true or not, because static tracepoints
+     don't work without in-process agent, so we don't bother users to type
+     `set agent on' when to use static tracepoint.  */
+
   old_chain
     = make_cleanup_ui_out_table_begin_end (uiout, 5, -1,
 					   "StaticTracepointMarkersTable");
-- 
1.7.0.4


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]