Re: [PATCH] gdb: better handling of 'S' packets

Andrew Burgess
2021-01-08T10:15:13Z

* Simon Marchi <simon.marchi@polymtl.ca> [2021-01-07 22:00:14 -0500]:

> Hi Pedro,
>
> It looks like you reviewed v1, but Andrew had posted v2:
>
> https://sourceware.org/pipermail/gdb-patches/2020-December/174277.html
>
> I don't know if that was on purpose.
>
> On 2021-01-07 7:51 p.m., Pedro Alves wrote:
> > I'll look at Simon's patch next, and your thread/frame patches after.
> > Hopefully tomorrow.
>
> I adapted Andrew's v2 to work on top of my patch that makes the remote
> target track the threads' resume state, and so far it looks good (Andrew's
> test still passes). I'll give it a test run and send a combined series.

Thanks Simon.

I think this is the best approach. The V2 patch addresses the core
issue that was raised in the bug only, while the V1 patch goes beyond
this.

The reason the V1 patch came about is that originally I struggled to
find a way to reproduce exactly the issue that was reported in the
bug. To reproduce I was testing gdbserver with 'T' and 'Tthread'
disabled and I kept running into other bugs in cases where I would
tell myself, yes the remote is wrong not to include a thread-id, but
GDB _should_ be able to figure out what I'm doing here.

As Pedro points out in his V1 patch review, we should probably go for
simplicity over complexity in the face of a badly behaving remote
target. So long as GDB doesn't crash/assert, just throwing a warning
at the user is fine.

NOTE: The V2 patch does address a _real_ bug in GDB, so absolutely
should still go in.

Once V2 is merged I'll rebase the V1 patch and see how complex it
actually looks. I might repost it if it doesn't add too much
complexity, but I might just drop it.

Thanks.

Andrew