Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: better handling of 'S' packets
Date: Fri, 8 Jan 2021 10:15:06 +0000	[thread overview]
Message-ID: <20210108101506.GW2945@embecosm.com> (raw)
In-Reply-To: <22799d1d-7713-7ab5-f029-d914eafbf624@polymtl.ca>

* 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

  reply	other threads:[~2021-01-08 10:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-11 15:35 Andrew Burgess
2020-12-10 16:29 ` Andrew Burgess
2020-12-23 23:09   ` [PATCHv2] " Andrew Burgess
2021-01-06 21:19     ` Simon Marchi via Gdb-patches
2021-01-07  9:57       ` Andrew Burgess
2021-01-08  0:51 ` [PATCH] " Pedro Alves
2021-01-08  3:00   ` Simon Marchi via Gdb-patches
2021-01-08 10:15     ` Andrew Burgess [this message]
2021-01-08  3:58   ` Simon Marchi via Gdb-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210108101506.GW2945@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox