* [PATCH v3 2/4] tracepoint multithread and multiprocess support (gdbserver)
@ 2013-12-31 3:30 Hui Zhu
0 siblings, 0 replies; only message in thread
From: Hui Zhu @ 2013-12-31 3:30 UTC (permalink / raw)
To: gdb-patches ml; +Cc: Pedro Alves, lgustavo
This patch is for the gdbserver.
Following part is the Pedro's comment and my reply:
>> 2013-12-19 Hui Zhu <teawater@gmail.com>
>>
>> * server.c (handle_query): Send ";MultiProcessTracepoint+".
>> * tracepoint.c (tracepoint): Add ptid.
>
> * tracepoint.c (tracepoint) <ptid>: New field.
>
>
>> (add_tracepoint): Initialize ptid.
>
> (add_tracepoint): Initialize ptid field.
>
>> (cmd_qtdp): Handle 'P'.
>> (tracepoint_was_hit): Add check if tpoint->ptid is same with
>> minus_one_ptid or tinfo->entry.id.
>
> (tracepoint_was_hit): Only collect if tpoint->ptid is
> minus_one_ptid or tinfo->entry.id.
>
Thanks. All of them were fixed.
>>
>> --- a/gdb/gdbserver/server.c
>> +++ b/gdb/gdbserver/server.c
>> @@ -1804,6 +1804,7 @@ handle_query (char *own_buf, int packet_
>> strcat (own_buf, ";EnableDisableTracepoints+");
>> strcat (own_buf, ";QTBuffer:size+");
>> strcat (own_buf, ";tracenz+");
>> + strcat (own_buf, ";MultiProcessTracepoint+");
>> }
>>
>> /* Support target-side breakpoint conditions and commands. */
>> --- a/gdb/gdbserver/tracepoint.c
>> +++ b/gdb/gdbserver/tracepoint.c
>> @@ -766,6 +766,8 @@ struct tracepoint
>>
>> CORE_ADDR compiled_cond;
>>
>> + ptid_t ptid;
>
> This needs a comment.
Changed this part to:
/* This tracepoint just be triggered by this ptid. */
ptid_t ptid;
>
>> +
>> /* Link to the next tracepoint in the list. */
>> struct tracepoint *next;
>>
>> @@ -1822,6 +1824,7 @@ add_tracepoint (int num, CORE_ADDR addr)
>> tpoint->source_strings = NULL;
>> tpoint->compiled_cond = 0;
>> tpoint->handle = NULL;
>> + tpoint->ptid = minus_one_ptid;
>> tpoint->next = NULL;
>>
>> /* Find a place to insert this tracepoint into list in order to keep
>> @@ -2551,6 +2554,29 @@ cmd_qtdp (char *own_buf)
>> tpoint->cond = gdb_parse_agent_expr (&actparm);
>> packet = actparm;
>> }
>> + else if (*packet == 'P')
>> + {
>> + ++packet;
>> + tpoint->ptid = read_ptid (packet, &packet);
>> +
>> + /* Check if this tracepoint is for current process. */
>> + if (ptid_get_pid (current_ptid)
>> + != ptid_get_pid (tpoint->ptid))
>> + {
>> + trace_debug ("\
>> +Tracepoint error: tracepoint %d is not for current process", (int) num);
>
> The number alone is not sufficient to identify the tracepoint.
>
Changed this part to:
trace_debug ("\
Tracepoint error: tracepoint %d at 0x%s is not for current process",
(int) num, paddress (addr));
>
>> + write_enn (own_buf);
>> + return;
>> + }
>> + if (ptid_get_lwp (tpoint->ptid) == 0
>> + && ptid_get_tid (tpoint->ptid) == 0)
>> + {
>> + /* This tracepoint is OK for all the thread of current
>> + process, set its ptid to minus_one_ptid to make its
>> + ptid is not checked before trigger this tracepoint. */
>> + tpoint->ptid = minus_one_ptid;
>
> I don't think this is right. GDBserver might be debugging multiple
> processes, with only one of the processes tracing. And ...
I removed this part of code and changed add_tracepoint's tpoint->ptid
initialize code to:
ptid_build (ptid_get_pid (current_ptid), 0, 0);
Then if a tracepoint doesn't for a special thread, it will has pid of
current inferior.
>
>> + }
>> + }
>> else if (*packet == '-')
>> break;
>> else if (*packet == '\0')
>> @@ -4562,10 +4588,14 @@ tracepoint_was_hit (struct thread_info *
>> target_pid_to_str (tinfo->entry.id),
>> tpoint->number, paddress (tpoint->address));
>>
>> - /* Test the condition if present, and collect if true. */
>> - if (!tpoint->cond
>> - || (condition_true_at_tracepoint
>> - ((struct tracepoint_hit_ctx *) &ctx, tpoint)))
>> + /* Check if tpoint->ptid is same with minus_one_ptid or
>> + tinfo->entry.id, test the condition if present,
>> + and collect if all true. */
>> + if ((ptid_equal (minus_one_ptid, tpoint->ptid)
>> + || ptid_equal (tinfo->entry.id, tpoint->ptid))
>
> Then if some other process happens to trigger an unrelated
> breakpoint at exactly the same address a tracepoint is
> set, then this will think a tracepoint was triggered.
> So I think you should leave the tracepoint's ptid as
> GDB sent, and then use ptid_match here.
>
I change this part of code to:
/* If pid of tpoint->ptid is available only and it is same with
pid of tinfo->entry.id, or tpoint->ptid tpoint->ptid is same
with tinfo->entry.id, this tracepoint is for this thread.
Then test the condition if present, and collect if all true. */
if (((!ptid_lwp_p (tpoint->ptid) && !ptid_tid_p (tpoint->ptid)
&& ptid_get_pid (tinfo->entry.id) == ptid_get_pid (tpoint->ptid))
|| ptid_equal (tinfo->entry.id, tpoint->ptid))
&& (!tpoint->cond
|| (condition_true_at_tracepoint
((struct tracepoint_hit_ctx *) &ctx, tpoint))))
collect_data_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx,
stop_pc, tpoint);
I didn't use ptid_match because it cannot handle the status that pid of
tpoint->ptid is available only.
Following part is the introduce of this patch:
This patch make gdbserver support multiprocess tracepoint and multithread
tracepoint, but not multiprocess tracepoint together.
It will send ";MultiThreadTracepoint+" back to GDB if this gdbserver
support tracepoint.
And add a ptid to "struct tracepoint", a tracepoint just can be triggered
by a task that ptid is same with this ptid.
When cmd_qtdp got a "QTDP" packets:
1. If this function call add_tracepoint to add a tracepoint, ptid of
tracepint will be initialized for current inferior. Because in default,
gdbserver need set this tracepoint for current inferior.
2. If got P@var{thread-id} Get ptid from thread-id.
3. If this ptid's pid is not same with current process, send exx packets
back to GDB.
4. Save this ptid to tracepoint.
Before GDB trigger a tracepoint, it will check if its ptid is same with
tinfo->entry.id. If not, doesn't trigger it.
Please help me review it.
Thanks,
Hui
2013-12-31 Hui Zhu <hui@codesourcery.com>
* server.c (handle_query): Send ";MultiProcessTracepoint+".
* tracepoint.c (tracepoint) <ptid>: New field.
(add_tracepoint): Initialize ptid field.
(cmd_qtdp): Handle 'P'.
(tracepoint_was_hit): If pid of tpoint->ptid is available only
and it is same with pid of tinfo->entry.id, or tpoint->ptid is
same with tinfo->entry.id, this tracepoint is for this thread.
Then test the condition if present, and collect if all true.
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1804,6 +1804,7 @@ handle_query (char *own_buf, int packet_
strcat (own_buf, ";EnableDisableTracepoints+");
strcat (own_buf, ";QTBuffer:size+");
strcat (own_buf, ";tracenz+");
+ strcat (own_buf, ";MultiThreadTracepoint+");
}
/* Support target-side breakpoint conditions and commands. */
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -766,6 +766,9 @@ struct tracepoint
CORE_ADDR compiled_cond;
+ /* This tracepoint just be triggered by this ptid. */
+ ptid_t ptid;
+
/* Link to the next tracepoint in the list. */
struct tracepoint *next;
@@ -1822,6 +1825,7 @@ add_tracepoint (int num, CORE_ADDR addr)
tpoint->source_strings = NULL;
tpoint->compiled_cond = 0;
tpoint->handle = NULL;
+ tpoint->ptid = ptid_build (ptid_get_pid (current_ptid), 0, 0);
tpoint->next = NULL;
/* Find a place to insert this tracepoint into list in order to keep
@@ -2551,6 +2555,22 @@ cmd_qtdp (char *own_buf)
tpoint->cond = gdb_parse_agent_expr (&actparm);
packet = actparm;
}
+ else if (*packet == 'P')
+ {
+ ++packet;
+ tpoint->ptid = read_ptid (packet, &packet);
+
+ /* Check if this tracepoint is for current process. */
+ if (ptid_get_pid (current_ptid)
+ != ptid_get_pid (tpoint->ptid))
+ {
+ trace_debug ("\
+Tracepoint error: tracepoint %d at 0x%s is not for current process",
+ (int) num, paddress (addr));
+ write_enn (own_buf);
+ return;
+ }
+ }
else if (*packet == '-')
break;
else if (*packet == '\0')
@@ -4562,10 +4582,16 @@ tracepoint_was_hit (struct thread_info *
target_pid_to_str (tinfo->entry.id),
tpoint->number, paddress (tpoint->address));
- /* Test the condition if present, and collect if true. */
- if (!tpoint->cond
- || (condition_true_at_tracepoint
- ((struct tracepoint_hit_ctx *) &ctx, tpoint)))
+ /* If pid of tpoint->ptid is available only and it is same with
+ pid of tinfo->entry.id, or tpoint->ptid is same with
+ tinfo->entry.id, this tracepoint is for this thread.
+ Then test the condition if present, and collect if all true. */
+ if (((!ptid_lwp_p (tpoint->ptid) && !ptid_tid_p (tpoint->ptid)
+ && ptid_get_pid (tinfo->entry.id) == ptid_get_pid (tpoint->ptid))
+ || ptid_equal (tinfo->entry.id, tpoint->ptid))
+ && (!tpoint->cond
+ || (condition_true_at_tracepoint
+ ((struct tracepoint_hit_ctx *) &ctx, tpoint))))
collect_data_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx,
stop_pc, tpoint);
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2013-12-31 3:30 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-31 3:30 [PATCH v3 2/4] tracepoint multithread and multiprocess support (gdbserver) Hui Zhu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).