Skip to content

Rework process attaching #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jonathanzetier wants to merge 1 commit into
base: dev/0.8
Choose a base branch
from
Open

Rework process attaching #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/transition_guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ Previously, if a thread had no `resume_set_action_XXX` methods called on it, the

This new behavior obsoletes the `MultiThreadSchedulerLocking` trait, which has been removed. If a stub is unable to handle executing a single thread and keeping all others locked, it should return an error in the `resume` method.

#### The `CurrentActivePid` trait no longer exists

The current PID is now tracked in the stub itself, so targets no longer need to implement this. No changes are needed for the `attach` method.

## `0.6` -> `0.7`

`0.7` is a fairly minimal "cleanup" release, landing a collection of small breaking changes that collectively improve various ergonomic issues in `gdbstub`'s API.
Expand Down
13 changes: 0 additions & 13 deletions examples/armv4t/gdb/extended_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,6 @@ impl target::ext::extended_mode::ExtendedMode for Emu {
) -> Option<target::ext::extended_mode::ConfigureWorkingDirOps<'_, Self>> {
Some(self)
}

#[inline(always)]
fn support_current_active_pid(
&mut self,
) -> Option<target::ext::extended_mode::CurrentActivePidOps<'_, Self>> {
Some(self)
}
}

impl target::ext::extended_mode::ConfigureAslr for Emu {
Expand Down Expand Up @@ -169,9 +162,3 @@ impl target::ext::extended_mode::ConfigureWorkingDir for Emu {
Ok(())
}
}

impl target::ext::extended_mode::CurrentActivePid for Emu {
fn current_active_pid(&mut self) -> Result<Pid, Self::Error> {
Ok(self.reported_pid)
}
}
13 changes: 11 additions & 2 deletions src/stub/core_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::common::IsValidTid;
use crate::common::Pid;
use crate::common::Signal;
use crate::conn::Connection;
use crate::protocol::commands::Command;
Expand All @@ -7,6 +8,7 @@ use crate::protocol::ResponseWriter;
use crate::protocol::SpecificIdKind;
use crate::stub::error::InternalError;
use crate::target::Target;
use crate::FAKE_PID;
use crate::SINGLE_THREAD_TID;
use core::marker::PhantomData;

Expand Down Expand Up @@ -105,6 +107,11 @@ pub(crate) struct GdbStubImpl<T: Target, C: Connection> {
_target: PhantomData<T>,
_connection: PhantomData<C>,

// The most recently attached PID
current_active_pid: Pid,

@daniel5151 daniel5151 May 15, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, IIRC, there's not real notion of "active" thread-id in the GDB RSP. The only firm concepts are the currently "selected" thread-id for memory operations, and resume operations.

as such, adding a new type here doesn't seem right. rather, I think the IsValidTid trait will need to grow some new methods for generically setting / getting the pid component of T::Tid.

@jonathanzetier jonathanzetier May 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a few places in the protocol that mention the notion of a "current thread":

  • The qC RSP message is described as this (annoyingly, this is entire thing, and there is no elaboration on what "current" means anywhere else...I know I already complained about this, but it doesn't make it any less annoying)

Return the current thread ID.

  • The documentation for qXfer:exec-file:read includes this:

If the annex part is empty the remote stub should return the filename corresponding to the currently executing process.

  • If a client doesn't support multiprocess extensions, it sends us thread IDs without a process ID, so there needs to be some notion of what process it's referring to; the process would be the "current" process (ie the attached process), which would combine to make the "current" thread.

Unless I'm misreading these, it seems like there's a need to represent the current process/thread somewhere; would you rather close this PR and keep on representing it in the target implementations like they were before this PR?

I do worry that handling it in the targets could be potentially confusing for multiprocess target implementations, who also have to keep track of what the "current" PID is, along with all their "attached" PIDs; like what does it really mean to have a "current" thread when you're keeping track of 3 separate processes that are all running at the same time?

Extending the IsValidTid trait to return the pid component seems like it would have to interact with the stub somehow to get this current pid for multithreaded stubs, since the client can still attach to an arbitrary process; we can't just return FAKE_PID...it would probably be more useful as a method of the stub, where the IsValidTid trait would return None if there was no pid component, and the stub could provide a current pid.

@daniel5151 daniel5151 May 17, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the GDB RSP docs definitely have some annoying ambiguities... sigh.

Inferring the correct behavior typically requires playing around with the code, and maybe cross-referencing with the upstream implementation in the GDB source code (i.e: in remote.c).

Unless I'm misreading these, it seems like there's a need to represent the current process/thread somewhere;

Well yes, of course. I'm not saying we shouldn't track it.

What I'm saying is that the correct place to track this info is in current_{mem,resume}_tid, which can only be done by extending T::Tid to properly track pids.

Or, in other words, to refine my initial comment - the terms "active" thread-id and "selected" thread-id are one and the same, and should be tracked via the same bit of storage.

Again, this all goes back to the fact that you really shouldn't be thinking about pids and tids as separate concepts for the vast majority of the GDB RSP.

Outside of a few narrow packets (e.g: attach, run), the only thing that ever really matters is a specific tuple of (pid, tid) (AKA, what the GDB RSP calls a thread-id). That's the thing that should be tracked via current_{mem,resume}_tid, that's the thing that will be passed to Target IDETs via the tid: T::Tid param, etc...

When you think about it that way, it should be very clear why a separate current_active_pid: Pid type doesn't really make sense. You're splitting out an indivisible aspect of the thread-id data type into its own field.

I do worry that handling it in the targets could be potentially confusing for multiprocess target implementations, who also have to keep track of what the "current" PID is, along with all their "attached" PIDs; like what does it really mean to have a "current" thread when you're keeping track of 3 separate processes that are all running at the same time?

The notion of the selected/active thread-id is something that should never leak out of the gdbstub implementation itself. And indeed, if you look, you'll find that qC doesn't even correspond to an IDET.

Indeed, not leaking "current active thread" semantics is is one of gdbstubs major ergonomic wins over trying to roll your own stub code!

From the target's perspective, there is no such thing as a 'Current PID'. The target is essentially just a set of stateless functions that say 'Read memory from PID X, Thread Y'. The only entity that needs to know which PID is 'current' is gdbstub, so it knows which numbers to pass into those functions.

It will only ever be asked to operate on specific thread-ids (i.e: (pid, tid) tuples), or all tids in a single pid (via some of the resume APIs). Sure, there are some APIs that will ask the target to do something related to a specific pid (i.e: attach), but it's only when the Target affirmatively responds to those packets (via a stop reason) that gdbstub checks to see what (pid, tid) is being reported as part of the stop response, and then updates its internal active-pid tracking.

Or, another example (not related to attach): when the GDB client wants to switch contexts, it sends an Hg (set general thread) or Hc (set continue thread) packet. gdbstub intercepts these and exclusively uses them to update its internal current_{mem,resume}_tid state. The target implementation never sees these H packets directly, and it never needs to maintain a concept of a 'selected' thread.

Instead, when a subsequent packet arrives that relies on this context (like a memory read m or a register read g), gdbstub automatically grabs that stored state and passes the fully-resolved (pid, tid) directly into the relevant Target trait method. Keeping that boundary strict is exactly why we want to avoid leaking a current_active_pid or similar state into the target traits.

Extending the IsValidTid trait to return the pid component seems like it would have to interact with the stub somehow to get this current pid for multithreaded stubs, since the client can still attach to an arbitrary process

Hopefully you can now see why this isn't really accurate. IsValidTid doesn't need to 'interact' with the stub to get the PID because the stub is the one that constructs the T::Tid and gives it to the trait in the first place. The trait simply acts as the storage container for that tuple.

i.e: when the client attaches to an arbitrary process, gdbstub will pass that request through to the state machine, and when the request is acknowledged by the Target with a corresponding stop-reason on attach, gdbstub will update its internal tracking to reflect the newly selected thread-id.

Does this all make a bit more sense now? Do you see why IsValidTid will need to be tweaked to also track pid handling?

wrt. handling the FAKE_PID codepaths - look at how I reworked gdbstub's code in #198 to report an error in cases where a single-threaded target (i.e: where T::Tid = ()) somehow runs into a situation where the GDB client is requesting something that isn't SINGLE_THREAD_TID. That same style of handling should be easy to do wrt. FAKE_PID when T::Tid is () or Tid in the single/multi-thread use cases, while transparently converting to/from (tid, pid) in the new multi-process use-cases.

@jonathanzetier jonathanzetier May 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a lot in there; I was already familiar with how tids (and one day, pids) get to the target method, but just to make sure I understood the actionable parts of your response, it's basically:

  1. We don't need the current_active_pid field of the GdbStubImpl (how this this PR does it) or the CurrentActivePid trait (what v0.8 currently does), because it should be tracked in current_{mem,resume}_tid
  2. We need to expand the IsValidTid to get the pid, and it would look something like this:
impl IsValidTid for () {
    def get_pid(&self) -> Pid {
        crate::FAKE_PID
    }

    ...
}

impl IsValidTid for Tid {
    def get_pid(&self) -> Pid {
        crate::FAKE_PID
    }

    ...
}

// Multiprocess name/representation in my local branch
impl IsValidTid for ExtendedThreadId {
    def get_pid(&self) -> Pid {
        self.pid
    }

    ...
}

If I'm mischaracterizing this, please ignore everything after this line and correct me!

I enthusiastically agree that this covers almost everything in the protocol, but I don't see how this handles a response to a qC packet. The client can still attach to any pid in the multithreaded (or even single threaded) case, but if the only candidates we can store that attached pid are in current_mem_tid/current_resume_tid, there's literally nowhere the value of the pid can go. When we use the above implementation for the return value of qC, we return FAKE_PID, and the gdb client crashes after a vAttach followed by a qC, with an assertion failure because the process portion of qC doesn't match the process it originally attached to (at least that's what versions 12.1, and IIRC 17.1, of the gdb client do).

Maybe there's a different path that fixes all of these? Currently the stub unconditionally reports it supports multiprocess features:

https://github.com/daniel5151/gdbstub/blob/dev/0.8/src/stub/core_impl/base.rs#L115-L116

This forces us to return extended thread IDs with a process and a thread in stop replies and qC. What if we only reply that we support multiprocess features in the (soon-to-exist) multiprocess mode? It helps us be more honest with the client about our actual capabilities, and then according to the protocol it's not supposed to even send us pids, or expect to receive pids from us. Then we don't have to worry about conjuring up some kind of pid (FAKE_PID...) to respond with for thread ID types that don't have one (like () or Tid).

@daniel5151 daniel5151 May 19, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost!

You're right on 1, but for 2, what you actually want to do is change the IsValidTid trait itself to operate on this new ExtendedThreadId type (which, should prob just be a (Pid, Tid) tuple, rather than a custom type).

// update trait
pub trait IsValidTid: private::Sealed + PartialEq + Copy {
    #[doc(hidden)]
    fn into_fully_qualified_tid(self) -> (Pid, Tid);
    #[doc(hidden)]
    fn from_fully_qualified_tid(pid: Pid, tid: Tid) -> Option<Self>;
    #[doc(hidden)]
    fn sentinel() -> Self;
}

// update existing impl
impl IsValidTid for Tid {
    fn into_fully_qualified_tid(self) -> (Pid, Tid) {
        (self, crate::FAKE_PID)
    }

    fn from_fully_qualified_tid(pid: Pid, tid: Tid) -> Option<Self> {
        // notice how the Tid impl now enforces the FAKE_PID?
        if pid == crate::FAKE_PID { Some(tid) } else { None }
    }

    fn sentinel() -> Self {
        crate::SINGLE_THREAD_TID
    }
}

// create new impl
impl IsValidTid for (Pid, Tid) {
    fn into_fully_qualified_tid(self) -> (Pid, Tid) {
        self
    }

    fn from_fully_qualified_tid(pid: Pid, tid: Tid) -> Option<Self> {
        Some((pid, tid))
    }

    fn sentinel() -> Self {
        (crate::FAKE_PID, crate::SINGLE_THREAD_TID)
    }
}

P.S: might be time to rename SINGLE_THREAD_TID to FAKE_TID for consistency, hah. Similarly, IsValidTid can prob be renamed to ValidTid, since it's now far more than just a marker trait. neither of these needs to happen now - just jotting down the thoughts so I don't forget.

I'm not totally following, so apologies if this response is us talking past eachother 😅

You are correct that there is a narrow window of time where gdbstub needs to use a "dummy" thread-id value internally, as it hasn't yet interacted with the Target enough to ascertain the true thread-id it should be reporting. The inline comments in GdbStubImpl::new talk about it briefly.

That said, IIRC, the very first message any GDB client sends over is ?, which will trigger the attach path I talked about above, and would therefore give gdbstub a chance to update its internal current_{mem,resume}_tid value from the stop reply issued by the user's integration.

Similarly, if the GDB client were to start off with a vAttach to a specific PID, same logic applies - it would trigger the same attach path, we would sniff the current thread-if from the stop reply packet, and we'd be all set.

Essentially, my claim is that qC is never sent prior to us having a chance to ascertain a specific thread-id to report from it.

If this turns out to be false in practice / in some narrow edge case... we can always have gdbstub artificially start its state machine into the attach state (instead of starting in the Idle state) in order to force users to declare up-front what thread-id is currently attached (and in that artificial attach state, simply swallow the stop reason they report, as the GDB client wouldn't be expecting a stop reason packet at that point in the GDB RSP sequence). But this isn't something I think we need... since my claim is that all GDB clients are "reasonable" insofar as starting off each conversation with a ? / vAttach / vRun, packet in order to understand what state the stub is in.

your idea of only using multiprocess mode in, well, multi-process mode is intriguing, but comes with a major caveat: it means that single/multi-threaded targets would be excluded from extended mode facilities. See this comment in the docs https://docs.rs/gdbstub/latest/gdbstub/target/ext/extended_mode/trait.ExtendedMode.html#extended-mode-for-singlemulti-threaded-targets

But in any case, hopefully the explanation I offer above makes it a bit clearer why - in practice - things should work fine.

@jonathanzetier jonathanzetier May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been operating under the assumption extended mode support and multiprocess support are orthogonal(-ish?), where stubs can support extended mode without multiprocess. The client can attach to any process it wants, it just can't attach to more than one of them at the same time. The documentation seems careful about specifying when packets are only available in extended mode versus when packets are only available in multiprocess mode, and I haven't seen anything suggesting that extended mode only applies for multiprocess stubs. The gdb client source code also doesn't seem to check for multiprocess support when it sends an extended mode packet (extended_remote_target::open in gdb/remote.c).

If you find that this isn't the case, please let me know!

Anyways, I agree that qC will always have a chance to snoop a Tid; the issue is that we lose the pid for Tid types that don't have a pid field. Please hear me out; I know there's already a lot written about how we don't need to care about the pid for 99% of the packets, and I agree: I'm saying that the response for qC is the 1% that does. Here's the exact scenario I'm worried about:

For the gdb clients I've tested, when the gdb client attaches to a process P, it sends vAttach P, Hgp0.0, qC, and then Hg<thread returned by qC>. The biggest note here is that the gdb client will crash with an assertion failure if the pid returned by qC doesn't match the pid passed by vAttach (aka P).

And here's how it would play out based on how I understand what you're proposing:

  1. gdbstub receives the vAttach P message, and passes P to the target's attach method.
  2. gdbstub snoops the stop reply that goes out to the client [0], and sets its internal current_{mem,resume}_tid fields. The stop reply takes a Tid (or ()), the stop reply's pid value is FAKE_PID after fully qualifying it, and we've totally lost track of P after this point.
  3. gdbstub receives the Hgp0.0 message, now current_mem_tid is an arbitrary thread. Not super relevant here.
  4. gdbstub receives the qC message. If it does not reply with a pid that is P, the gdb client will crash. When we fully qualify current_{mem,resume}_tid in our response, we send back FAKE_PID, which is probably not equal to P. There's nothing else we can do; gdbstub lost track of P in step 2! The gdb client crashes.

As always, please let me know if I'm misunderstanding what you're proposing and how it fits in here!

This is also why I'm saying not advertising multiprocess support might be the fix here: we don't have to include a process field in the response to qC anymore. We can reply with just a Tid value, and gdb doesn't lose its mind when we don't keep track of information we shouldn't have to care about.

I'm not sure what you mean about how this would break single/multi-threaded extended mode; the linked documentation seems to agree that we can have extended mode without needing to be multiprocess.

[0] I assume this is a future enhancement; gdbstub doesn't currently snoop the stop reply: vAttach and ? use report_reasonable_stop_reason, which doesn't call write_stop_common, which is where the current_{mem,resume}_tid get set.

@daniel5151 daniel5151 May 19, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... that's what I get for drafting a response before the morning coffee has fully kicked in, hah.

Yes, multi process extensions are orthogonal to extended mode, whoops.

[0] I assume this is a future enhancement; gdbstub doesn't currently snoop the stop reply: vAttach and ? use report_reasonable_stop_reason, which doesn't call write_stop_common, which is where the current_{mem,resume}_tid get set.

yes, all my comments are refer to a world post #194, where report_reasonable_stop_reason is in the rear-view mirror.

  1. gdbstub receives the vAttach P message, and passes P to the target's attach method.
  2. gdbstub snoops the stop reply that goes out to the client [0], and sets its internal current_{mem,resume}_tid fields. The stop reply takes a Tid (or ()), the stop reply's pid value is FAKE_PID after fully qualifying it, and we've totally lost track of P after this point.

Lets zoom in on these steps.

In a nutshell, here is my thesis wrt multi-process in 0.8: once proper multi-process support lands, any time the GDB client attempts to attach / interact with a PID that isn't FAKE_PID (i.e: 1), gdbstub raises a runtime error (likely including some error text that nudges users towards implementing proper support for multi-process). This is similar to the current behavior in single-threaded mode, in cases where a non-1 tid gets sent/recv'd by gdbstub.

And in this world, I see two ways this scenario you're describing could play out:

  1. with a descriptive runtime failure (as discussed above)
  2. if we simply respond to the vAttach P message with a stop reason that uses FAKE_PID... maybe the GDB client itself simply disregards the P it sent, and only cares that the effect of the vAttach (i.e: the stop reply packet) was that we attached to a process with pid = FAKE_PID?

If the behavior is 2, I feel like we're totally in the clear with my thesis, since it sidesteps the crash scenario entirely. If the behavior is 1, that's less fun for the user... but it doesn't seem unreasonable that if they want to multi-process drift, they should support multi-process extensions?

One thing to note about the multiprocess+ feature:

Note that reporting this feature indicates support for the syntactic extensions only, not that the stub necessarily supports debugging of more than one process at a time.

I bring this up because I'd be curious to understand how a stub would communicate to GDB that it only supports connecting to a single process at a time?

This ties into the question of enabling/disable this feature, as well as my thesis above... as this implies there is a class of targets that support multi-threaded debugging, and can switch between debugging a set of processes... but only one at a time. This is essentially the only kind of "multi-process" target that gdbstub currently supports (via the various hax that have landed in 0.7).

On one hand, it seems a bit sad to lose support for modeling these sorts of targets in multi-thread mode... but on the other hand, maybe it's fine if there's a "complexity jump" in gdbstub's API if you need to support jumping between processes as a multi-thread target?

But again, the crux of my question is how does such a target tell GDB / how does GDB infer that a target that supports multiprocess+ is a target that supports debugging multiple processes simultaneously, vs. one at a time

Here is one theory (which, full disclosure - the robot helped me draft):

GDB's architecture for multi-process debugging is entirely "try it and find out" (optimistic execution). It never infers the stub's capacity upfront because the protocol has no mechanism to communicate "I am a simultaneous target" vs "I am a one-at-a-time target".

Here is how the distinction actually plays out using that remote.c logic:

  1. The "Simultaneous" Target:
  • You attach to P1. GDB sends vAttach;P1. The stub replies with a stop packet. Success.
  • You type add-inferior and attach P2. GDB sends vAttach;P2.
  • The stub supports simultaneous debugging, so it attaches to P2 and sends another stop packet. GDB is now debugging both.
  1. The "One-at-a-Time" Target:
  • You attach to P1. GDB sends vAttach;P1. The stub replies with a stop packet. Success.
  • You type add-inferior and attach P2. GDB sends vAttach;P2.
  • The stub only supports one at a time. Because it is already occupied by P1, it rejects the packet by returning E01.
  • GDB parses the Enn, hits the default: case, triggers the error() macro, and aborts the attach. It tells the user "Attaching to P2 failed."
  • However, if the user had sent a D (detach) or vKill for P1 before trying to attach to P2, the stub would be "empty" again, and would accept the vAttach;P2 request.

So, maybe gdbstub can include some extra logic in single/multi-thread mode to enforce disconnect prior to re-attach?

@jonathanzetier jonathanzetier May 19, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha! Okay, I think that's the root cause of all this discussion! I was trying to keep the ability to attach to non-FAKE_PID processes in non-multiprocess targets, and it sounds like you're okay with dropping it.

Nice catch on the fact that multiprocess is just syntactic and means that the client/stub can send process IDs back and forth. Maybe this is still a compelling reason to not advertise multiprocess+ though? If the intention is to ignore any thread ID with pid != FAKE_PID, then telling the client "don't bother sending us a PID, we're only using the TID anyways" helps keep the client from sending us messages formatted in a way we don't care about...which ironically would actually help preserve the behavior of being able to attach to arbitrary processes.

@daniel5151 daniel5151 May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to keep the ability to attach to non-FAKE_PID processes in non-multiprocess targets, and it sounds like you're okay with dropping it.

Yes. Well... mostly yes.

All the current code related to overriding FAKE_PID is only really there as a transient hack until proper multi-process support can land. I suspect that things "work" today in a pretty jank sense, and it was never totally clear to me what the existing semantics are / what they should be. It worked "well enough" in #129, and that's about it, hah.

So, while thinking about this problem (multi process, attach semantics, etc...), I strongly suggest imagining a codebase before #129.

Now, with that mindset, the question at hand - how to handle attaching?

Honestly, the more I think about it... the more I think that we should just straight up disallow vAttach when running in single/multi-threaded mode, eh? I think it just complicates the problem space way too much.

If someone wants to multi-process drift, they should go ahead and implement the multi-process handlers - simple as that. And whether or not they support attaching to multiple processes simultaneously or not is up to them (and the optionality of supporting simultaneous process debugging can be pointed out in the docs).

That said, I do want to preserve support for vRun when running in single/multi-threaded mode, as its super useful for swapping out the currently running code in, say, emulation contexts. I think that's more tractable, since I think the GDB client understands that the current process has been "swapped out" when you respond with a stop reason that has the same FAKE_PID?

To achieve these new attach / run semantics, we'll need to execute on that ExtendedMode trait rework that I hinted at a while back (recall that it was designed a long time ago, and isn't really aligned with modern gdbstub API design wrt. how it bundles all those ops together), since the current organization really isn't cutting it.

And honestly... now that I think about it... why does "extended mode" even need to be a thing that consumers are aware of?

Concepts of attach, run, etc... can all just be modeled as their own IDETs that hang directly off of {Single,Multi}ThreadBase/MultiProcessBase, and gdbstub can simply infer how to respond to the ! packet based on whether the user has implemented any of those IDETs.

Yeah... it really feels like the ExtendedMode trait should just go away, and its API surface re-allocated across the *Base traits appropriately. No reason to leak this RSP-ism to end users, right?

Maybe this is still a compelling reason to not advertise multiprocess+ though?

I'm open to the idea!

Forcing the feature on was a choice I made a loooooong time ago, and if I had to guess why, it was likely related to future proofing / improving client compatibility... but honestly, I don't totally know.

I do know that at some point, multiprocess+ feature negation was added (bfe83e1) to work around WinDbg being a really dumb GDB RSP client (at the time), but I wager most targets still want multiprocess+ extensions.

Indeed, I'd be interested to see how the changes we're making affect LLDB. The stance on LLDB compat is fuzzy (see #99), but the tl;dr is that I certainly don't want to break LLDB (especially since we just landed some juicy LLDB-only WASM extensions, hah).

If the intention is to ignore any thread ID with pid != FAKE_PID, then telling the client "don't bother sending us a PID, we're only using the TID anyways" helps keep the client from sending us messages formatted in a way we don't care about...which ironically would actually help preserve the behavior of being able to attach to arbitrary processes.

This doesn't really apply if we just straight up say "don't even expose vAttach in multi-threaded mode", but assuming you think that's a bad idea on my part, I'd be interested to see what the GDB client does if you force multiprocess+ off and then try doing some multi-process shenanigans with vAttach.


// The TID (and when multiprocess support added, PID) used for memory/register
// operations
current_mem_tid: T::Tid,
current_resume_tid: SpecificIdKind,
features: ProtocolFeatures,
Expand All @@ -123,14 +130,16 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
_target: PhantomData,
_connection: PhantomData,

// NOTE: `current_mem_tid` and `current_resume_tid` are never queried prior to being set
// by the GDB client (via the 'H' packet), so it's fine to use dummy values here.
// NOTE: `current_active_pid`, `current_mem_tid` and `current_resume_tid` are never
// queried prior to being set by the GDB client (via the 'H' packet), so
// it's fine to use dummy values here.
//
// The alternative would be to use `Option`, and while this would be more "correct", it
// would introduce a _lot_ of noisy and heavy error handling logic all over the place.
//
// Plus, even if the GDB client is acting strangely and doesn't overwrite these values,
// the target will simply return a non-fatal error, which is totally fine.
current_active_pid: FAKE_PID,
current_mem_tid: T::Tid::sentinel(),
// FUTURE: this should get switched over to T::Tid as well
current_resume_tid: SpecificIdKind::WithId(SINGLE_THREAD_TID),
Expand Down
20 changes: 2 additions & 18 deletions src/stub/core_impl/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ use super::prelude::*;
use super::DisconnectReason;
use crate::arch::Arch;
use crate::arch::Registers;
use crate::common::Pid;
use crate::common::Tid;
use crate::protocol::commands::ext::Base;
use crate::protocol::IdKind;
use crate::protocol::SpecificIdKind;
use crate::protocol::SpecificThreadId;
use crate::target::ext::base::BaseOps;
use crate::target::ext::base::ResumeOps;
use crate::FAKE_PID;
use crate::SINGLE_THREAD_TID;

impl<T: Target, C: Connection> GdbStubImpl<T, C> {
Expand Down Expand Up @@ -38,20 +36,6 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
Ok(tid)
}

pub(crate) fn get_current_pid(
&mut self,
target: &mut T,
) -> Result<Pid, Error<T::Error, C::Error>> {
if let Some(ops) = target
.support_extended_mode()
.and_then(|ops| ops.support_current_active_pid())
{
ops.current_active_pid().map_err(Error::TargetError)
} else {
Ok(FAKE_PID)
}
}

// Used by `?` and `vAttach` to return a "reasonable" stop reason.
//
// This is a bit of an implementation wart, since this is really something
Expand All @@ -71,7 +55,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
pid: self
.features
.multiprocess()
.then_some(SpecificIdKind::WithId(self.get_current_pid(target)?)),
.then_some(SpecificIdKind::WithId(self.current_active_pid)),
tid: SpecificIdKind::WithId(tid),
})?;
} else {
Expand Down Expand Up @@ -402,7 +386,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
}
Base::qfThreadInfo(_) => {
res.write_str("m")?;
let pid = self.get_current_pid(target)?;
let pid = self.current_active_pid;

match target.base_ops() {
BaseOps::SingleThread(_) => res.write_specific_thread_id(SpecificThreadId {
Expand Down
15 changes: 7 additions & 8 deletions src/stub/core_impl/extended_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::protocol::commands::ext::ExtendedMode;
use crate::protocol::SpecificIdKind;
use crate::protocol::SpecificThreadId;
use crate::target::ext::base::BaseOps;
use crate::FAKE_PID;

impl<T: Target, C: Connection> GdbStubImpl<T, C> {
pub(crate) fn handle_extended_mode(
Expand All @@ -28,18 +29,16 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
HandlerStatus::Handled
}
ExtendedMode::vAttach(cmd) => {
if ops.support_current_active_pid().is_none() {
return Err(Error::MissingCurrentActivePidImpl);
if let Err(e) = ops.attach(cmd.pid).handle_error() {
self.current_active_pid = FAKE_PID;
return Err(e);
}

ops.attach(cmd.pid).handle_error()?;
self.current_active_pid = cmd.pid;
self.report_reasonable_stop_reason(res, target)?
}
ExtendedMode::qC(_cmd) if ops.support_current_active_pid().is_some() => {
let ops = ops.support_current_active_pid().unwrap();

ExtendedMode::qC(_cmd) => {
res.write_str("QC")?;
let pid = ops.current_active_pid().map_err(Error::TargetError)?;
let pid = self.current_active_pid;
let tid = match target.base_ops() {
BaseOps::SingleThread(_) => T::Tid::sentinel(),
BaseOps::MultiThread(ops) => {
Expand Down
31 changes: 14 additions & 17 deletions src/stub/core_impl/resume.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,6 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
fn write_stop_common(
&mut self,
res: &mut ResponseWriter<'_, C>,
target: &mut T,
tid: Option<T::Tid>,
signal: Signal,
) -> Result<(), Error<T::Error, C::Error>> {
Expand All @@ -309,7 +308,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
pid: self
.features
.multiprocess()
.then_some(SpecificIdKind::WithId(self.get_current_pid(target)?)),
.then_some(SpecificIdKind::WithId(self.current_active_pid)),

@daniel5151 daniel5151 May 15, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems wrong. this should be using the pid specified by the user via the tid parameter?

this ties into my other comment wrt. how current_active_pid shouldn't really be a "thing".

@jonathanzetier jonathanzetier May 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah nice catch! That looks like a remnant that's been there for awhile, but nothing exposed the bug because only one process at a time has been able to be attached to (and before the changes from last week, it was only ever passed a Tid anyways), so the current_active_pid was never different from the PID that the gdb client sent.

tid: SpecificIdKind::WithId(tid.into_fully_qualified_tid()),
})?;
res.write_str(";")?;
Expand Down Expand Up @@ -365,11 +364,10 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
pub(crate) fn finish_signal_with_thread(
&mut self,
res: &mut ResponseWriter<'_, C>,
target: &mut T,
tid: T::Tid,
sig: Signal,
) -> Result<(), Error<T::Error, C::Error>> {
self.write_stop_common(res, target, Some(tid), sig)?;
self.write_stop_common(res, Some(tid), sig)?;
Ok(())
}

Expand All @@ -390,7 +388,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {

crate::__dead_code_marker!("sw_breakpoint", "stop_reason");

self.write_stop_common(res, target, Some(tid), Signal::SIGTRAP)?;
self.write_stop_common(res, Some(tid), Signal::SIGTRAP)?;
res.write_str("swbreak:;")?;
Ok(())
}
Expand All @@ -412,7 +410,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {

crate::__dead_code_marker!("hw_breakpoint", "stop_reason");

self.write_stop_common(res, target, Some(tid), Signal::SIGTRAP)?;
self.write_stop_common(res, Some(tid), Signal::SIGTRAP)?;
res.write_str("hwbreak:;")?;
Ok(())
}
Expand All @@ -436,7 +434,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {

crate::__dead_code_marker!("hw_watchpoint", "stop_reason");

self.write_stop_common(res, target, Some(tid), Signal::SIGTRAP)?;
self.write_stop_common(res, Some(tid), Signal::SIGTRAP)?;

res.write_str(match kind {
WatchKind::Write => "watch:",
Expand Down Expand Up @@ -479,7 +477,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {

crate::__dead_code_marker!("reverse_exec", "stop_reason");

self.write_stop_common(res, target, tid, Signal::SIGTRAP)?;
self.write_stop_common(res, tid, Signal::SIGTRAP)?;

res.write_str("replaylog:")?;
res.write_str(match pos {
Expand All @@ -505,7 +503,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {

crate::__dead_code_marker!("catch_syscall", "stop_reason");

self.write_stop_common(res, target, tid, Signal::SIGTRAP)?;
self.write_stop_common(res, tid, Signal::SIGTRAP)?;

res.write_str(match position {
CatchSyscallPosition::Entry => "syscall_entry:",
Expand All @@ -520,10 +518,9 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
pub(crate) fn finish_library(
&mut self,
res: &mut ResponseWriter<'_, C>,
target: &mut T,
tid: T::Tid,
) -> Result<(), Error<T::Error, C::Error>> {
self.write_stop_common(res, target, Some(tid), Signal::SIGTRAP)?;
self.write_stop_common(res, Some(tid), Signal::SIGTRAP)?;
res.write_str("library:;")?;
Ok(())
}
Expand All @@ -541,13 +538,13 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
}

crate::__dead_code_marker!("fork_events", "stop_reason");
self.write_stop_common(res, target, Some(cur_tid), Signal::SIGTRAP)?;
self.write_stop_common(res, Some(cur_tid), Signal::SIGTRAP)?;
res.write_str("fork:")?;
res.write_specific_thread_id(SpecificThreadId {
pid: self
.features
.multiprocess()
.then_some(SpecificIdKind::WithId(self.get_current_pid(target)?)),
.then_some(SpecificIdKind::WithId(self.current_active_pid)),
tid: SpecificIdKind::WithId(new_tid.into_fully_qualified_tid()),
})?;
res.write_str(";")?;
Expand All @@ -567,13 +564,13 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
}

crate::__dead_code_marker!("vfork_events", "stop_reason");
self.write_stop_common(res, target, Some(cur_tid), Signal::SIGTRAP)?;
self.write_stop_common(res, Some(cur_tid), Signal::SIGTRAP)?;
res.write_str("vfork:")?;
res.write_specific_thread_id(SpecificThreadId {
pid: self
.features
.multiprocess()
.then_some(SpecificIdKind::WithId(self.get_current_pid(target)?)),
.then_some(SpecificIdKind::WithId(self.current_active_pid)),
tid: SpecificIdKind::WithId(new_tid.into_fully_qualified_tid()),
})?;
res.write_str(";")?;
Expand All @@ -592,7 +589,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
}

crate::__dead_code_marker!("vforkdone_events", "stop_reason");
self.write_stop_common(res, target, Some(tid), Signal::SIGTRAP)?;
self.write_stop_common(res, Some(tid), Signal::SIGTRAP)?;
res.write_str("vforkdone:;")?;
Ok(())
}
Expand All @@ -609,7 +606,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
}

crate::__dead_code_marker!("vforkdone_events", "stop_reason");
self.write_stop_common(res, target, None, Signal::SIGTRAP)?;
self.write_stop_common(res, None, Signal::SIGTRAP)?;
res.write_str("exec:")?;
res.write_hex_buf(path)?;
res.write_str(";")?;
Expand Down
8 changes: 0 additions & 8 deletions src/stub/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ pub(crate) enum InternalError<T, C> {

// Errors indicative of a error in the user's `Target` implementation / `gdbstub` integration.
ImplicitSwBreakpoints,
// DEVNOTE: this is a temporary workaround for something that can and should
// be caught at compile time via IDETs. That said, since i'm not sure when
// I'll find the time to cut a breaking release of gdbstub, I'd prefer to
// push out this feature as a non-breaking change now.
MissingCurrentActivePidImpl,
MissingToRawId,
TracepointUnsupportedSourceEnumeration,
UnsupportedStopReason,
Expand Down Expand Up @@ -88,7 +83,6 @@ impl<T, C> InternalError<T, C> {

TargetError(_)
| ImplicitSwBreakpoints
| MissingCurrentActivePidImpl
| MissingToRawId
| TracepointUnsupportedSourceEnumeration
| UnsupportedStopReason => "Target",
Expand All @@ -104,7 +98,6 @@ impl<T, C> InternalError<T, C> {
ClientSentNack
| Connection(_, _)
| ImplicitSwBreakpoints
| MissingCurrentActivePidImpl
| MissingToRawId
| PacketBufferOverflow
| PacketParse(_)
Expand Down Expand Up @@ -208,7 +201,6 @@ where

// Errors indicating an error in the user's `Target` implementation / `gdbstub` integration.
ImplicitSwBreakpoints => write!(f, "The target has not opted into using implicit software breakpoints. See `Target::guard_rail_implicit_sw_breakpoints` for more information"),
MissingCurrentActivePidImpl => write!(f, "GDB client attempted to attach to a new process, but the target has not implemented support for `ExtendedMode::support_current_active_pid`"),
MissingToRawId => write!(f, "A RegId was used with an API that requires raw register IDs to be available (e.g. `StopReasonReporter::add_reg`) but returned `None` from `to_raw_id()`"),
TracepointUnsupportedSourceEnumeration => write!(f, "The target doesn't support the gdbstub TracepointSource extension, but attempted to transition to enumerating tracepoint sources"),
UnsupportedStopReason => write!(f, "{} {}", unsupported_stop_reason!(), SEE_DOCS_FOR_CONTEXT),
Expand Down
4 changes: 2 additions & 2 deletions src/stub/state_machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ where

gdb.i
.inner
.finish_signal_with_thread(&mut res, target, tid, signal)?;
.finish_signal_with_thread(&mut res, tid, signal)?;

Ok(StopReasonReporter {
target,
Expand Down Expand Up @@ -681,7 +681,7 @@ where

let mut res = ResponseWriter::from_state(&mut gdb.i.conn, res);

gdb.i.inner.finish_library(&mut res, target, tid)?;
gdb.i.inner.finish_library(&mut res, tid)?;

Ok(StopReasonReporter {
target,
Expand Down
Loading