This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patchv2 2/2] Workaround gdbserver<7.7 for setfs
- From: Pedro Alves <palves at redhat dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Gary Benson <gbenson at redhat dot com>
- Date: Wed, 30 Mar 2016 15:17:11 +0100
- Subject: Re: [patchv2 2/2] Workaround gdbserver<7.7 for setfs
- Authentication-results: sourceware.org; auth=none
- References: <20160319201842 dot GA16540 at host1 dot jankratochvil dot net> <56F13963 dot 9040204 at redhat dot com> <20160322131604 dot GA24312 at host1 dot jankratochvil dot net> <56F14F1E dot 5010606 at redhat dot com> <20160323211547 dot GA17400 at host1 dot jankratochvil dot net> <20160324220933 dot GA27445 at host1 dot jankratochvil dot net> <20160324223241 dot GB2548 at host1 dot jankratochvil dot net>
On 03/24/2016 10:32 PM, Jan Kratochvil wrote:
> There was a bug in patchv1.
>
>
> move2.patch
>
Please include self-contained a commit/rationale along with patch
posts (and reposts). You had context in your intro to v1 that was
lost in v2.
>
> gdb/ChangeLog
> 2016-03-24 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * remote.c (packet_ok): Add workaround for PACKET_vFile_setfs.
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index bb027cf..f80fee8 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1453,7 +1453,15 @@ packet_ok (const char *buf, struct packet_config *config)
> internal_error (__FILE__, __LINE__,
> _("packet_ok: attempt to use a disabled packet"));
>
> - result = packet_check_result (buf);
> + if (config == &remote_protocol_packets[PACKET_vFile_setfs]
> + && strcmp (buf, "OK") == 0)
> + {
> + /* Workaround gdbserver < 7.7 before its fix from 2013-12-11. */
> + result = PACKET_UNKNOWN;
> + }
This comment could use more detail.
E.g., reading this I'm left wondering, did it always respond OK to
unknown vFile packets, or to all unknown packets? I think it was
actually the latter.
AFAICS from the commit you pointed at in v1, the "OK" was
gdbserver mistaking any unknown packet for a vStopped packet,
with vStopped being the notification ack for the "%Stop" RSP async
notification. So it could also happen that gdb sends the setfs
packet while gdbserver had a pending notification, and then
gdbserver replies back a stop reply instead of "OK"...
We may need to guarantee an early enough setfs is attempted.
Is that already the case?
If I'm right and gdbserver mishandled _any_ unknown packet,
then I wonder whether you fix this one, but will trip on another
when you get past initial connection and actually do any serious
debugging?
If not, this may be sufficient. Otherwise, we may need to come up with
a different workaround, maybe based on sending an early probe packet,
like "MustReplyEmpty", to which well behaved stubs reply empty, just because
that's not a known packet to them. If a stub replies something other than
empty to that one, then maybe we should disable all other
auto-probed packets... That may force-disable too much functionality though...
So in sum:
- Expand comment a bit / include commit log with rationale in the patch.
- Give assurance that this is sufficient and that we won't trip on the
same thing with other packets anyway. Otherwise we may need to think
of something else.
Thanks,
Pedro Alves