Add direct way to access GDB client's current selected TID

[jakab922]

In the gdb docs for handling threads there is the thread <thread_id> command listed for which the documentation is the following:

Make thread ID thread-id the current thread. The command argument thread-id is the GDB thread ID, as shown in the first field of the ‘info threads’ display, with or without an inferior qualifier (e.g., ‘2.1’ or ‘1’).
...

When I issue thread 1 on the client side a $T<client_side_thread_id>#<checksum> message is sent to the server.

On the server side this message is handled in here by calling is_thread_alive which is not quite right. This call should select a thread as active on a multithreaded environment rather than check if the thread is alive.

The reason this is an issue for me is because I have a multi core environment where each core supports hardware breakpoints and is represented by a thread on the server side. For someone to set a hardware breakpoint they need to issue the following commands for example:

thread <thread_id>
hbreak *<address>

I can add some thread activation logic to the is_thread_alive method, but it's a hack. What would be the right way to fix this?

[daniel5151]

Based on the GDB RSP docs for the T packet, I'm not sure gdbstub is doing anything wrong here?

From https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html#index-T-packet

‘T thread-id’

Find out if the thread thread-id is alive. See thread-id syntax.

Reply:

‘OK’ - thread is still alive

While it's certainly possible that the GDB RSP docs are subtly incorrect, I don't recall ever finding an issue with those docs in the past...

IIRC, its the H packet is responsible for setting what thread is considered active.
T seems to be purely informational in nature.

So I'm not totally sure what's going on here...

[daniel5151]

@jakab922, any updates here?

I'll leave this open a while longer, but if there's no movement here, I'll likely close this as unactionable 😅

[jakab922]

I added a workaround to the mentioned function. The thing is that the Z<number> messages don't have any information on where one wants to set the breakpoint/watchpoint. In case of a software breakpoint it's not an issue, but hardware breakpoints are part of a core and without any extra information I can only only make guesses.

[daniel5151]

The thing is that the Z<number> messages don't have any information on where one wants to set the breakpoint/watchpoint

Is addr not precisely what you're interested in?

‘z1,addr,kind’
‘Z1,addr,kind[;cond_list…][;cmds:persist,cmd_list…]’

Insert (‘Z1’) or remove (‘z1’) a hardware breakpoint at address addr.

A hardware breakpoint is implemented using a mechanism that is not dependent on being able to modify the target’s memory. The kind, cond_list, and cmd_list arguments have the same meaning as in ‘Z0’ packets.

Implementation note: A hardware breakpoint is not affected by code movement.

‘z2,addr,kind’
‘Z2,addr,kind’

Insert (‘Z2’) or remove (‘z2’) a write watchpoint at addr. The number of bytes to watch is specified by kind.

---

...unless by "where one wants to set the breakpoint/watchpoint", you're referring to "what is the currently selected core"?

If so, then you might be on to something... the current gdbstub API doesn't really offer a direct way to easily detect what the "currently selected thread" is from within the context of the Hw{Watch,Break}point traits.

i.e: historically, gdbstub has taken care of handling the H packet entirely within itself (and storing the result in self.current_resume_tid):

‘H op thread-id’

Set thread for subsequent operations (‘m’, ‘M’, ‘g’, ‘G’, et.al.). Depending on the operation to be performed, op should be ‘c’ for step and continue operations (note that this is deprecated, supporting the ‘vCont’ command is a better option), and ‘g’ for other operations.

So, let me get your thoughts on two proposals:

1. Updating the Hw{Watch,Break}point traits to include a selected_tid: Tid parameter, surfacing the current value of self.current_resume_tid
2. Adding a new (optional) on_changed_resume_tid method to {Single,Mulit}ThreadBase, that users can override to "hook" into the H packet handler, and get a signal when the current selected TID has changed.

The 1st option is, IMO, cleaner from an overall API design. That said - it will require a breaking change to implement, and I'm not sure I'm ready to cut gdbstub 0.8 just yet.

The 2nd option does not require a breaking change, but it will be a bit less elegant, requiring users to do a bit of manual wiring between the on_changed_resume_tid method and the Hw{Watch,Break}point methods.

---

Let me know if that makes sense, of if I'm misunderstanding anything

[daniel5151]

@jakab922, friendly ping :)

Before implementing any changes, I want to make sure I've got a good grasp of the pain point / gap here.

[jakab922]

Sorry for the hiatus, I was busy with other things. I think handling the 'H' package is probably the best option. The '[zZ][0-4]' packages although support thread id based filtering but this is client side information only.

Given that the official documentation says that the 'H' message selects a specific thread for subsequent operations I think storing the selected thread's id makes sense. I actually wasn't aware of this message and that was the reason I wanted to use the 'T' message for this.

So in short I think option 2 is better. My current implementation is basically storing the selected id one layer below gdbstub and I added a python plugin which basically selects a thread and then sets a breakpoint or whatever needs to be done on the client side. It's not an ideal solution, but this is due to the nature of the gdb remote protocol which even it if let's users issue commands for specific threads that information is kept on the client side.

[daniel5151]

No worries.

The second approach of allowing users to hook into the 'H' packet via some sort of MultiThreadBase::on_changed_resume_tid method does indeed seem like a reasonable way to address this feature gap.

I'm a bit occupied with work / personal life at the moment, so I'm not sure I'll be able to implement this feature myself in the near future. That said - if you'd like to take a stab at implementing this and sending in a PR for it, I'll be more than happy to review, merge, and publish a new point-release of gdbstub with this feature :)

---

Sidenote: GDB appears to have a 'native' way to set a thread-specific breakpoint: https://sourceware.org/gdb/current/onlinedocs/gdb.html/Thread_002dSpecific-Breakpoints.html

It might be interesting to explore what packet sequence that feature actually corresponds to, in case we're missing some non-obvious third approach to supporting this sort of feature.