microsoft / MIEngine

The Visual Studio MI Debug Engine ("MIEngine") provides an open-source Visual Studio Debugger extension that works with MI-enabled debuggers such as gdb and lldb.
MIT License
813 stars 219 forks source link

[VS Code] Add memory view, assembly view and debugging for C/C++ #816

Closed Izikiel closed 3 years ago

Izikiel commented 5 years ago

Referencing this issue in the cpptools repo, I would be willing to try and implement the feature if I could get a few examples or pointers of how the engine code interacts with gdb and vs code.

gregg-miskelly commented 5 years ago

This is a fairly large amount of work to take on. So if you have the cycles, that would be very cool, but you might find this a bit scary.

You wouldn't actually need to interact with GDB at all -- all that work is already done because this project supports disassembly and memory in full Visual Studio.

Here is the work that is needed:

  1. The C++ extension (or VS Code itself) needs to provide a disassembly window. Assuming this is done in the extension, I believe this would need to work by having the C++ extension provide HTML/CSS back to VS Code for VS Code to render using the VS Code Webview API. This is probably the single biggest work item. There would also need to be code to kick off the web view. I recently have been looking at adding a 'set next statement' command for C# which is slightly similar (it adds a command to the context menu, which we would probably want as one of the gestures for bringing up the disassembly window -- https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/coreclr-debug/setNextStatement.ts) that might get you slightly started.
  2. A protocol needs to be worked out between the debug adapter and the UI. This doesn't necessarily need to be an official part of the protocol. But it probably does make sense to at least propose something to avoid breaking changes if VS Code later creates something of their own. The protocol is at https://github.com/Microsoft/debug-adapter-protocol/blob/gh-pages/debugAdapterProtocol.json.
  3. The OpenDebugAD7 layer needs to implement the new request to obtain disassembly. To actually fetch diassembly, use IDebugProgram2.GetDisassemblyStream which the MIEngine already implements here.
Izikiel commented 5 years ago

Ok, I will check how to create a webview and populate it

haneefdm commented 5 years ago

Hi @Izikiel did you make any progress? I am interested in the same features but for me, a register view is very important. If you have already started, I want to help. I am an experienced C/C++/C# developer on desktop and embedded software.

Yes, I have the cycles for the next three months -- near full+ time. Half/part time after that.

@gregg-miskelly is spot on. Non trivial work for sure. It is easy to see the user point of view but there are some architectural issues that involve VSCode base, MIEngine and cpptools. There are also presentation decisions. For instance, a (optional) register view can go along with the existing set of views for Variables, Watch, Stack, etc. But memory and disassembly views (with inline source) require a lot of real estate and might require their own window(s) in the document area.

Technically, almost everything can go into cpptools or an extension of my own but I was thinking that any natively compiled code can benefit from these features. Not specifically for C/C++. Last thing I want is to create my own extension/debug-adapters when these features are generally needed by the community.

Please let me know how I can help contribute ... the right way. I am requesting the architects of VSCode base, MIEngine, OpenDebugAD7 and cpptools to discuss, review and approve of a plan, I will do the rest. I just don't know how to contact them all. @pieandcakes could perhaps guide me? I see similar requests on cpptools as well.

Please PM me

haneefdm commented 5 years ago

@gregg-miskelly, looks like @pieandcakes is on vacation until May 1st. Is there someone on the MIEngine or cpptools team that I could converse with?

gregg-miskelly commented 5 years ago

Hi @haneefdm, thanks for your interest!

Based on https://github.com/Microsoft/vscode/issues/31901, I don't think the VS Code team will have any interest in adding a disassembly window or memory window to VS Code. But you can open a new issue to ask if they would be willing to accept one. If not, adding it through the C++ extension should be fine. We just should be careful to keep the code isolated so that we can use it other places in the future.

Let me follow up with some others and I will try to get back to you tomorrow with some guidance.

haneefdm commented 5 years ago

Thanks @gregg-miskelly

Yeah, I had very little hope the vscode core would entertain this, but I thought they should be in the loop just in case. I thought it was between MIEngine and vscode-cpptools. If another extension is preferable, then it would be nice if MIEngine and vscode-cpptools themselves are extensible for custom commands to the debugger. Would hate to duplicate all that great work. I already saw a couple of extensions that did that. I was thinking beyond my needs that it doesn't have to be C/C++ specific.

haneefdm commented 5 years ago

Also, my belief is that MIEngine already has basic support for registers and memory, etc. Even stepping through assembly code (stepi, nexti). Just presenting them to the user in a UI is what is missing in the end

gregg-miskelly commented 5 years ago

Yeah, I had very little hope the vscode core would entertain this, but I thought they should be in the loop just in case. I thought it was between MIEngine and vscode-cpptools. If another extension is preferable, then it would be nice if MIEngine and vscode-cpptools themselves are extensible for custom commands to the debugger.

Yes, I definitively think it should be done with as few ties as possible to MIEngine and/or cpptools. You are certainly welcolme to talk to the VS Code folks on that issue and see if they would accept a PR. Either way, we want to design an extension to the Debug Adapter Protocol which the disassembly/memory windows can consume. This could either be done as an npm package, or just by keeping the code isolated enough inside the cpptools repo that if another component wants it the code is easy to.

Also, my belief is that MIEngine already has basic support for registers and memory, etc. Even stepping through assembly code (stepi, nexti). Just presenting them to the user in a UI is what is missing in the end

For the disassembly and memory windows -- MIEngine should already have the support we need. It is just a matter of adding debug protocol elements for them and then connecting that to the debug protocol by enhancing OpenDebugAD7.

For registers - I am thinking we might be better off just reusing either the watch window (by adding a '$registers' pseduo-variable) or by reusing the locals variable (by adding a custom scope for registers) rather than designing a registers window since such a window would need to be in the document well in VS Code and I am not sure that is really where folks would want it.

haneefdm commented 5 years ago

I made some progress, but I need help getting events when the debugger stops and continues to properly refresh a register view. I made a feature request here. My only testing so far has been with lldb on MacOS and gdb on an ARM board attached to my Mac. Will test soon on Windows.

I posted my progress on adv-cppdbg as a separate extension (not ready for marketplace anytime soon)

Please visit https://github.com/haneefdm/adv-cppdbg

@gregg-miskelly has a better idea (than mine) for registers is to create a fake global scope that contains all the registers. For performance reasons (and usability) I like a global scope (that is what they are) vs. the Watch window. I just took it as far as I could left to my own devices :-)

I know this thread is about memory and disassembly views. Disassembly view is very involved but limiting without a Register view. So, I started there (baby steps). I know how to do a disassembly view especially for embedded folks but I don't think I have the needed API's to do it as an extension.

ci70 commented 5 years ago

What is the status on this?

pieandcakes commented 5 years ago

@optomux I think @haneefdm (thank you!) is working on this during their free time.

haneefdm commented 5 years ago

Thanks to @pieandcakes,] & @gregg-miskelly I got my major hurdle (almost) out of the way #844 and #855. Needed that to justify working on the rest.

The DAP spec has to be enhanced first and adopted/prototyped before they can be implemented in MIEngine/cpptools or another extension. Thanks to @andrewcrawley for the following proposals

Registers: https://github.com/Microsoft/debug-adapter-protocol/issues/41 Memory / Disassembly: https://github.com/Microsoft/debug-adapter-protocol/issues/42

Sorry, I've been away for a while but now I am back. Looking forward to taking a deep dive.

We can implement views/windows without a spec change (outside of MS debug tools), but will not be pretty or efficient or generally available. The protocol proposals are for more than just C/C++, and it would be good to have them formalized.

andrewcrawley commented 5 years ago

Moving this discussion off of the defunct protocol proposal PR:

@haneefdm:

@gregg-miskelly may I ask what the next steps are? and where I can help? my first wish is 'registers'

I'm currently working on putting out a new version of the NuGet package that MIEngine uses to implement the Debug Adapter Protocol. Once the new version with the new protocol objects is out, you'll need to do the following to implement registers:

  1. In OpenDebugAD7's HandleScopesRequestAsync method, call EnumProperties on the frame again, passing the GUID for the "registers" filter (223ae797-bd09-4f28-8241-2763bdc5f713).
  2. If any properties are returned, the frame has registers, and you should emit a "Registers" scope with presentationHint set to registers and expensive set to true.
  3. Make other changes to OpenDebugAD7 to handle a variables request against that scope.

Note that (as I described in the original registers proposal), VS uses a two-level structure for registers, where the set of register properties returned from a stack frame represent register categories (e.g. "CPU", "CPU Flags", etc), and those properties contain the actual registers and their values.

Feel free to email me if you have specific implementation questions once the new NuGet package is out and you have a chance to get into this!

haneefdm commented 5 years ago

@andrewcrawley Sorry for posting in the wrong thread. TBH, I completely forgot about the origin thread (this one)

For some reason, I thought, procedurally, there was more needed (approvals wise) before work should/can begin. psyched!

Yes, I love the 2-level structure and what I had in mind as well. I thought I also saw some source code already describing the listing/nesting for registers in MIEngine. I will find it again.

No hurry on the NuGet Package. My weekend is Tue/Wed

andrewcrawley commented 5 years ago

@haneefdm The updated protocol library package is now available: https://www.nuget.org/packages/Microsoft.VisualStudio.Shared.VsCodeDebugProtocol/16.2.30529.1

haneefdm commented 5 years ago

@andrewcrawley Thanks, that will keep me busy for a few days. You pretty much did the work for me in this post

One thing, I wasn't sure exactly how to use the updated debug-protocol NuGet package into my copy of MIEngine. NuGet PM kept failing (null reference), found another way but I had to edit the packages.config & OpenDebugAD7.csproj manually to change the version and build works and references are updated. I don't think this is how I was supposed to do it :-)

Do you all have a recommended testing matrix? I am planning to test with all applicable debuggers supported by cpptools on latest Win10/Ubuntu/Mac. I don't have any 32-bit OSes though. Plan includes testing with native programs as well as an embedded ARM-CM4 processor.

gregg-miskelly commented 5 years ago

That test matrix sounds very reasonable.

pieandcakes commented 5 years ago

@haneefdm I think you did it right. The steps to update the debug protocol should be to update this line in the packages.config and per the comment above it, update this line in OpenDebugAD7.csproj. I don't think the NuGet Package manager will do the right thing since our projects are configured differently.

We can do a test pass on a 32bit OS for native programs when you have it ready.

haneefdm commented 5 years ago

Thanks all. I found a 32-bit Win10 VM that I forgot all about.

@pieandcakes Yes, those are the exact two lines I edited.

ogamespec commented 4 years ago

Is it so hard to do?

image

haneefdm commented 4 years ago

I implemented the whole thing according to spec. I am just NOT satisfied with my MIEngine source code changes. I will fail my own code/design review. It was just too many changes to fit the existing code structure. It must be a lot simpler than what I did or had to do at the time.

Yes, it is simple to implement on the surface. But there are quite a few details. MIEngine supports multiple debuggers (gdb, lldb, etc.) and I wanted it to work for Intel and ARM architectures. The Test-matrix is kinda big :-) when you include OSes. There are details like what happens you modify a variable in the Variables panel.

FYI: The proposal/spec is to put the Registers as a scope in the Variables window. When the feature does show up, it will be in the Variables panel.

The MIEngine/cpptools team is super helpful, professional and I don't want to let them down or myself. They already put quite a bit of time on this and I will finish the job.

I will do another clean slate implementation in Jan/Feb 2020. Please bear with me. I apologize for the delay

ci70 commented 4 years ago

Thx

andl commented 4 years ago

I will do another clean slate implementation in Jan/Feb 2020. Please bear with me. I apologize for the delay

Hi @haneefdm, is this feature have any preview version, I'd love to try it out if available.

Trass3r commented 4 years ago

The prototype is there: https://github.com/haneefdm/adv-cppdbg But nothing has changed since the last comment.

haneefdm commented 4 years ago

About a memory viewer

I am planning on deploying this in our debugger But to add it to the general-purpose cpptools extension, I am not sure how receptive they will be on the UI side. I know the MIEngine part of cpptools, I can do it there. I can always create a separate extension but I don't have bandwidth to maintain it. I prefer it to be part of an existing extension.

To add it to cpptools, it is a few dozen lines of code. I have a prototype with our debugger which is still work in progress. I need to prove it out before proposing it to cpptools :-)

Trass3r commented 4 years ago

Interesting idea! I also wonder how PlatformIO solved all of this and whether something could be reused: https://docs.platformio.org/en/latest/plus/debugging.html

haneefdm commented 4 years ago

Our extension also has a register, memory, peripheral viewers, and disassembly etc. Except we rolled our own using custom debugger extensions (spec allows this), which is what platform IO probably also did.

I proposed spec changes to the debug protocol so we don't have to do that anymore and they have been approved. Thing left was an implementation. One thing missing is context change notifications when user selects a different thread/stack/frame -- a lot of views have to be updated

We are trying to work within the VSCode framework for just the debugger and using standard specs (or extending them with community involvement) rather than create an IDE. with build management, board support, etc. We are trying to be device/board agnostic.

Trass3r commented 4 years ago

Our extension also has a register, memory, peripheral viewers, and disassembly etc. Except we rolled our own using custom debugger extensions (spec allows this), which is what platform IO probably also did.

Interestingly I can't find the source for https://www.npmjs.com/package/platformio-vscode-debug even though they allegedly open-sourced PIO Plus. It contains code just like https://github.com/Marus/cortex-debug/blob/master/src/frontend/disassembly_content_provider.ts.

yannickowow commented 4 years ago

Hi I think adding this views can be an interesting point. Do you know how to manage correctly this view ? Does it need to be a part in VS Code itself to create this views or can we manage properly a communication between DebugAdapter and UI ? I ask this because it can be a "tricky part" to manage instructionBreakpoints from a custom view instead of a dedicated one, no ?

Thanks in advance

kumardesappan commented 4 years ago

About a memory viewer

I am planning on deploying this in our debugger But to add it to the general-purpose cpptools extension, I am not sure how receptive they will be on the UI side. I know the MIEngine part of cpptools, I can do it there. I can always create a separate extension but I don't have bandwidth to maintain it. I prefer it to be part of an existing extension.

To add it to cpptools, it is a few dozen lines of code. I have a prototype with our debugger which is still work in progress. I need to prove it out before proposing it to cpptools :-)

Hi , Do you a version of this as preview to try out? Any tentative timeline for making available for all?

Thanks for working on this

Regards Kumar.

pieandcakes commented 4 years ago

@WardenGnaw Can you see if we can take what @haneefdm has done and incorporate it into the extension code?

haneefdm commented 4 years ago

I will do a PR this weekend for the vscode-hexeditor. I hope. The changes are super simple. I am sorry it is taking so long.

How it was incorporated into my debugger is also very simple. But that is because we already had a custom request to read memory. I simply switched out our UI with the hexeditor using file IO

To make it into general C++ tools, I feel like we have to the following

WardenGnaw commented 4 years ago

@haneefdm Thank you for working on this.

MIEngine has to implement the read memory request. This is the hard part as it is in C# and has to be tested against gdb, llvm and cppdbg and across all OS platforms

I can add ReadMemory Request into OpenDebugAD7 and add the gdb/lldb MI commands in MIEngine to get the correct ReadMemory response.

cpptools has to repeat what I did for invoking the hexeditor. I added a command to create a memory window and ask the user to enter an expression for the address and a size (constant) and create a local URI to write to a file and open the URI if not already open. The location of the URI was a sticking point. I currently do it in the .vscode dir (where we also store register/peripheral settings) but maybe that is not the best idea. But, that is up to the extension

I couldn't find where you implemented this. I'll wait for your PR to the hextools extension.

cpptools also has to implement an event handler for whenever the debugger pauses to refresh the contents of the URI

I see this is just using the DebugAdapterTracker from the vscode package.

haneefdm commented 4 years ago

@WardenGnaw

I couldn't find where you implemented this. I'll wait for your PR to the hextools extension.

Yes, it is on a separate branch on our debugger that I have not published yet. I will provide the details soon A lot of it is already there for our old memory viewer It was done a long time ago by the wonderful author of that extension. I enhanced it a bit and trying for more

I see this is just using the DebugAdapterTracker from the vscode package.

Yes, but we used custom events generated from our own debug adapter. Again it was done a while ago. We should migrate to the DebugAdapterTracker but have not had the time to do so. It actually simplifies a few things. We also have to make sure we are responding to the right debugger session

haneefdm commented 4 years ago

The first step is done. I did a PR to use a non-zero base address and I hope it will be accepted

https://github.com/microsoft/vscode-hexeditor/pull/170

haneefdm commented 4 years ago

The first step is done. I did a PR to use a non-zero base address and I hope it will be accepted

https://github.com/microsoft/vscode-hexeditor/pull/170

WardenGnaw commented 4 years ago

I also have handled the DAP ReadMemory Request in a seperate branch and will be testing it with a modified VS Code C++ Extension that does the DebugAdapterTracking and can open the HexEditor with the URI.

haneefdm commented 4 years ago

What I did in my debugger extension frontend, once you have read the bytes from the debugger (gdb, llvm, etc.) is to write the data to a file and create a URI. My URI fsPath contains a C-Expression (instead of an address) which can fail because of invalid chars when writing to the file. For now, we left the URI the same and switchable between our memory viewer vs the new one. I still need to make sure my error handling is right, so please ignore that Please don't take the code below literally.

                const filePath = path.join(vscode.workspace.workspaceFolders[0].uri.fsPath, '.vscode', 'cdmem', uri.fsPath);
                const fsURI = vscode.Uri.parse(`file://${filePath}?baseAddress=${data.startAddress}`);
                vscode.workspace.fs.writeFile(fsURI, Uint8Array.from(data.bytes)).then(() => {
                    vscode.commands.executeCommand("vscode.openWith", fsURI, "hexEditor.hexedit", { preview: false });
                    resolve('succesz');
                }, (error) => {
                    const msg = (error.message || '') + '\nPerhaps expression for "address" contains invalid file name characters';
                    vscode.window.showErrorMessage(`Unable to create/write memory file memory from ${fsURI.toString()}: ${msg}`);
                    reject(error.toString());                    
                });

I have a problem right now that I am looking for a solution. Since the code above executes everytime the debugger pauses, it changes focus in a bad way (normal revert does not trigger showing stale contents although file on disk changed) and it changes the active window. Just FYI. Neither of that is good. I want to create a new window only if it does not already exist but since the hexeditor is a virtual document, I don't know if it was already has been created. I don't want to call vscode.commands.executeCommand if the window/tab already exists. I did not see a way to iterate/enumerate over virtual docs. This is actually a serious problem, I hope I will figure it out.

When the debugger pauses for whatever reason, the focus should be on the code location and not the memory window(s). You can have multiple.right? Baby steps :-)

Trass3r commented 4 years ago

Just for the record, I have a crude but working prototype for disassembly debugging when no source is available.

RazCrimson commented 4 years ago

Can u provide something like a preview of the extension? I(a student) am supposed to learn Assembly this semester and it seems like this will make debugging raw assembly code a lot more easier than running a bunch of -exec info registers.

vannaka commented 4 years ago

@RazCrimson3 A great solution while your waiting on this is to configure gdb with register and memory views. See this config file: here. This gdb init file is a whole other experience to what you're used to with gdb, check it out.

Trass3r commented 4 years ago

@RazCrimson3 I recommend using the CodeLLDB extension or gdbgui until this is ready.

RazCrimson commented 3 years ago

@vannaka @Trass3r Thank you for your recommendations, will try them out!

WardenGnaw commented 3 years ago

DAP Protocol for Disassembly and Memory have been implemented in MIEngine. The associated UI requirements are on VS Code.

Please follow the following issues: