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]

[PATCH 2/2] OO tracepoint_action


This is the major part of this patch series, which replace several
switch/case blocks in code with function vectors.  It is easier to add
new operation to tracepoint_action in the future.

gdb/gdbserver:

	* tracepoint.c (struct tracepoint_action_ops): New.
	(struct tracepoint_action) <ops>: New field.
	(record_tracepoint_error): Removed.
	(record_expr_eval_error): New.
	(add_tracepoint_action): Move code to ...
	(m_tracepoint_action_init, r_tracepoint_action_init):
	(x_tracepoint_action_init, l_tracepoint_action_init: ... here.
	New.
	(download_tracepoint_1): Move code to ...
	(m_tracepoint_action_download, r_tracepoint_action_download):
	(x_tracepoint_action_download, l_tracepoint_action_download):
	.. .here.  New.
	(do_action_at_tracepoint): Move code to ...
	(m_tracepoint_action_execute, r_tracepoint_action_execute):
	(x_tracepoint_action_execute, l_tracepoint_action_execute):
	... here.  New.
	(condition_true_at_tracepoint): Call record_expr_eval_error
	instead.
---
 gdb/gdbserver/tracepoint.c |  560 ++++++++++++++++++++++++++++----------------
 1 files changed, 358 insertions(+), 202 deletions(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 5dcc7a4..bc23825 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -469,12 +469,38 @@ write_inferior_uinteger (CORE_ADDR symaddr, unsigned int val)
   return write_inferior_memory (symaddr, (unsigned char *) &val, sizeof (val));
 }
 
+static CORE_ADDR target_malloc (ULONGEST size);
+static int write_inferior_data_ptr (CORE_ADDR where, CORE_ADDR ptr);
 #endif
 
+/* Operations on various types of tracepoint actions.  */
+
+struct tracepoint_action;
+struct tracepoint_hit_ctx;
+
+struct tracepoint_action_ops
+{
+#ifndef IN_PROCESS_AGENT
+  /* Parse rsp packet PACKET and initialize ACTION.  Return the updated
+     pointer to rsp packet..  */
+  char* (*init) (char *packet,  struct tracepoint_action* action);
+
+  /* Download tracepoint action ACTION to IPA.  Return the address of action
+     in IPA/inferior.  */
+  CORE_ADDR (*download) (struct tracepoint_action *action);
+#endif
+
+  /* Do tracepoint action ACTION.  Return 0 if success otherwise return
+     non-zero.   */
+  int (*execute) (struct tracepoint_action *action,
+		  struct tracepoint_hit_ctx *ctx, struct traceframe *tframe);
+};
+
 /* Base action.  Concrete actions inherit this.  */
 
 struct tracepoint_action
 {
+  struct tracepoint_action_ops *ops;
   char type;
 };
 
@@ -1247,12 +1273,8 @@ static LONGEST get_timestamp (void);
 /* Record that an error occurred during expression evaluation.  */
 
 static void
-record_tracepoint_error (struct tracepoint *tpoint, const char *which,
-			 enum eval_result_type rtype)
+record_expr_eval_error (enum eval_result_type rtype)
 {
-  trace_debug ("Tracepoint %d at %s %s eval reports error %d",
-	       tpoint->number, paddress (tpoint->address), which, rtype);
-
 #ifdef IN_PROCESS_AGENT
   /* Only record the first error we get.  */
   if (cmpxchg (&expr_eval_result,
@@ -1263,8 +1285,6 @@ record_tracepoint_error (struct tracepoint *tpoint, const char *which,
   if (expr_eval_result != expr_eval_no_error)
     return;
 #endif
-
-  error_tracepoint = tpoint;
 }
 
 /* Trace buffer management.  */
@@ -1603,6 +1623,298 @@ trace_buffer_alloc (size_t amt)
 }
 
 #ifndef IN_PROCESS_AGENT
+static char*
+m_tracepoint_action_init (char *packet, struct tracepoint_action* action)
+{
+  ULONGEST basereg;
+  int is_neg;
+  struct collect_memory_action *maction
+    = (struct collect_memory_action *) action;
+
+  maction->base.type = *packet;
+  ++packet;
+
+  is_neg = (*packet == '-');
+  if (*packet == '-')
+    ++packet;
+  packet = unpack_varlen_hex (packet, &basereg);
+  ++packet;
+  packet = unpack_varlen_hex (packet, &maction->addr);
+  ++packet;
+  packet = unpack_varlen_hex (packet, &maction->len);
+  maction->basereg = (is_neg ? - (int) basereg : (int) basereg);
+  trace_debug ("Want to collect %s bytes at 0x%s (basereg %d)",
+	       pulongest (maction->len),
+	       paddress (maction->addr), maction->basereg);
+
+  return packet;
+}
+
+static CORE_ADDR
+m_tracepoint_action_download (struct tracepoint_action *action)
+{
+  CORE_ADDR ipa_action = target_malloc (sizeof (struct collect_memory_action));
+
+  write_inferior_memory (ipa_action, (unsigned char *) action,
+			 sizeof (struct collect_memory_action));
+  /* Write NULL to field `ops'.  */
+  write_inferior_data_ptr(ipa_action + offsetof (struct tracepoint_action, ops),
+			  0);
+
+  return ipa_action;
+}
+#endif /* !IN_PROCESS_AGENT */
+
+static int
+m_tracepoint_action_execute (struct tracepoint_action *action,
+			     struct tracepoint_hit_ctx *ctx,
+			     struct traceframe *tframe)
+{
+  struct collect_memory_action *maction;
+
+  maction = (struct collect_memory_action *) action;
+
+  trace_debug ("Want to collect %s bytes at 0x%s (basereg %d)",
+	       pulongest (maction->len), paddress (maction->addr),
+	       maction->basereg);
+  /* (should use basereg) */
+  agent_mem_read (tframe, NULL,
+		  (CORE_ADDR) maction->addr, maction->len);
+
+  return 0;
+}
+
+static struct tracepoint_action_ops m_tracepoint_action_ops =
+{
+#ifndef IN_PROCESS_AGENT
+  m_tracepoint_action_init,
+  m_tracepoint_action_download,
+#endif
+  m_tracepoint_action_execute,
+};
+
+#ifndef IN_PROCESS_AGENT
+static char*
+r_tracepoint_action_init (char * packet, struct tracepoint_action* action)
+{
+  action->type = *packet;
+  ++packet;
+
+  trace_debug ("Want to collect registers");
+
+  /* skip past hex digits of mask for now */
+  while (isxdigit(*packet))
+    ++packet;
+
+  return packet;
+}
+
+static CORE_ADDR
+r_tracepoint_action_download (struct tracepoint_action *action)
+{
+  CORE_ADDR ipa_action
+    = target_malloc (sizeof (struct collect_registers_action));
+
+  write_inferior_memory (ipa_action, (unsigned char *) action,
+			 sizeof (struct collect_registers_action));
+  /* Write NULL to field `ops'.  */
+  write_inferior_data_ptr(ipa_action + offsetof (struct tracepoint_action, ops),
+			  0);
+
+  return ipa_action;
+}
+#endif /* !IN_PROCESS_AGENT */
+
+static unsigned char* add_traceframe_block (struct traceframe *tframe, int amt);
+static struct regcache* get_context_regcache (struct tracepoint_hit_ctx *ctx);
+
+static int
+r_tracepoint_action_execute (struct tracepoint_action *action,
+			     struct tracepoint_hit_ctx *ctx,
+			     struct traceframe *tframe)
+{
+  unsigned char *regspace;
+  struct regcache tregcache;
+  struct regcache *context_regcache;
+
+  trace_debug ("Want to collect registers");
+
+  /* Collect all registers for now.  */
+  regspace = add_traceframe_block (tframe, 1 + register_cache_size ());
+  if (regspace == NULL)
+    {
+      trace_debug ("Trace buffer block allocation failed, skipping");
+      return 1;
+    }
+  /* Identify a register block.  */
+  *regspace = 'R';
+
+  context_regcache = get_context_regcache (ctx);
+
+  /* Wrap the regblock in a register cache (in the stack, we don't want
+     to malloc here).  */
+  init_register_cache (&tregcache, regspace + 1);
+
+  /* Copy the register data to the regblock.  */
+  regcache_cpy (&tregcache, context_regcache);
+
+#ifndef IN_PROCESS_AGENT
+  /* On some platforms, trap-based tracepoints will have the PC pointing
+     to the next instruction after the trap, but we don't want the user
+     or GDB trying to guess whether the saved PC needs adjusting; so
+     always record the adjusted stop_pc.  Note that we can't use
+     tpoint->address instead, since it will be wrong for while-stepping
+     actions.  This adjustment is a nop for fast tracepoints collected from
+     the in-process lib (but not if GDBserver is collecting one preemptively),
+     since the PC had already been adjusted to contain the tracepoint's
+     address by the jump pad.  */
+  trace_debug ("Storing stop pc (0x%s) in regblock",
+	       paddress (ctx->stop_pc));
+
+  /* This changes the regblock, not the thread's regcache.  */
+  regcache_write_pc (&tregcache, ctx->stop_pc);
+#endif
+
+  return 0;
+}
+
+static struct tracepoint_action_ops r_tracepoint_action_ops =
+{
+#ifndef IN_PROCESS_AGENT
+  r_tracepoint_action_init,
+  r_tracepoint_action_download,
+#endif
+  r_tracepoint_action_execute,
+};
+
+#ifndef IN_PROCESS_AGENT
+static char*
+x_tracepoint_action_init (char *packet, struct tracepoint_action* action)
+{
+  action->type = *packet;
+
+  trace_debug ("Want to evaluate expression");
+  ((struct eval_expr_action *) action)->expr = gdb_parse_agent_expr (&packet);
+
+  return packet;
+}
+
+static CORE_ADDR download_agent_expr (struct agent_expr *expr);
+
+static CORE_ADDR
+x_tracepoint_action_download (struct tracepoint_action *action)
+{
+  CORE_ADDR ipa_action = target_malloc (sizeof (struct eval_expr_action));
+  CORE_ADDR expr;
+
+  write_inferior_memory (ipa_action, (unsigned char *) action,
+			 sizeof (struct eval_expr_action));
+  expr = download_agent_expr (((struct eval_expr_action *)action)->expr);
+  write_inferior_data_ptr(ipa_action + offsetof (struct eval_expr_action, expr),
+			  expr);
+  /* Write NULL to field `ops'.  */
+  write_inferior_data_ptr(ipa_action + offsetof (struct tracepoint_action, ops),
+			  0);
+
+  return ipa_action;
+}
+#endif /* !IN_PROCESS_AGENT */
+
+static void record_expr_eval_error (enum eval_result_type rtype);
+
+static int
+x_tracepoint_action_execute (struct tracepoint_action *action,
+			     struct tracepoint_hit_ctx *ctx,
+			     struct traceframe *tframe)
+{
+  struct eval_expr_action *eaction = (struct eval_expr_action *) action;
+  enum eval_result_type err;
+
+  trace_debug ("Want to evaluate expression");
+
+  err = eval_tracepoint_agent_expr (ctx, tframe, eaction->expr, NULL);
+
+  if (err != expr_eval_no_error)
+    {
+      trace_debug ("Tracepoint at %s action expression eval reports error %d",
+		   paddress (ctx->stop_pc), err);
+
+      record_expr_eval_error (err);
+
+      return 1;
+    }
+
+  return 0;
+}
+
+static struct tracepoint_action_ops x_tracepoint_action_ops =
+{
+#ifndef IN_PROCESS_AGENT
+  x_tracepoint_action_init,
+  x_tracepoint_action_download,
+#endif
+  x_tracepoint_action_execute,
+};
+
+#ifndef IN_PROCESS_AGENT
+static char*
+l_tracepoint_action_init (char *packet, struct tracepoint_action* action)
+{
+  action->type = *packet;
+  packet++;
+
+  trace_debug ("Want to collect static trace data");
+
+  return packet;
+}
+
+static CORE_ADDR
+l_tracepoint_action_download (struct tracepoint_action *action)
+{
+  CORE_ADDR ipa_action
+    = target_malloc (sizeof (struct  collect_static_trace_data_action));
+
+  write_inferior_memory (ipa_action, (unsigned char *) action,
+			 sizeof (struct  collect_static_trace_data_action));
+  /* Write NULL to field `ops'.  */
+  write_inferior_data_ptr(ipa_action + offsetof (struct tracepoint_action, ops),
+			  0);
+
+  return ipa_action;
+}
+#endif /* !IN_PROCESS_AGENT */
+
+#if defined IN_PROCESS_AGENT && defined HAVE_UST
+static void collect_ust_data_at_tracepoint (struct tracepoint_hit_ctx *ctx,
+					    struct traceframe *tframe);
+#endif
+
+static int
+l_tracepoint_action_execute (struct tracepoint_action *action,
+			     struct tracepoint_hit_ctx *ctx,
+			     struct traceframe *tframe)
+{
+#if defined IN_PROCESS_AGENT && defined HAVE_UST
+  trace_debug ("Want to collect static trace data");
+  collect_ust_data_at_tracepoint (ctx, tframe);
+  return 0;
+#else
+  trace_debug ("warning: collecting static trace data, "
+	       "but static tracepoints are not supported");
+  return 1;
+#endif
+}
+
+static struct tracepoint_action_ops l_tracepoint_action_ops =
+{
+#ifndef IN_PROCESS_AGENT
+  l_tracepoint_action_init,
+  l_tracepoint_action_download,
+#endif
+  l_tracepoint_action_execute,
+};
+
+#ifndef IN_PROCESS_AGENT
 
 /* Return the total free space.  This is not necessarily the largest
    block we can allocate, because of the two-part case.  */
@@ -1755,56 +2067,23 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet)
 	{
 	case 'M':
 	  {
-	    struct collect_memory_action *maction;
-	    ULONGEST basereg;
-	    int is_neg;
-
-	    maction = xmalloc (sizeof *maction);
-	    maction->base.type = *act;
-	    action = &maction->base;
-
-	    ++act;
-	    is_neg = (*act == '-');
-	    if (*act == '-')
-	      ++act;
-	    act = unpack_varlen_hex (act, &basereg);
-	    ++act;
-	    act = unpack_varlen_hex (act, &maction->addr);
-	    ++act;
-	    act = unpack_varlen_hex (act, &maction->len);
-	    maction->basereg = (is_neg
-				? - (int) basereg
-				: (int) basereg);
-	    trace_debug ("Want to collect %s bytes at 0x%s (basereg %d)",
-			 pulongest (maction->len),
-			 paddress (maction->addr), maction->basereg);
+	    action = xmalloc (sizeof (struct collect_memory_action));
+	    action->ops = &m_tracepoint_action_ops;
+
 	    break;
 	  }
 	case 'R':
 	  {
-	    struct collect_registers_action *raction;
-
-	    raction = xmalloc (sizeof *raction);
-	    raction->base.type = *act;
-	    action = &raction->base;
+	    action = xmalloc (sizeof (struct collect_registers_action));
+	    action->ops = &r_tracepoint_action_ops;
 
-	    trace_debug ("Want to collect registers");
-	    ++act;
-	    /* skip past hex digits of mask for now */
-	    while (isxdigit(*act))
-	      ++act;
 	    break;
 	  }
 	case 'L':
 	  {
-	    struct collect_static_trace_data_action *raction;
+	    action = xmalloc (sizeof (struct collect_static_trace_data_action));
+	    action->ops = &l_tracepoint_action_ops;
 
-	    raction = xmalloc (sizeof *raction);
-	    raction->base.type = *act;
-	    action = &raction->base;
-
-	    trace_debug ("Want to collect static trace data");
-	    ++act;
 	    break;
 	  }
 	case 'S':
@@ -1813,14 +2092,8 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet)
 	  break;
 	case 'X':
 	  {
-	    struct eval_expr_action *xaction;
-
-	    xaction = xmalloc (sizeof (*xaction));
-	    xaction->base.type = *act;
-	    action = &xaction->base;
-
-	    trace_debug ("Want to evaluate expression");
-	    xaction->expr = gdb_parse_agent_expr (&act);
+	    action = xmalloc (sizeof (struct eval_expr_action));
+	    action->ops = &x_tracepoint_action_ops;
 	    break;
 	  }
 	default:
@@ -1833,6 +2106,8 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet)
       if (action == NULL)
 	break;
 
+      act = action->ops->init (act, action);
+
       if (seen_step_action_flag)
 	{
 	  tpoint->num_step_actions++;
@@ -4263,8 +4538,6 @@ tracepoint_was_hit (struct thread_info *tinfo, CORE_ADDR stop_pc)
 
 #if defined IN_PROCESS_AGENT && defined HAVE_UST
 struct ust_marker_data;
-static void collect_ust_data_at_tracepoint (struct tracepoint_hit_ctx *ctx,
-					    struct traceframe *tframe);
 #endif
 
 /* Create a trace frame for the hit of the given tracepoint in the
@@ -4412,105 +4685,31 @@ do_action_at_tracepoint (struct tracepoint_hit_ctx *ctx,
 			 struct traceframe *tframe,
 			 struct tracepoint_action *taction)
 {
-  enum eval_result_type err;
-
-  switch (taction->type)
+#ifdef IN_PROCESS_AGENT
+  if (taction->ops == NULL)
     {
-    case 'M':
-      {
-	struct collect_memory_action *maction;
-
-	maction = (struct collect_memory_action *) taction;
-
-	trace_debug ("Want to collect %s bytes at 0x%s (basereg %d)",
-		     pulongest (maction->len),
-		     paddress (maction->addr), maction->basereg);
-	/* (should use basereg) */
-	agent_mem_read (tframe, NULL,
-			(CORE_ADDR) maction->addr, maction->len);
-	break;
-      }
-    case 'R':
-      {
-	unsigned char *regspace;
-	struct regcache tregcache;
-	struct regcache *context_regcache;
-
-
-	trace_debug ("Want to collect registers");
-
-	/* Collect all registers for now.  */
-	regspace = add_traceframe_block (tframe,
-					 1 + register_cache_size ());
-	if (regspace == NULL)
-	  {
-	    trace_debug ("Trace buffer block allocation failed, skipping");
-	    break;
-	  }
-	/* Identify a register block.  */
-	*regspace = 'R';
-
-	context_regcache = get_context_regcache (ctx);
-
-	/* Wrap the regblock in a register cache (in the stack, we
-	   don't want to malloc here).  */
-	init_register_cache (&tregcache, regspace + 1);
-
-	/* Copy the register data to the regblock.  */
-	regcache_cpy (&tregcache, context_regcache);
-
-#ifndef IN_PROCESS_AGENT
-	/* On some platforms, trap-based tracepoints will have the PC
-	   pointing to the next instruction after the trap, but we
-	   don't want the user or GDB trying to guess whether the
-	   saved PC needs adjusting; so always record the adjusted
-	   stop_pc.  Note that we can't use tpoint->address instead,
-	   since it will be wrong for while-stepping actions.  This
-	   adjustment is a nop for fast tracepoints collected from the
-	   in-process lib (but not if GDBserver is collecting one
-	   preemptively), since the PC had already been adjusted to
-	   contain the tracepoint's address by the jump pad.  */
-	trace_debug ("Storing stop pc (0x%s) in regblock",
-		     paddress (ctx->stop_pc));
-
-	/* This changes the regblock, not the thread's
-	   regcache.  */
-	regcache_write_pc (&tregcache, ctx->stop_pc);
+      switch (taction->type)
+	{
+	case 'M':
+	  taction->ops = &m_tracepoint_action_ops;
+	  break;
+	case 'X':
+	  taction->ops = &x_tracepoint_action_ops;
+	  break;
+	case 'R':
+	  taction->ops = &r_tracepoint_action_ops;
+	  break;
+	case 'L':
+	  taction->ops = &l_tracepoint_action_ops;
+	  break;
+	default:
+	  return;
+	};
+    }
 #endif
-      }
-      break;
-    case 'X':
-      {
-	struct eval_expr_action *eaction;
-
-	eaction = (struct eval_expr_action *) taction;
 
-	trace_debug ("Want to evaluate expression");
-
-	err = eval_tracepoint_agent_expr (ctx, tframe, eaction->expr, NULL);
-
-	if (err != expr_eval_no_error)
-	  {
-	    record_tracepoint_error (tpoint, "action expression", err);
-	    return;
-	  }
-      }
-      break;
-    case 'L':
-      {
-#if defined IN_PROCESS_AGENT && defined HAVE_UST
-	trace_debug ("Want to collect static trace data");
-	collect_ust_data_at_tracepoint (ctx, tframe);
-#else
-	trace_debug ("warning: collecting static trace data, "
-		     "but static tracepoints are not supported");
-#endif
-      }
-      break;
-    default:
-      trace_debug ("unknown trace action '%c', ignoring", taction->type);
-      break;
-    }
+  if (taction->ops->execute (taction, ctx, tframe))
+    error_tracepoint = tpoint;
 }
 
 static int
@@ -4543,7 +4742,11 @@ condition_true_at_tracepoint (struct tracepoint_hit_ctx *ctx,
 
   if (err != expr_eval_no_error)
     {
-      record_tracepoint_error (tpoint, "condition", err);
+      trace_debug ("Tracepoint %d at %s condition eval reports error %d",
+		   tpoint->number, paddress (tpoint->address), err);
+
+      record_expr_eval_error (err);
+      error_tracepoint = tpoint;
       /* The error case must return false.  */
       return 0;
     }
@@ -5666,55 +5869,8 @@ download_tracepoint_1 (struct tracepoint *tpoint)
       /* Now for each pointer, download the action.  */
       for (i = 0; i < tpoint->numactions; i++)
 	{
-	  CORE_ADDR ipa_action = 0;
 	  struct tracepoint_action *action = tpoint->actions[i];
-
-	  switch (action->type)
-	    {
-	    case 'M':
-	      ipa_action
-		= target_malloc (sizeof (struct collect_memory_action));
-	      write_inferior_memory (ipa_action,
-				     (unsigned char *) action,
-				     sizeof (struct collect_memory_action));
-	      break;
-	    case 'R':
-	      ipa_action
-		= target_malloc (sizeof (struct collect_registers_action));
-	      write_inferior_memory (ipa_action,
-				     (unsigned char *) action,
-				     sizeof (struct collect_registers_action));
-	      break;
-	    case 'X':
-	      {
-		CORE_ADDR expr;
-		struct eval_expr_action *eaction
-		  = (struct eval_expr_action *) action;
-
-		ipa_action = target_malloc (sizeof (*eaction));
-		write_inferior_memory (ipa_action,
-				       (unsigned char *) eaction,
-				       sizeof (*eaction));
-
-		expr = download_agent_expr (eaction->expr);
-		write_inferior_data_ptr
-		  (ipa_action + offsetof (struct eval_expr_action, expr),
-		   expr);
-		break;
-	      }
-	    case 'L':
-	      ipa_action = target_malloc
-		(sizeof (struct collect_static_trace_data_action));
-	      write_inferior_memory
-		(ipa_action,
-		 (unsigned char *) action,
-		 sizeof (struct collect_static_trace_data_action));
-	      break;
-	    default:
-	      trace_debug ("unknown trace action '%c', ignoring",
-			   action->type);
-	      break;
-	    }
+	  CORE_ADDR ipa_action = action->ops->download (action);
 
 	  if (ipa_action != 0)
 	    write_inferior_data_ptr
-- 
1.7.0.4


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