Supporting p flag for H packets - #191
Conversation
Adding parsing out p flag from H packets
Added accepting the process flag to attach to appropriate process on H packet
| Some(H { kind, thread }) | ||
| let thread: ThreadId; | ||
| let process: Pid; | ||
| if body[1] == b'p' { //process is attached via H |
There was a problem hiding this comment.
I'm not totally sure what this is all about, given that ThreadId already parses the (optional) Process ID?
https://github.com/daniel5151/gdbstub/blob/master/src/protocol/common/thread_id.rs#L21
If there's a bug in that parsing, we should fix the code in ThreadId, not dupe it here
There was a problem hiding this comment.
No bug it just doesn't cover my use case. I'm using the single thread base but for multiple processes, so I need to trigger the extended mode attach to switch processes. The handling for the H command is designed for the multithreaded case, it does not impact a single threaded state machine (at least in a meaningful enough way to switch what is being debugged under the hood). It would definitely be more appropriate to use the existing thread field, I just quickly cobbled this together for my purposes.
There was a problem hiding this comment.
Ah, ok, I see. You just missed the fact the pid was already being parsed, so this code is double parsing it.
| } | ||
| if let Some(ops) = target.support_extended_mode() { | ||
| if cmd.attach_process == true { | ||
| ops.attach(cmd.process); |
There was a problem hiding this comment.
overloading the semantics of existing well-defined GDB protocol extensions isn't something we'd want to do... but depending on the outcome of our exploration / discussion - I could maybe imagine adding a new fn on_current_mem_tid_change(&mut self, tid: Tid, pid: Pid) method to EntendedMode (with an empty default body) that users can overwrite to react to these packets?
There was a problem hiding this comment.
That would be a much cleaner approach than what I have here, I'll be frank and did not realize tid contained an optional pid field but alas. Using extended-mode for multi process debugging requires calling attach (or attach like functionality) to switch because I don't use self.current_mem_tid for state. Though perhaps it would be easier If I just made the transition to doing so.
There was a problem hiding this comment.
Like I said in #124 (comment), I'm not opposed to adding more workarounds to the missing target::ext::base::multiprocess support if they're useful, but in this case, if the existing attach-based workaround would work for you - I'd rather just keep one workaround, vs. adding a second slightly different (but similarly/equivalently powerful) workaround
There was a problem hiding this comment.
Unless I'm missing something, the attach based work around falls short just a little with inferiors. So perhaps I should of clarified, even with the CurrentActivePid trait, the client (Ubuntu gdb-multiarch) does not query for the current pid on an inferior switch. The H packet contains the new pid and the client expects the stub to have switched. I was experiencing an issue where switching inferiors was not actually changing the process being debugged, which is what prompted me to write these changes. I realize this might be highly client dependent.
Or are you suggesting I just use attach exclusively as the switching mechanism? I am trying to avoid doing that, because that kills the currently attached process.
There was a problem hiding this comment.
Ok, I believe I understand what you're getting at here. I'll respond on the main multi-process debugging comment.
(and depending on how that discussion shakes out - maybe we'll come back to this draft PR to tweak it into something merge-able, or just abandon it)
2 participants
Description
I have been building a remote attachable gdbserver in a new os using gdbstub. I am currently working towards single threaded, but concurrently multi-process being debugged. To do so I have added a set of monitor commands to list processes and attach to one. I have implemented the extended mode so I can connect via extended-remote, and spawn multiple inferiors on the same connection. I can attach different processes to each inferior and debug correctly. However, when I attempt to switch back the underlying program being debugged does not change, as I can see through my monitor commands. This is because my gdbclient on switching to an attached inferior sends:
(gdb) inferior 3
[Switching to inferior 3 [process 80] ()]
[Switching to thread 3.1 (Thread 80.1)]
[remote] Sending packet: $Hgp50.1#e3
With the H packet containing the hex process id after p. The current implementation of the gdbstub does not handle the p flag for H commands. However, doing so would allow support for debugging multiple attached processes through the extended mode. I am not confident of the impact adding this will have for multi-threaded programs, but it seems like a step in the right direction. I have added some preliminary code that is stable enough to support what I need from my singlethreaded environment. Now when I call inferior, the Pid is extracted and an internal attach is called without killing the other program so both can persist and each inferior correctly links to one process. Is this something you would be interested in without the effort of creating new traits for multiprocess? This is not robust support but empirically is sufficient with extended mode support to multiple inferiors at once.
Addresses #124
API Stability
Checklist
rustdocformatting looks good (viacargo doc)examples/armv4twithRUST_LOG=trace+ any relevant GDB output under the "Validation" section below./example_no_std/check_size.shbefore/after changes under the "Validation" section belowexamples/armv4t./example_no_std/check_size.sh)ArchimplementationValidation
More of a starting point, but I have validated it.