This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Fix PR 13392 : check offset of JMP insn
On 03/07/2012 08:24 AM, Yao Qi wrote:
> On 03/07/2012 01:03 AM, Pedro Alves wrote:
>> > send a clearer error back to GDB, and we end up with:
>> >
>> > Target returns error code '.Tracepoint 2 at 0x7ffff67c2579 already exists'.
> It is OK for me to let remote target to reports a duplicated tracepoint.
I don't know what you mean then. It already does, but with a cryptic E01.
> In this version, some changes compared with last one,
>
> - move `tracepoint already exist' patch out, which can be a separate one,
> - remove downloaded tracepoint in target when failed to install,
We should also check what happens when we do "enable TRACEPOINT_FOO", and
that tracepoints fails to be likewise inserted. Following this direction,
we should make sure it stays disabled in the target, but _not_ deleted.
> +/* Remove TPOINT from global list. */
> +
> +static void
> +remove_tracepoint (struct tracepoint *tpoint)
> +{
> + struct tracepoint *tp, *tp_prev;
> +
> + for (tp = tracepoints, tp_prev = NULL; tp && tp != tpoint;
> + tp_prev = tp, tp = tp->next)
> + ;
> +
> + if (tp)
> + {
> + if (tp_prev)
> + tp_prev->next = tp->next;
> + else
> + tracepoints->next = tp->next;
This doesn't look correct. If there's no tp_prev, then TP must be
TRACEPOINTS, and so you need to do 'tracepoints = tp->next;'.
Or just rewrite this as:
static int
remove_tracepoint (struct tracepoint *todel)
{
struct tracepoint *tp, **tp_link;
tp = tracepoints;
tp_link = &tracepoints;
while (tp != NULL)
{
if (tp == todel)
{
*tp_link = tp->next;
xfree (tp);
return;
}
tp_link = &tp->next;
tp = *tp_link;
}
}
> /* There may be several tracepoints with the same number (because they
> are "locations", in GDB parlance); return the next one after the
> given tracepoint, or search from the beginning of the list if the
> @@ -2391,6 +2413,8 @@ cmd_qtdp (char *own_buf)
>
> download_tracepoint (tpoint);
> install_tracepoint (tpoint, own_buf);
> + if (strncmp (own_buf, "OK", 2) != 0)
> + remove_tracepoint (tpoint);
own_buf is a \0 terminated string. You can make that a straight strcmp(own_buf, "OK").
(Or change install_tracepoint's return type to int, to signal error.)
> + -re "Target returns error code .* too far .*$gdb_prompt $" {
> + if [string equal $trace_type "ftrace"] {
> + # The target was unable to install the fast tracepoint
> + # (e.g., jump pad too far from tracepoint).
> + pass "$test (too far)"
> + # Skip the rest of the tests.
> + return
Can you check tab/spaces here, and in the similar places, please? I may
have messed that up myself.
Otherwise okay.
--
Pedro Alves