Support more stop reasons

[bet4it]

Description

I need the exec stop reason, so I create this PR.

As for exec, how to design the type?

What do you think about it:

    Exec {
        tid: Tid,
        name: &'a [u8]
    },

But in this way, I must add a lot lifetime specifiers.

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).