[patch/gdbserver] Fix a bug when setting two fast tracepoints at the same place

[Yao Qi <yao@codesourcery.com>]

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

Some days ago, I sent a test case (trace-break.exp) and a fix about
setting breakpoint and fast tracepoint at the same the same address,

  [patch, gdbserver] Uninsert bpkt when regular and fast tracepoint are
set at the same address
  http://sourceware.org/ml/gdb-patches/2011-10/msg00714.html

I read trace-break.exp again, and find we didn't test setting two fast
tracepoints at the same address.  This test can be done easily by adding
one line in trace-brea.exp (see below in my patch), however, it reveals
one bug about tracepoint.

  FAIL: gdb.trace/trace-break.exp: 2 ftrace ftrace on: tfind frame 1
  FAIL: gdb.trace/trace-break.exp: 2 ftrace ftrace off: tfind frame 1

The problem is that when setting two fast tracepoints at the same place,
only one works.  When setting fast tracepoint at an address, a jump pad
is created to connect a jmp insn written at tracepoint place and
collection routine.  Even multiple fast tracepoints are set at the same
address, there is only jump pad associated with this address.  Inferior
will jump to gdb_collect from jump pad with the first tracepoint
information, rather than all tracepoints information at that address.
This is cause of this problem.

Fortunately, tracepoints list are sorted, and jump pad is associated
with the "first" tracepoint of them.  So tracepoint passed to
gdb_collect is the "first" tracepoint of them which have the same
address, then we can loop over tracepoints started from "first"
tracepoint, and call collect_data_at_tracepoint in the loop.  This is
how this written.  Tested on x86_64-linux native-gdbserver, OK to apply?

-- 
Yao (齐尧)

[-- Attachment #2: 0001-two-ftrace-point-at-the-same-addr.patch --]
[-- Type: text/x-patch, Size: 4545 bytes --]


gdb/gdbserver:

2011-11-03  Yao Qi  <yao@codesourcery.com>

        * tracepoint.c (gdb_collect): Loop over TPOINT.

gdb/testsuite:

2011-11-03  Yao Qi  <yao@codesourcery.com>

        * gdb.trace/trace-break.exp: Add test on setting two
	fast tracepoints at the same address.
---
 gdb/gdbserver/tracepoint.c              |   83 ++++++++++++++++++-------------
 gdb/testsuite/gdb.trace/trace-break.exp |    1 +
 2 files changed, 50 insertions(+), 34 deletions(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 0a64104..f09a7d8 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -5417,47 +5417,62 @@ gdb_collect (struct tracepoint *tpoint, unsigned char *regs)
   if (!tracing)
     return;
 
-  if (!tpoint->enabled)
-    return;
-
   ctx.base.type = fast_tracepoint;
   ctx.regs = regs;
   ctx.regcache_initted = 0;
-  ctx.tpoint = tpoint;
 
-  /* Wrap the regblock in a register cache (in the stack, we don't
-     want to malloc here).  */
-  ctx.regspace = alloca (register_cache_size ());
-  if (ctx.regspace == NULL)
+  ctx.tpoint = tpoint;
+  for (; ctx.tpoint && ctx.tpoint->address == tpoint->address;
+       ctx.tpoint = ctx.tpoint->next)
     {
-      trace_debug ("Trace buffer block allocation failed, skipping");
-      return;
-    }
+      if (!ctx.tpoint->enabled)
+	continue;
 
-  /* Test the condition if present, and collect if true.  */
-  if (tpoint->cond == NULL
-      || condition_true_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx,
-				       tpoint))
-    {
-      collect_data_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx,
-				  tpoint->address, tpoint);
+      /* Multiple tracepoints of different types, such as fast tracepoint and
+	 static tracepoint, can be set at the same address.  */
+      if (ctx.tpoint->type != tpoint->type)
+	continue;
 
-      /* Note that this will cause original insns to be written back
-	 to where we jumped from, but that's OK because we're jumping
-	 back to the next whole instruction.  This will go badly if
-	 instruction restoration is not atomic though.  */
-      if (stopping_tracepoint
-	  || trace_buffer_is_full
-	  || expr_eval_result != expr_eval_no_error)
-	stop_tracing ();
-    }
-  else
-    {
-      /* If there was a condition and it evaluated to false, the only
-	 way we would stop tracing is if there was an error during
-	 condition expression evaluation.  */
-      if (expr_eval_result != expr_eval_no_error)
-	stop_tracing ();
+      /* Wrap the regblock in a register cache (in the stack, we don't
+	 want to malloc here).  */
+      ctx.regspace = alloca (register_cache_size ());
+      if (ctx.regspace == NULL)
+	{
+	  trace_debug ("Trace buffer block allocation failed, skipping");
+	  continue;
+	}
+
+      /* Test the condition if present, and collect if true.  */
+      if (ctx.tpoint->cond == NULL
+	  || condition_true_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx,
+					   ctx.tpoint))
+	{
+	  collect_data_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx,
+				      ctx.tpoint->address, ctx.tpoint);
+
+	  /* Note that this will cause original insns to be written back
+	     to where we jumped from, but that's OK because we're jumping
+	     back to the next whole instruction.  This will go badly if
+	     instruction restoration is not atomic though.  */
+	  if (stopping_tracepoint
+	      || trace_buffer_is_full
+	      || expr_eval_result != expr_eval_no_error)
+	    {
+	      stop_tracing ();
+	      break;
+	    }
+	}
+      else
+	{
+	  /* If there was a condition and it evaluated to false, the only
+	     way we would stop tracing is if there was an error during
+	     condition expression evaluation.  */
+	  if (expr_eval_result != expr_eval_no_error)
+	    {
+	      stop_tracing ();
+	      break;
+	    }
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.trace/trace-break.exp b/gdb/testsuite/gdb.trace/trace-break.exp
index 4a14d32..c2d7b2c 100644
--- a/gdb/testsuite/gdb.trace/trace-break.exp
+++ b/gdb/testsuite/gdb.trace/trace-break.exp
@@ -234,6 +234,7 @@ if { [gdb_test "info sharedlibrary" ".*libinproctrace\.so.*" "IPA loaded"] != 0
 	break_trace_same_addr_1 "ftrace" ${break_always_inserted}
 	break_trace_same_addr_2 "trace" "ftrace" ${break_always_inserted}
 	break_trace_same_addr_2 "ftrace" "trace" ${break_always_inserted}
+	break_trace_same_addr_2 "ftrace" "ftrace" ${break_always_inserted}
 	break_trace_same_addr_3 "ftrace" ${break_always_inserted}
 	break_trace_same_addr_4 "ftrace" ${break_always_inserted}
     }
-- 
1.7.0.4