Support more stop reasons

[bet4it]

Description

This PR extends the BaseStopReason enum to support reporting dynamic library changes and process forking events to the GDB client.

New Stop Reasons

• Library(Tid)

  • Purpose: Indicates that the list of loaded libraries has changed. GDB should refresh its symbol table.
  • Parameters: Tid of the thread reporting the event.

• Fork { cur_tid, new_tid }

  • Purpose: Reports that a thread has executed a fork system call, creating a new child process.
  • Parameters:
    • cur_tid: The thread ID of the parent (current thread).
    • new_tid: The thread ID/Process ID of the newly created child.

• VFork { cur_tid, new_tid }

  • Purpose: Reports a vfork event. Similar to fork, but implies the parent is suspended and shares address space with the child until exec or exit.
  • Parameters: Same as Fork (cur_tid, new_tid).

• VForkDone(Tid)

  • Purpose: Indicates that a child process created via vfork has completed its initialization (called exec or exited), allowing the parent process to resume.
  • Parameters: Tid of the parent thread.

Based on GDB documentation: Stop Reply Packets.

API Stability

• This PR does not require a breaking API change

Checklist

• Documentation
  • Ensured any public-facing rustdoc formatting looks good (via cargo doc)
  • (if appropriate) Added feature to "Debugging Features" in README.md
• Validation
  • Included output of running examples/armv4t with RUST_LOG=trace + any relevant GDB output under the "Validation" section below
  • Included output of running ./example_no_std/check_size.sh before/after changes under the "Validation" section below
• If implementing a new protocol extension IDET
  • Included a basic sample implementation in examples/armv4t
  • IDET can be optimized out (confirmed via ./example_no_std/check_size.sh)
  • OR implementation requires introducing non-optional binary bloat (please elaborate under "Description")
• If upstreaming an Arch implementation
  • I have tested this code in my project, and to the best of my knowledge, it is working as intended.

[daniel5151]

Ahhh, you gotta love the GDB RSP, eh? Every time I think we've made a reasonable choice around representing some part of the protocol, there's always gotta be a feature that makes things more complicated than needed...

Looking through the docs and the code, it does appear to be the case that adding a lifetime to BaseStopReason and wiring will be what we'll want to do in the long term.

...that said, this is quite an intrusive change, and would be API breaking, requiring a 0.8 release. This isn't necessarily a bad thing (0.7 has been out for quite a while at this point, it seems fine to cut a new major version), but if we're doing 0.8, there are other breaking changes I'd want to get around to implementing alongside that release as well.

As such, maybe there's a stop-gap solution that would allow you to add this variant in a non breaking way, so we can release it via a 0.7.x release? Namely: lets use the fact that BaseStopReason is marked #[non_exhaustive], and simply add a new #[cfg(feature = "alloc")]-gated variant called ExecOwned { tid: Tid, name: Vec<u8> }?

If we do that, then later one, when I get around to release 0.8, I can go and do all the proper lifetime plumbing myself, and make the breaking change alongside various other breaking changes.

What do you think? Would that work?

[bet4it]

lets use the fact that BaseStopReason is marked #[non_exhaustive], and simply add a new #[cfg(feature = "alloc")]-gated variant called ExecOwned { tid: Tid, name: Vec<u8> }?

I will get this error:

error[E0204]: the trait `std::marker::Copy` cannot be implemented for this type
   --> src/stub/stop_reason.rs:28:17
    |
28  | #[derive(Clone, Copy, Debug, PartialEq, Eq)]
    |                 ^^^^
...
144 |         name: Vec<u8>
    |         ------------- this field does not implement `std::marker::Copy`

[daniel5151]

Well shoot. That's unfortunate. I didn't realize the type was Copy...
In that case, it seems we have no choice but to make a breaking change here. Bummer.

I've gone ahead and spun up a new dev/0.8 branch, so please re-target the PR to that branch.

Since we're making a breaking change after all, lets go with the lifetime-based approach for the Exec stop reason. If that proves to be untenable for whatever reason, we might want to consider dropping the Copy requirement from the type... but I'll need to think through the downstream implications of making that change before committing to doing that.

[bet4it]

So are there any other things I can do? 😅

[daniel5151]

Apologies for the late response. I'm currently in the process of moving from Seattle to NYC, so I may be a bit slow to respond.

I'm open to hearing other ideas that don't require breaking changes... but I'm afraid I don't have any clever suggestions for you. I think your best bet is to submit the PR with the necessary breaking changes over to the dev/0.8 branch.

If you'd like to land these fork-related stop reasons into the master branch and punt the exec stop reason to another PR, that would be totally fine with me.

[bet4it]

First of all, congratulations on starting a new chapter in life!

I don't really need these fork-related stop reasons, so I'll just leave them here. Also, I think the Exec-related implementation still needs to be completed by you. Feel free to complete it at your convenience!

[daniel5151]

Thank you for the kind words! Moving to NYC is something I've wanted to do for a long time, so I'm excited its finally happened, hah.

As for the PR - no worries, I totally understand. I've got a few weeks of "unpaid vacation" before I start my new job, and finding some time to re-invest in gdbstub is something I'm trying to prioritize. As always, the fastest way to ensure something lands in gdbstub would be to iterate on it yourself, but if not, fingers crossed I'll find some time in the near future to drive this Exec stop reason work over the finish line.

If you want to land this PR as is (i.e: without the Exec stop reason), feel free to un-draft it, update the description to reflect its reduced scope, and I'll be happy to review + land it into master (and cut a new 0.7.x release with these changes).

[daniel5151]

@bet4it thanks for the update here (and sorry for letting this PR languish for so long!).

Any chance you could update the PR description to more accurately reflect the stop reasons being added here? I can also fix that up for you if you want, up to you.

I'll make sure we land these new stop reasons before the end of this weekend.

Also, as for the Exec stop reason... yeah, that Copy bound really makes adding it in a non-breaking fashion annoying 😭. It'll def have to be part of a separate PR that includes some breaking changes 😢

[bet4it]

Any chance you could update the PR description to more accurately reflect the stop reasons being added here? I can also fix that up for you if you want, up to you.

Updated.

Also, as for the Exec stop reason... yeah, that Copy bound really makes adding it in a non-breaking fashion annoying 😭. It'll def have to be part of a separate PR that includes some breaking changes 😢

Yeah, implementing Exec elegantly in a no_std environment is indeed quite tricky.

[daniel5151]

Figured it'd be easier to push a commit on this branch to fix up a few minor nits (wrt. naming, adding docs).

Also, I took this chance to make sure the size guards are doing the right thing, and they do indeed seem to work correctly (on my local machine).

Thank you for sending this in!