[patch] Fix a double collect of static tracepoint

[patch] Fix a double collect of static tracepoint

[Yao Qi]

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

Hi,
This patch fixes a problem that static tracepoint's collect is performed
twice (one from gdbserver and the other one from IPA), when there is
another regular tracepoint setting at the same address.

When program hits a trap, gdbserver will iterate all tracepoints (in
tracepoint.c:tracepoint_was_hit) and call collect_data_at_tracepoint if
condition is null or result is true. In this iteration, static
tracepoint is in this list, so gdbserver will collect for this static
tracepoint, even the trap is caused by another regular tracepoint, which
is set at the same address.  The collect of static tracepoint is
performed for the first time.

Then, gdbserver resumes program, and UST marker/probe is executed.  In
gdb_probe, the collect of static tracepoint is performed for the 2nd
time.  So we have one static tracepoint, "hit" once, but get two trace
frames finally.

This patch is to fix this problem described above.  I'll send out
another patch for test case, and this patch fixes one fail in it.

-- 
Yao (齐尧)

[-- Attachment #2: 0007-Don-t-collect-for-static-tracepoint.patch --]
[-- Type: text/x-patch, Size: 1143 bytes --]


2011-12-20  Yao Qi  <yao@codesourcery.com>

	* tracepoint.c (tracepoint_was_hit): Don't collect for
	static tracepoint.
---
 gdb/gdbserver/tracepoint.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index ac07db9..1cc4b4b 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -4276,8 +4276,12 @@ tracepoint_was_hit (struct thread_info *tinfo, CORE_ADDR stop_pc)
     {
       /* Note that we collect fast tracepoints here as well.  We'll
 	 step over the fast tracepoint jump later, which avoids the
-	 double collect.  */
-      if (tpoint->enabled && stop_pc == tpoint->address)
+	 double collect.  However, we don't collect for static
+	 tracepoints here, because UST markers are compiled in program,
+	 and probes will be executed in program.  So static tracepoints
+	 are collected in there.   */
+      if (tpoint->enabled && stop_pc == tpoint->address
+	  && tpoint->type != static_tracepoint)
 	{
 	  trace_debug ("Thread %s at address of tracepoint %d at 0x%s",
 		       target_pid_to_str (tinfo->entry.id),
-- 
1.7.0.4

[ping]: [patch] Fix a double collect of static tracepoint

[Yao Qi]

On 12/21/2011 12:29 AM, Yao Qi wrote:
> 2011-12-20  Yao Qi  <yao@codesourcery.com>
> 
> 	* tracepoint.c (tracepoint_was_hit): Don't collect for
> 	static tracepoint.
Ping.  http://sourceware.org/ml/gdb-patches/2011-12/msg00682.html

-- 
Yao (齐尧)

Re: [patch] Fix a double collect of static tracepoint

[Pedro Alves]

On 12/20/2011 04:29 PM, Yao Qi wrote:
> Hi,
> This patch fixes a problem that static tracepoint's collect is performed
> twice (one from gdbserver and the other one from IPA), when there is
> another regular tracepoint setting at the same address.
>
> When program hits a trap, gdbserver will iterate all tracepoints (in
> tracepoint.c:tracepoint_was_hit) and call collect_data_at_tracepoint if
> condition is null or result is true. In this iteration, static
> tracepoint is in this list, so gdbserver will collect for this static
> tracepoint, even the trap is caused by another regular tracepoint, which
> is set at the same address.  The collect of static tracepoint is
> performed for the first time.
>
> Then, gdbserver resumes program, and UST marker/probe is executed.  In
> gdb_probe, the collect of static tracepoint is performed for the 2nd
> time.  So we have one static tracepoint, "hit" once, but get two trace
> frames finally.
>
> This patch is to fix this problem described above.  I'll send out
> another patch for test case, and this patch fixes one fail in it.

Yeah.  We instead step over fast tracepoints to solve this problem,
to give a consistent view of when a tracepoint is hit to the user.
Although it would be possible to step over static tracepoints
similarly, it wouldn't work, because of the possibility of a
"collect $_sdata" action, which can only be done by the
inferior.  IOW, if the inferior stops at a static tracepoint's
address today, one with a "collect $_sdata", and you do "tdump",
you'll see that $_sdata will be missing or unavailable, or maybe even
an error.

So, okay.  Thanks.

>
> -- Yao (齐尧)
>
>
> 0007-Don-t-collect-for-static-tracepoint.patch
>
>
> 2011-12-20  Yao Qi<yao@codesourcery.com>
>
> 	* tracepoint.c (tracepoint_was_hit): Don't collect for
> 	static tracepoint.
> ---
>   gdb/gdbserver/tracepoint.c |    8 ++++++--
>   1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
> index ac07db9..1cc4b4b 100644
> --- a/gdb/gdbserver/tracepoint.c
> +++ b/gdb/gdbserver/tracepoint.c
> @@ -4276,8 +4276,12 @@ tracepoint_was_hit (struct thread_info *tinfo, CORE_ADDR stop_pc)
>       {
>         /* Note that we collect fast tracepoints here as well.  We'll
>   	 step over the fast tracepoint jump later, which avoids the
> -	 double collect.  */
> -      if (tpoint->enabled&&  stop_pc == tpoint->address)
> +	 double collect.  However, we don't collect for static
> +	 tracepoints here, because UST markers are compiled in program,
> +	 and probes will be executed in program.  So static tracepoints
> +	 are collected in there.   */
> +      if (tpoint->enabled&&  stop_pc == tpoint->address
> +	&&  tpoint->type != static_tracepoint)
>   	{
>   	  trace_debug ("Thread %s at address of tracepoint %d at 0x%s",
>   		       target_pid_to_str (tinfo->entry.id),


-- 
Pedro Alves