Skip to content
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.

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

Support readMemory/writeMemory from debuggers #126268

Closed
connor4312 opened this issue May 25, 2021 · 12 comments
Closed

Support readMemory/writeMemory from debuggers #126268

connor4312 opened this issue May 25, 2021 · 12 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@connor4312
Copy link
Member

Similar: microsoft/vscode-hexeditor#70

@connor4312 connor4312 self-assigned this May 25, 2021
@jasonwilliams
Copy link
Contributor

This would be very cool

@connor4312 connor4312 transferred this issue from microsoft/vscode-js-debug Jun 14, 2021
@connor4312
Copy link
Member Author

connor4312 commented Jun 14, 2021

DAP has support for readMemory requests which would do what we need in a generic way. However, we don't use it in vscode yet. I think either adding support for it in core, or having the hex editor extension use the DebugAdapterTracker to support memory requests itself--which can add context menu actions and so on--is the way to go.

cc @weinand @isidorn @lramos15

@connor4312 connor4312 added the debug Debug viewlet, configurations, breakpoints, adapter issues label Jun 14, 2021
@weinand
Copy link
Contributor

weinand commented Jun 15, 2021

@connor4312 I wonder whether we should align the work on a "Memory View" with the current work on the "Disassembly View" (see #124163 and the corresponding PR #125737).
Reason being: a "Disassembly" view could be considered as just another representation of a "Memory" view.

In addition, I'm currently working on the spec for the "writing" side and change notifications of DAP's memory feature. You might want to chime in on the spec discussions.

@weinand weinand added this to the On Deck milestone Jun 15, 2021
@weinand weinand added the feature-request Request for new features or functionality label Jun 15, 2021
@lramos15
Copy link
Member

I would be happy to help with the hex editor side if need be. Although the hex editor being an external extension might feel out of place if we have a disassembly view in one case and a hex extension in another.

@connor4312
Copy link
Member Author

connor4312 commented Jun 15, 2021

I think disassembly is mostly separate as far as implementation goes. Visually the left side of the disassembly table is similar, but it doesn't need the "editor" part of the hex editor -- static raw display is rather trivial.

👍 on getting the writing side going! Left a couple comments

@connor4312 connor4312 changed the title Allow viewing Buffers/ArrayBuffers/Uint8Arrays in hex editor Support readMemory/writeMemory from debuggers Sep 21, 2021
@connor4312
Copy link
Member Author

Update here: now that we have a full set of read/writeMemory in DAP, we should implement this in a generic way for all debuggers who implement these methods.

I think it makes sense to deduplicate the hex editor -- either implementing an editor for binary data core, or using the hex editor for DAP read/write memory. @lramos15 @roblourens any thoughts on this?

@roblourens
Copy link
Member

roblourens commented Sep 21, 2021

I like using the hex editor to do this. We just have some data that we want to edit, we can launch an appropriate editor to edit it. Maybe that's the hex editor by default, then if it represents text, I could open it in the text editor. If it represents an image, I can open it in an image editor. Is that possible?

@lramos15
Copy link
Member

I guess the question becomes how does this work if it takes a bunch of dependencies on non built-in extensions

@roblourens
Copy link
Member

Let's build it in 😄

@connor4312
Copy link
Member Author

connor4312 commented Sep 22, 2021

Talked about built-in versus the hex editor in standup a bit with @Tyriar.

I don't think our APIs in the extension host are suitable for this right now. The FileSystemProvider has proposed file-descriptor based low level read/write APIs that would work for this, but these aren't exposed in the read side and sound like they might be going away regardless (#84515). Also, something that cannot be represented in them is the "unreadable" ranges which DAP supports. Generally we might be only able to (or requested) read certain regions of program memory. The APIs, and the UI, should deal with this.

For the hex editor we'd need to introduce new proposed APIs for this, or just have a bunch of commands and special logic within the editor. This simple way would be to have analogous open/read/write/close commands that operate on debug memory.

if it represents text, I could open it in the text editor. If it represents an image, I can open it in an image editor. Is that possible?

Because of the different semantics around reading program memory, I don't think this is feasible in the current protocol. For an image editor to work we'd need to load the entire image, which we don't necessarily know how to deal with. For example, a memory reference in C might point to a heap address from a void * pointer. Neither DAP nor the program itself provides a representation to indicate much memory is associated with that address.

@connor4312
Copy link
Member Author

connor4312 commented Sep 22, 2021

Actually, a more reusable way to do this would be to use the existing filesystem APIs and allow the URIs to include the memory ranges to read. We would still need some way to indicate "unreadable" memory, but that would work. It also allows other extensions to hook into debugger memory relatively easily for more creative things like @hediet's extensions.

connor4312 added a commit that referenced this issue Sep 22, 2021
This implements the core memory "model" for debugging that reflects
DAP, in `debugModel.ts`. It also implements a filesystem provider based
on that in `debugMemory.ts`, for tentative application in the hex editor.

Finally it adds context menu items for these. This works with changes in
mock debug, but for some reason reopening the ".bin" file in the hex
editor results in a blank editor. Still need to look at that.

Ultimately though, as indicated in #126268, we'll probably want custom
commands for the hex editor to call as low level read/write is not
supported in the stable API. Also, the file API doesn't represent the
"unreadable" ranges which DAP supports.
@connor4312 connor4312 modified the milestones: On Deck, November 2021 Oct 30, 2021
@connor4312
Copy link
Member Author

For visibility, the approach we decided on is to indeed use the Hex Editor, which we're working on making usable for this scenario. Initially we'll probably have some special commands for debug memory since workspace.fs doesn't have the i/o primitives we need, but this can be later used to push forward formal extension API.

connor4312 added a commit that referenced this issue Dec 30, 2021
This implements the core memory "model" for debugging that reflects
DAP, in `debugModel.ts`. It also implements a filesystem provider based
on that in `debugMemory.ts`, for tentative application in the hex editor.

Finally it adds context menu items for these. This works with changes in
mock debug, but for some reason reopening the ".bin" file in the hex
editor results in a blank editor. Still need to look at that.

Ultimately though, as indicated in #126268, we'll probably want custom
commands for the hex editor to call as low level read/write is not
supported in the stable API. Also, the file API doesn't represent the
"unreadable" ranges which DAP supports.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests

6 participants