[patch 1/8] Download tracepoint on location level

Yao Qi
2011-11-08T06:08:44Z

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

Existing gdb target hook to_download_tracepoint is about downloading
tracepoint on tracepoint level, rather than location level. However,
locations of one tracepoint may change during inferior run, so we need a
fine-granularity target hook on tracepoint locations, instead of tracepoint.

This patch is to refactor to_download_tracepoint to
to_download_tracepoint_loc. Beside this refactor, a return value is
added in new hook to reflect whether tracepoint location is
downloaded/installed successfully. Functionality of gdb is not changed.

--
Yao (é½å°§)

[-- Attachment #2: 0001-refactor-download_tracepoint-to-download_tracepoint_.patch --]
[-- Type: text/x-patch, Size: 15717 bytes --]

* target.h (struct target): Rename field to_download_tracepoint
to to_download_tracepoint_loc. Change type of parameter from
tracepoint bp_location.
* target.c (update_current_target): Update.
* tracepoint.c (start_tracing): Update.
* remote.c: Move remote_download_tracepoint. to
remote_download_tracepoint_loc. Remove loop for each location
of a tracepoint.
---
gdb/remote.c | 284 +++++++++++++++++++++++++++---------------------------
gdb/target.c | 6 +-
gdb/target.h | 12 ++-
gdb/tracepoint.c | 6 +-
4 files changed, 157 insertions(+), 151 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 5182ef1..1c43f39 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9834,10 +9834,10 @@ remote_download_command_source (int num, ULONGEST addr,
}
}

-static void
-remote_download_tracepoint (struct breakpoint *b)
+static int
+remote_download_tracepoint_loc (struct bp_location *loc)
{
- struct bp_location *loc;
+
CORE_ADDR tpaddr;
char addrbuf[40];
char buf[2048];
@@ -9848,169 +9848,167 @@ remote_download_tracepoint (struct breakpoint *b)
struct agent_expr *aexpr;
struct cleanup *aexpr_chain = NULL;
char *pkt;
+ struct breakpoint *b = loc->owner;
struct tracepoint *t = (struct tracepoint *) b;

- /* Iterate over all the tracepoint locations. It's up to the target to
- notice multiple tracepoint packets with the same number but different
- addresses, and treat them as multiple locations. */
- for (loc = b->loc; loc; loc = loc->next)
- {
- encode_actions (b, loc, &tdp_actions, &stepping_actions);
- old_chain = make_cleanup (free_actions_list_cleanup_wrapper,
- tdp_actions);
- (void) make_cleanup (free_actions_list_cleanup_wrapper,
- stepping_actions);
-
- tpaddr = loc->address;
- sprintf_vma (addrbuf, tpaddr);
- sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", b->number,
- addrbuf, /* address */
- (b->enable_state == bp_enabled ? 'E' : 'D'),
- t->step_count, t->pass_count);
- /* Fast tracepoints are mostly handled by the target, but we can
- tell the target how big of an instruction block should be moved
- around. */
- if (b->type == bp_fast_tracepoint)
+ encode_actions (loc->owner, loc, &tdp_actions, &stepping_actions);
+ old_chain = make_cleanup (free_actions_list_cleanup_wrapper,
+ tdp_actions);
+ (void) make_cleanup (free_actions_list_cleanup_wrapper,
+ stepping_actions);
+
+ tpaddr = loc->address;
+ sprintf_vma (addrbuf, tpaddr);
+ sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", b->number,
+ addrbuf, /* address */
+ (b->enable_state == bp_enabled ? 'E' : 'D'),
+ t->step_count, t->pass_count);
+ /* Fast tracepoints are mostly handled by the target, but we can
+ tell the target how big of an instruction block should be moved
+ around. */
+ if (b->type == bp_fast_tracepoint)
+ {
+ /* Only test for support at download time; we may not know
+ target capabilities at definition time. */
+ if (remote_supports_fast_tracepoints ())
{
- /* Only test for support at download time; we may not know
- target capabilities at definition time. */
- if (remote_supports_fast_tracepoints ())
- {
- int isize;
+ int isize;

- if (gdbarch_fast_tracepoint_valid_at (target_gdbarch,
- tpaddr, &isize, NULL))
- sprintf (buf + strlen (buf), ":F%x", isize);
- else
- /* If it passed validation at definition but fails now,
- something is very wrong. */
- internal_error (__FILE__, __LINE__,
- _("Fast tracepoint not "
- "valid during download"));
- }
+ if (gdbarch_fast_tracepoint_valid_at (target_gdbarch,
+ tpaddr, &isize, NULL))
+ sprintf (buf + strlen (buf), ":F%x", isize);
else
- /* Fast tracepoints are functionally identical to regular
- tracepoints, so don't take lack of support as a reason to
- give up on the trace run. */
- warning (_("Target does not support fast tracepoints, "
- "downloading %d as regular tracepoint"), b->number);
+ /* If it passed validation at definition but fails now,
+ something is very wrong. */
+ internal_error (__FILE__, __LINE__,
+ _("Fast tracepoint not "
+ "valid during download"));
}
- else if (b->type == bp_static_tracepoint)
+ else
+ /* Fast tracepoints are functionally identical to regular
+ tracepoints, so don't take lack of support as a reason to
+ give up on the trace run. */
+ warning (_("Target does not support fast tracepoints, "
+ "downloading %d as regular tracepoint"), b->number);
+ }
+ else if (b->type == bp_static_tracepoint)
+ {
+ /* Only test for support at download time; we may not know
+ target capabilities at definition time. */
+ if (remote_supports_static_tracepoints ())
{
- /* Only test for support at download time; we may not know
- target capabilities at definition time. */
- if (remote_supports_static_tracepoints ())
- {
- struct static_tracepoint_marker marker;
+ struct static_tracepoint_marker marker;

- if (target_static_tracepoint_marker_at (tpaddr, &marker))
- strcat (buf, ":S");
- else
- error (_("Static tracepoint not valid during download"));
- }
+ if (target_static_tracepoint_marker_at (tpaddr, &marker))
+ strcat (buf, ":S");
else
- /* Fast tracepoints are functionally identical to regular
- tracepoints, so don't take lack of support as a reason
- to give up on the trace run. */
- error (_("Target does not support static tracepoints"));
+ error (_("Static tracepoint not valid during download"));
}
- /* If the tracepoint has a conditional, make it into an agent
- expression and append to the definition. */
- if (loc->cond)
+ else
+ /* Fast tracepoints are functionally identical to regular
+ tracepoints, so don't take lack of support as a reason
+ to give up on the trace run. */
+ error (_("Target does not support static tracepoints"));
+ }
+ /* If the tracepoint has a conditional, make it into an agent
+ expression and append to the definition. */
+ if (loc->cond)
+ {
+ /* Only test support at download time, we may not know target
+ capabilities at definition time. */
+ if (remote_supports_cond_tracepoints ())
{
- /* Only test support at download time, we may not know target
- capabilities at definition time. */
- if (remote_supports_cond_tracepoints ())
- {
- aexpr = gen_eval_for_expr (tpaddr, loc->cond);
- aexpr_chain = make_cleanup_free_agent_expr (aexpr);
- sprintf (buf + strlen (buf), ":X%x,", aexpr->len);
- pkt = buf + strlen (buf);
- for (ndx = 0; ndx < aexpr->len; ++ndx)
- pkt = pack_hex_byte (pkt, aexpr->buf[ndx]);
- *pkt = '\0';
- do_cleanups (aexpr_chain);
- }
- else
- warning (_("Target does not support conditional tracepoints, "
- "ignoring tp %d cond"), b->number);
+ aexpr = gen_eval_for_expr (tpaddr, loc->cond);
+ aexpr_chain = make_cleanup_free_agent_expr (aexpr);
+ sprintf (buf + strlen (buf), ":X%x,", aexpr->len);
+ pkt = buf + strlen (buf);
+ for (ndx = 0; ndx < aexpr->len; ++ndx)
+ pkt = pack_hex_byte (pkt, aexpr->buf[ndx]);
+ *pkt = '\0';
+ do_cleanups (aexpr_chain);
}
+ else
+ warning (_("Target does not support conditional tracepoints, "
+ "ignoring tp %d cond"), b->number);
+ }

if (b->commands || *default_collect)
- strcat (buf, "-");
- putpkt (buf);
- remote_get_noisy_reply (&target_buf, &target_buf_size);
- if (strcmp (target_buf, "OK"))
- error (_("Target does not support tracepoints."));
+ strcat (buf, "-");
+ putpkt (buf);
+ remote_get_noisy_reply (&target_buf, &target_buf_size);
+ if (strcmp (target_buf, "OK"))
+ error (_("Target does not support tracepoints."));

- /* do_single_steps (t); */
- if (tdp_actions)
+ /* do_single_steps (t); */
+ if (tdp_actions)
+ {
+ for (ndx = 0; tdp_actions[ndx]; ndx++)
{
- for (ndx = 0; tdp_actions[ndx]; ndx++)
- {
- QUIT; /* Allow user to bail out with ^C. */
- sprintf (buf, "QTDP:-%x:%s:%s%c",
- b->number, addrbuf, /* address */
- tdp_actions[ndx],
- ((tdp_actions[ndx + 1] || stepping_actions)
- ? '-' : 0));
- putpkt (buf);
- remote_get_noisy_reply (&target_buf,
- &target_buf_size);
- if (strcmp (target_buf, "OK"))
- error (_("Error on target while setting tracepoints."));
- }
+ QUIT; /* Allow user to bail out with ^C. */
+ sprintf (buf, "QTDP:-%x:%s:%s%c",
+ b->number, addrbuf, /* address */
+ tdp_actions[ndx],
+ ((tdp_actions[ndx + 1] || stepping_actions)
+ ? '-' : 0));
+ putpkt (buf);
+ remote_get_noisy_reply (&target_buf,
+ &target_buf_size);
+ if (strcmp (target_buf, "OK"))
+ error (_("Error on target while setting tracepoints."));
}
- if (stepping_actions)
+ }
+ if (stepping_actions)
+ {
+ for (ndx = 0; stepping_actions[ndx]; ndx++)
{
- for (ndx = 0; stepping_actions[ndx]; ndx++)
- {
- QUIT; /* Allow user to bail out with ^C. */
- sprintf (buf, "QTDP:-%x:%s:%s%s%s",
- b->number, addrbuf, /* address */
- ((ndx == 0) ? "S" : ""),
- stepping_actions[ndx],
- (stepping_actions[ndx + 1] ? "-" : ""));
- putpkt (buf);
- remote_get_noisy_reply (&target_buf,
- &target_buf_size);
- if (strcmp (target_buf, "OK"))
- error (_("Error on target while setting tracepoints."));
- }
+ QUIT; /* Allow user to bail out with ^C. */
+ sprintf (buf, "QTDP:-%x:%s:%s%s%s",
+ b->number, addrbuf, /* address */
+ ((ndx == 0) ? "S" : ""),
+ stepping_actions[ndx],
+ (stepping_actions[ndx + 1] ? "-" : ""));
+ putpkt (buf);
+ remote_get_noisy_reply (&target_buf,
+ &target_buf_size);
+ if (strcmp (target_buf, "OK"))
+ error (_("Error on target while setting tracepoints."));
}
+ }

- if (remote_protocol_packets[PACKET_TracepointSource].support
- == PACKET_ENABLE)
+ if (remote_protocol_packets[PACKET_TracepointSource].support
+ == PACKET_ENABLE)
+ {
+ if (b->addr_string)
{
- if (b->addr_string)
- {
- strcpy (buf, "QTDPsrc:");
- encode_source_string (b->number, loc->address,
- "at", b->addr_string, buf + strlen (buf),
- 2048 - strlen (buf));
+ strcpy (buf, "QTDPsrc:");
+ encode_source_string (b->number, loc->address,
+ "at", b->addr_string, buf + strlen (buf),
+ 2048 - strlen (buf));

- putpkt (buf);
- remote_get_noisy_reply (&target_buf, &target_buf_size);
- if (strcmp (target_buf, "OK"))
- warning (_("Target does not support source download."));
- }
- if (b->cond_string)
- {
- strcpy (buf, "QTDPsrc:");
- encode_source_string (b->number, loc->address,
- "cond", b->cond_string, buf + strlen (buf),
- 2048 - strlen (buf));
- putpkt (buf);
- remote_get_noisy_reply (&target_buf, &target_buf_size);
- if (strcmp (target_buf, "OK"))
- warning (_("Target does not support source download."));
- }
- remote_download_command_source (b->number, loc->address,
- breakpoint_commands (b));
+ putpkt (buf);
+ remote_get_noisy_reply (&target_buf, &target_buf_size);
+ if (strcmp (target_buf, "OK"))
+ warning (_("Target does not support source download."));
}
-
- do_cleanups (old_chain);
+ if (b->cond_string)
+ {
+ strcpy (buf, "QTDPsrc:");
+ encode_source_string (b->number, loc->address,
+ "cond", b->cond_string, buf + strlen (buf),
+ 2048 - strlen (buf));
+ putpkt (buf);
+ remote_get_noisy_reply (&target_buf, &target_buf_size);
+ if (strcmp (target_buf, "OK"))
+ warning (_("Target does not support source download."));
+ }
+ remote_download_command_source (b->number, loc->address,
+ breakpoint_commands (b));
}
+
+ do_cleanups (old_chain);
+
+ return 0;
+
}

static void
@@ -10484,7 +10482,7 @@ Specify the serial device it is connected to\n\
remote_ops.to_supports_enable_disable_tracepoint = remote_supports_enable_disable_tracepoint;
remote_ops.to_supports_string_tracing = remote_supports_string_tracing;
remote_ops.to_trace_init = remote_trace_init;
- remote_ops.to_download_tracepoint = remote_download_tracepoint;
+ remote_ops.to_download_tracepoint_loc = remote_download_tracepoint_loc;
remote_ops.to_download_trace_state_variable
= remote_download_trace_state_variable;
remote_ops.to_enable_tracepoint = remote_enable_tracepoint;
diff --git a/gdb/target.c b/gdb/target.c
index 41ff6cf..48cd888 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -674,7 +674,7 @@ update_current_target (void)
INHERIT (to_supports_enable_disable_tracepoint, t);
INHERIT (to_supports_string_tracing, t);
INHERIT (to_trace_init, t);
- INHERIT (to_download_tracepoint, t);
+ INHERIT (to_download_tracepoint_loc, t);
INHERIT (to_download_trace_state_variable, t);
INHERIT (to_enable_tracepoint, t);
INHERIT (to_disable_tracepoint, t);
@@ -847,8 +847,8 @@ update_current_target (void)
de_fault (to_trace_init,
(void (*) (void))
tcomplain);
- de_fault (to_download_tracepoint,
- (void (*) (struct breakpoint *))
+ de_fault (to_download_tracepoint_loc,
+ (int (*) (struct bp_location *))
tcomplain);
de_fault (to_download_trace_state_variable,
(void (*) (struct trace_state_variable *))
diff --git a/gdb/target.h b/gdb/target.h
index 352f2df..74e1d8a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -686,8 +686,12 @@ struct target_ops
/* Prepare the target for a tracing run. */
void (*to_trace_init) (void);

- /* Send full details of a tracepoint to the target. */
- void (*to_download_tracepoint) (struct breakpoint *t);
+ /* Send full details of a tracepoint location to the target. Target may
+ install or not install tracepoint locations to inferior, which is
+ determined by target's factors, such tracing state. If the location
+ of tracepoint is downloaded and installed successfully, return 0,
+ otherwise return 1. */
+ int (*to_download_tracepoint_loc) (struct bp_location *location);

/* Send full details of a trace state variable to the target. */
void (*to_download_trace_state_variable) (struct trace_state_variable *tsv);
@@ -1474,8 +1478,8 @@ extern int target_search_memory (CORE_ADDR start_addr,
#define target_trace_init() \
(*current_target.to_trace_init) ()

-#define target_download_tracepoint(t) \
- (*current_target.to_download_tracepoint) (t)
+#define target_download_tracepoint_loc(t) \
+ (*current_target.to_download_tracepoint_loc) (t)

#define target_download_trace_state_variable(tsv) \
(*current_target.to_download_trace_state_variable) (tsv)
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 4ca4ec2..493d324 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1701,6 +1701,7 @@ start_tracing (void)
for (ix = 0; VEC_iterate (breakpoint_p, tp_vec, ix, b); ix++)
{
struct tracepoint *t = (struct tracepoint *) b;
+ struct bp_location *loc;

if ((b->type == bp_fast_tracepoint
? !may_insert_fast_tracepoints
@@ -1708,7 +1709,10 @@ start_tracing (void)
continue;

t->number_on_target = 0;
- target_download_tracepoint (b);
+
+ for (loc = b->loc; loc; loc = loc->next)
+ target_download_tracepoint_loc (loc);
+
t->number_on_target = b->number;
}
VEC_free (breakpoint_p, tp_vec);
--
1.7.0.4