GDB: allow access while CPU is running (code refactor)#383
Conversation
Cannot use GDB CLI commands while CPU is running eclipse-cdt-cloud#82
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors GDB-related variable and command handling to support accessing GDB while the CPU/MCU is running, as a groundwork for the upcoming "gdb non stop mode" feature. The changes replace separate frame and thread identifiers with a consolidated FrameReference object, and update related functions and command generation accordingly.
- Replaced individual frameId/threadId parameters with FrameReference objects.
- Updated associated variable manager, MI command functions, and debug session interactions.
- Adjusted function signatures and command formats across multiple files to ensure consistent handling of frame references.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/varManager.ts | Updated variable management functions to use FrameReference |
| src/mi/var.ts | Modified var creation to use FrameReference |
| src/mi/stack.ts | Refactored stack frame selection and variable listing |
| src/mi/interpreter.ts | Adjusted interpreter exec command to use FrameReference |
| src/mi/exec.ts | Changed exec finish to accept a FrameReference |
| src/mi/data.ts | Updated data commands to use FrameReference |
| src/gdb/GDBDebugSessionBase.ts | Refactored various debug session commands to incorporate FrameReference |
Comments suppressed due to low confidence (1)
src/gdb/GDBDebugSessionBase.ts:1248
- Using 'args.frameId' as a truthy check can misinterpret a valid value (e.g. 0) as falsy. Consider explicitly checking for null or undefined (for example,
args.frameId != null) to ensure frame ID 0 is handled correctly.
const frameRef = args.frameId ? this.frameHandles.get(args.frameId) : undefined;
|
The refactoring seems ok to me on the whole. The adapter has historically been a little underwhelming on achieving semantic versioning. As there are many changes to method signatures in this change, I think a little bit of effort on ensuring extenders know about the change is needed. Perhaps a version bump, a changelog entry, or something else. cc: @asimgunes AFAICT this change in method signatures wouldn't break Renesas' extension, but I thought you could comment on the above paragraph. This relates to #362 (@thorstendb-ARM please put such links in commit message + PR summary so we can identify what goes with what) |
Would suggest to go with changelog entries. We've added one with the latest release: https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/blob/main/CHANGELOG.md |
Hi @jonahgraham , @thorstendb-ARM , Thanks for letting me know about the update. I was not expecting any particular issue that should break Renesas extension. |
|
Thanks for the reviews. I am merging now. We may however want to wait with another release for another breakpoint related PR coming soon. Based on that I would kick off another adapter and extension release. |
|
Seeing test timeouts after merge. Restarting jobs for a second time (this helped elsewhere before). Will look in the afternoon in bumping up test timeouts and if that helps. |
…loud#383) * GDB: allow access while CPU is running Cannot use GDB CLI commands while CPU is running eclipse-cdt-cloud#82
Issue:
This PR does some code refactoring in preperation for the "gdb non stop mode" feature, or, in general: Access gdb (MCU) while MCU is running.