This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH v3 2/4] tracepoint multithread and multiprocess support (gdbserver)
- From: Hui Zhu <hui_zhu at mentor dot com>
- To: gdb-patches ml <gdb-patches at sourceware dot org>
- Cc: Pedro Alves <palves at redhat dot com>, <lgustavo at codesourcery dot com>
- Date: Tue, 31 Dec 2013 11:29:44 +0800
- Subject: [PATCH v3 2/4] tracepoint multithread and multiprocess support (gdbserver)
- Authentication-results: sourceware.org; auth=none
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);