Make single-stepping optional

[daniel5151]

Make single-stepping optional

[bet4it]

We could just use cont!

[bet4it]

resume could also be optional?

[daniel5151]

As per https://sourceware.org/gdb/onlinedocs/gdb/Overview.html#Overview

At a minimum, a stub is required to support the ‘?’ command to tell GDB the reason for halting, ‘g’ and ‘G’ commands for register access, and the ‘m’ and ‘M’ commands for memory access. Stubs that only control single-threaded targets can implement run control with the ‘c’ (continue) command, and if the target architecture supports hardware-assisted single-stepping, the ‘s’ (step) command. Stubs that support multi-threading targets should support the ‘vCont’ command. All other commands are optional.

While it's technically possible to start a debugging session without implementing resume, the spec still mandates that resumption must be supported by the target.

If the target doesn't wish to implement resume, it can simply provide a dummy implementation, such as immediately returning a StopReason::Signal(5).

[bet4it]

Stubs that only control single-threaded targets can implement run control with the ‘c’ (continue) command

It's just 'can', not 'must' or 'should', right?

Stubs that support multi-threading targets should support the ‘vCont’ command

It's talking about vCont, but vCont has some subcommands such as vCont;c or vCont;s.
For example, I may only support vCont;s and vCont;S, I don't think I violate this spec.

---

The document here is talking about "run control", but is run control necessary? I can think of a situation that I just want to use GDB RSP to probe current memory and register status, then I will 'detach' rather than 'continue'.

---

If the target doesn't wish to implement resume, it can simply provide a dummy implementation, such as immediately returning a StopReason::Signal(5).

We can also do the same thing to step, so why you make it optional?

[daniel5151]

Ahh, you know what, that's totally a reasonable way to read the spec.

In that case, it might even make sense to spit the entire notion of "resumption" into a separate IDET, leaving BaseOps to only deal with reading/writing registers/memory + thread enumeration...

While that would technically be more "correct" with the base spec, this might be a good moment to discuss the entire concept of "base spec" in gdbstub:

---

As hypocritical as it sounds coming from me (given my proclivity to follow the spec to a tee), gdbstub has always taken a more "pragmatic" approach to what constitutes the GDB RSP's "base" protocol.

A strict reading of the spec would suggest that the only packets that need to be implemented by a minimal stub are literally ?, m, M, g, and G, but in reality, trying to connect to a GDB stub that only supports those packets using a "modern" GDB client will fail miserably. Namely, a minimal stub must also support packets such as H, qAttached, qsThreadInfo, qSupported, T, etc... to get past the initial handshake.

In addition, in order to simplify gdbstub's internal architecture, gdbstub reports support for the GDB RSP's multiprocess extensions (i.e: vCont, expanded Thread Id syntax, etc...). Theoretically, this is something I might be able to make optional, but it would be quite tricky, as the way ThreadId structs are parsed would depend on if the IDET is enabled, which might be tricky to "plumb through". Not to mention, I suspect that several newer protocol extensions would simply assume multiprocess extensions are enabled, and would subtly not work with a stub that doesn't support them...

Lastly, gdbstub also unconditionally implements several "end of connection" packets, such as D, k, and vKill to support graceful exit from a gdbstub debug session. Though now that I think about it, these could also be made optional...

Long story short: when it comes to the "base" protocol (i.e: the extensions that most implementations would be expected to implement), gdbstub has typically taken a more pragmatic approach, whereby it does consider the fact that most users will be using the standard GDB client, and that said client doesn't necessarily respect the altruistically optional nature of a minimal GDB RSP implementation.

---

With this in mind, how does this "pragmatic" approach relate to how gdbstub handles resume?

You're technically correct that - at least according to the spec - it should be possible to implement a stub that only supports single-stepping, while not supporting continue (i.e: by reporting back only vCont;s;S). Unfortunately, if we peek under the hood of the GDB Client implementation, you'll find this code, which effectively enforces that clients support continue. If you poke around the code some more, you'll also find that the GDB client assumes that c is also unconditionally supported.

In other words: the mainline GDB client cannot drive a GDB RSP target using only single-stepping, and assumes that the target supports continue.

IMHO, that seems like a pretty reasonable assumption. After all, it's far easier to implement continue that it is to implement single-stepping.

If the target doesn't wish to implement resume, it can simply provide a dummy implementation, such as immediately returning a StopReason::Signal(5).

We can also do the same thing to step, so why you make it optional?

Consider the case where a client wants to support continue, but doesn't want to explicitly implement step. This is a use-case that I've come across myself recently.

With the current implementation (i.e: step support is always reported to the GDB client), it is invalid to simply return StopReason::Signal(5) from the single-step operation, as the target wouldn't have actually single stepped, which could lead to Bad Things happening. Instead, a implementer is forced to get single-stepping working as part of their initial gdbstub bringup, which doesn't need to be the case if step was optional.

---

TL;DR

• the entire notion of "resuming the target" could probably be put into a separate IDET, and gdbstub's minimal configuration would therefore only allow connecting to a paused target + manipulating its memory/register state.
  • I'll actually seriously consider doing this.
• while continue is not required in the strict reading of the spec, it is de-factor required by the mainline GDB client implementation, and since gdbstub takes a pragmatic approach to what constitutes the "base" protocol (in the name of user empathy), gdbstub will require that users support continue.

[bet4it]

the entire notion of "resuming the target" could probably be put into a separate IDET, and gdbstub's minimal configuration would therefore only allow connecting to a paused target + manipulating its memory/register state.

That's what I want!
Then we should set resume to be optional, and rename it to continue.

---

Unfortunately, if we peek under the hood of the GDB Client implementation, you'll find this code, which effectively enforces that clients support continue. If you poke around the code some more, you'll also find that the GDB client assumes that c is also unconditionally supported.

I think the "base" protocol we must implement includes commands GDB will request automatically when connect. If you capture the packets when connect GDB RSP server with GDB, you can find that vCont? won't be requested until you send commands such as s or c. I'm not sure if we can only implement single-step but not continue, but I'm sure that we can implement with neither of single-step nor continue is implemented.

If users not implement resume, they may encounter some errors when using some commands, but it's their responsibility to deal with it. For example, we set step to optional because if user not implement it, GDB will step manually with software breakpoint. But what if software breakpoint is also not implemented? We can't do all the things for the user.

And it's more friendly for newbies if we set resume to optional. It's much easier to implement register/memory access than run control. If a newbie try to use gdbstub on his repository, he can set up a GDB RSP server easily without need to change much code if run control is not considered.

[daniel5151]

I think the "base" protocol we must implement includes commands GDB will request automatically when connect.

The more I think about it, the more I think this is actually the right approach...

The only question is whether or not this should be a part of #92. Technically, the change of "making single stepping optional" and the change of "making resumption in general" option are two separate ones, and probably merit two separate PRs.

I think I'll open a new tracking issue to make resumption optional, and move forward with #92 in its current state.

If users not implement resume, they may encounter some errors when using some commands, but it's their responsibility to deal with it. For example, we set step to optional because if user not implement it, GDB will step manually with software breakpoint. But what if software breakpoint is also not implemented? We can't do all the things for the user.

Funny you bring this up as an example, because I literally added a guard rail for this lol: 1d55539

[bet4it]

I think it's better to set resume to be optional in the same PR. And resume is useless now, we need to split it to step and continue function.
We may also need to rename SingleThreadSingleStep to SingleThreadRunControl or something.

Certainly, it's all up to you😄

[daniel5151]

And resume is useless now, we need to split it to step and continue function.

Not sure what you mean by this. If you're talking about the current implementation in #92, resume plays a very clear purpose in both the single threaded and multi threaded implementation. In the single threaded implementation, the base level resume handles the base "continue" case. In the multi threaded implementation, I've split the old base level set_resume_action into a base level set_resume_action_continue function + a set_resume_action_step method (hidden behind an IDET). The resume method in the multithread case serves the same purpose as it always has - to signal that all resume actions have been set, and that the target should un-pause.

We may also need to rename SingleThreadSingleStep to SingleThreadRunControl or something.

Again, I don't know what you mean by this.

Even if I were to split resumption into its own IDET (i.e: Single/MultiThreadRunContol), there would still need to be a Single/MultiThreadSingleStep sub-IDET to toggle support for single stepping on and off...

[bet4it]

When will resume be called after we make step optional? Only when command continue is called? Then resume should be renamed to continue now?

What I want likes what we do on breakpoint:
Breakpoints -> (sw_breakpoint / hw_breakpoint / hw_watchpoint)
RunContol -> (step / continue)

Or you can just implement it and we can discuss on the code...

[daniel5151]

Yes, continue would absolutely be the most appropriate name - too bad it's a keyword 😅

Though I agree - in the single-step case, it might make sense to rename resume to something like resume_cont (and then likely change step to resume_step for consistency, etc...)

While this is technically bikeshedding, I do agree that the names ought to be clearer, so I'll update them in the current PR appropriately...

---

Aside from that though, it seems the specific issue of "making single step optional" has been pretty much resolved.

After tweaking some of the method names, I'll merge the current PR as is, and then look into splitting resumption as a whole out into an optional IDET in a follow up PR...