public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@palves.net>
To: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>,
	gdb-patches@sourceware.org
Subject: Re: [PATCH] gdbserver: try selecting a thread first to access memory
Date: Tue, 18 Jul 2023 14:09:07 +0100	[thread overview]
Message-ID: <90f33e9c-a586-a94a-3584-5288ce14d410@palves.net> (raw)
In-Reply-To: <20230620083519.1295801-1-tankut.baris.aktemur@intel.com>

On 2023-06-20 09:35, Tankut Baris Aktemur via Gdb-patches wrote:
> Since commit
> 
>   commit 7f8acedeebe295fc8cc1d11ed971cbfc1942618c
>   Author: Pedro Alves <pedro@palves.net>
>   Date:   Tue Apr 5 13:57:11 2022 +0100
> 
>     gdbserver: track current process as well as current thread
> 
> gdbserver switches to a process, rather than a thread, before
> processing a memory access request coming from the GDB side.  This
> switch sets current_thread to null.  Some memory accesses on certain
> targets, however, may require having a thread context.  Therefore, try
> switching to the selected thread first.  If this fails for a reason
> (e.g.  the thread has exited), switch to the process.

The reason I liked always selecting a process (with thread == null), is
that it made the corner case be the case that runs all the time.  I.e., any
target backend that incorrectly assumes there's always a thread selected,
is immediately exposed to the problem scenario and is forced to fix it.
Trying to select the thread first loses that.

If the access is for a thread-specific address space, and the thread exited,
what is supposed to happen?  Is the backend supposed to error out in that
scenario?  Or will it misbehave?

How do you tell what address space the access is targeting?  Are you encoding
that in the address, or do you have more changes to gdbserver, and maybe
RSP extensions?

I'm wondering whether we should have something like:

  if ((aspace_thread_specific (...) && set_desired_thread ())
      || set_desired_process ())
     res = read_inferior_memory (memaddr, myaddr, len);
  else
     res = 1;


  reply	other threads:[~2023-07-18 13:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-20  8:35 Tankut Baris Aktemur
2023-07-18 13:09 ` Pedro Alves [this message]
2023-07-18 13:36   ` Aktemur, Tankut Baris
2023-07-18 14:03     ` Pedro Alves
2023-07-18 14:23       ` Aktemur, Tankut Baris
2023-08-01 13:20       ` [PATCH v2] gdbserver: select a thread, if necessary, to access memory (was: [PATCH] gdbserver: try selecting a thread first to access memory) Tankut Baris Aktemur
2023-11-21 19:45         ` Aktemur, Tankut Baris

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=90f33e9c-a586-a94a-3584-5288ce14d410@palves.net \
    --to=pedro@palves.net \
    --cc=gdb-patches@sourceware.org \
    --cc=tankut.baris.aktemur@intel.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).