This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 23 May 2013 18:44:21 +0100
- Subject: Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
- References: <20130514191026 dot 13213 dot 39574 dot stgit at brno dot lan> <20130514191054 dot 13213 dot 58686 dot stgit at brno dot lan> <87obcd48vj dot fsf at fleche dot redhat dot com>
On 05/14/2013 09:14 PM, Tom Tromey wrote:
> Pedro> + /* Note that all addresses are always "out the step range" when
> Pedro> + there's no range to begin with. */
>
> I think "out of the step range".
Thanks. I fixed that one.
> Pedro> + else if (p[0] == 'r')
> Pedro> + {
> Pedro> + char *p1;
> Pedro> +
> Pedro> + p = p + 1;
> Pedro> + p1 = strchr (p, ',');
> Pedro> + decode_address (&resume_info[i].step_range_start, p, p1 - p);
> Pedro> +
> Pedro> + p = p1 + 1;
> Pedro> + p1 = strchr (p, ':');
> Pedro> + decode_address (&resume_info[i].step_range_end, p, p1 - p);
> Pedro> +
> Pedro> + p = p1;
> Pedro> + }
>
> I don't know much about gdbserver, but reading this made me wonder if it
> needs to do any kind of error-checking on its input.
Yeah, it doesn't tend to do that much validation. Perhaps we should.
> Like - what if the wrong format is sent,
Crash in many different places most likely. When looking at
validation, I'm more looking at making it easier for possible
future packet extensions not break older gdbserver.
In this case, I do believe this bit:
+ p1 = strchr (p, ':');
+ decode_address (&resume_info[i].step_range_end, p, p1 - p);
should not expect the ':' to be there. An action
without a ptid is valid. I means it applies to all and
is handled as the default action, further below:
if (p[0] == 0)
{
resume_info[i].thread = minus_one_ptid;
default_action = resume_info[i];
/* Note: we don't increment i here, we'll overwrite this entry
the next time through. */
}
else if (p[0] == ':')
I'll fix it in a follow up.
> or if the range end is before the range start?
Yeah, considering that MIPS has signed addresses, I'm
leaving this one open.
--
Pedro Alves